Allow MetadataMap to be a status in TrySeq (#28490)

* Enable metadata to be a 'status' for TrySeq

Begins to allow us to return trailing metadata and get the right
behaviors.

* comment

CMakeLists.txt

@@ -1014,6 +1014,7 @@ if(gRPC_BUILD_TESTS)
   add_dependencies(buildtests_cxx too_many_pings_test)
   add_dependencies(buildtests_cxx transport_stream_receiver_test)
   add_dependencies(buildtests_cxx try_join_test)
+  add_dependencies(buildtests_cxx try_seq_metadata_test)
   add_dependencies(buildtests_cxx try_seq_test)
   add_dependencies(buildtests_cxx unknown_frame_bad_client_test)
   add_dependencies(buildtests_cxx uri_parser_test)
@@ -16145,6 +16146,41 @@ target_link_libraries(try_join_test
 )
 
 
+endif()
+if(gRPC_BUILD_TESTS)
+
+add_executable(try_seq_metadata_test
+  test/core/promise/try_seq_metadata_test.cc
+  third_party/googletest/googletest/src/gtest-all.cc
+  third_party/googletest/googlemock/src/gmock-all.cc
+)
+
+target_include_directories(try_seq_metadata_test
+  PRIVATE
+    ${CMAKE_CURRENT_SOURCE_DIR}
+    ${CMAKE_CURRENT_SOURCE_DIR}/include
+    ${_gRPC_ADDRESS_SORTING_INCLUDE_DIR}
+    ${_gRPC_RE2_INCLUDE_DIR}
+    ${_gRPC_SSL_INCLUDE_DIR}
+    ${_gRPC_UPB_GENERATED_DIR}
+    ${_gRPC_UPB_GRPC_GENERATED_DIR}
+    ${_gRPC_UPB_INCLUDE_DIR}
+    ${_gRPC_XXHASH_INCLUDE_DIR}
+    ${_gRPC_ZLIB_INCLUDE_DIR}
+    third_party/googletest/googletest/include
+    third_party/googletest/googletest
+    third_party/googletest/googlemock/include
+    third_party/googletest/googlemock
+    ${_gRPC_PROTO_GENS_DIR}
+)
+
+target_link_libraries(try_seq_metadata_test
+  ${_gRPC_PROTOBUF_LIBRARIES}
+  ${_gRPC_ALLTARGETS_LIBRARIES}
+  grpc
+)
+
+
 endif()
 if(gRPC_BUILD_TESTS)
 

build_autogenerated.yaml

@@ -8022,6 +8022,17 @@ targets:
   - absl/status:statusor
   - absl/types:variant
   uses_polling: false
+- name: try_seq_metadata_test
+  gtest: true
+  build: test
+  language: c++
+  headers:
+  - src/core/lib/promise/try_seq.h
+  src:
+  - test/core/promise/try_seq_metadata_test.cc
+  deps:
+  - grpc
+  uses_polling: false
 - name: try_seq_test
   gtest: true
   build: test

src/core/lib/promise/detail/status.h

@@ -41,4 +41,9 @@ inline absl::Status IntoStatus(absl::Status* status) {
 }  // namespace promise_detail
 }  // namespace grpc_core
 
+// Return true if the status represented by the argument is ok, false if not.
+// By implementing this function for other, non-absl::Status types, those types
+// can participate in TrySeq as result types that affect control flow.
+inline bool IsStatusOk(const absl::Status& status) { return status.ok(); }
+
 #endif  // GRPC_CORE_LIB_PROMISE_DETAIL_STATUS_H

src/core/lib/promise/try_seq.h

@@ -25,14 +25,15 @@
 #include "absl/types/variant.h"
 
 #include "src/core/lib/promise/detail/basic_seq.h"
+#include "src/core/lib/promise/detail/status.h"
 #include "src/core/lib/promise/poll.h"
 
 namespace grpc_core {
 
 namespace promise_detail {
 
-template <typename T>
+template <typename T, typename Ignored = void>
-struct TrySeqTraits {
+struct TrySeqTraitsWithSfinae {
   using UnwrappedType = T;
   using WrappedType = absl::StatusOr<T>;
   template <typename Next>
@@ -47,7 +48,7 @@ struct TrySeqTraits {
 };
 
 template <typename T>
-struct TrySeqTraits<absl::StatusOr<T>> {
+struct TrySeqTraitsWithSfinae<absl::StatusOr<T>> {
   using UnwrappedType = T;
   using WrappedType = absl::StatusOr<T>;
   template <typename Next>
@@ -62,23 +63,30 @@ struct TrySeqTraits<absl::StatusOr<T>> {
     return run_next(std::move(prior));
   }
 };
-template <>
+// If there exists a function 'IsStatusOk(const T&) -> bool' then we assume that
-struct TrySeqTraits<absl::Status> {
+// T is a status type for the purposes of promise sequences, and a non-OK T
+// should terminate the sequence and return.
+template <typename T>
+struct TrySeqTraitsWithSfinae<
+    T, absl::enable_if_t<
+           std::is_same<decltype(IsStatusOk(std::declval<T>())), bool>::value,
+           void>> {
   using UnwrappedType = void;
-  using WrappedType = absl::Status;
+  using WrappedType = T;
   template <typename Next>
-  static auto CallFactory(Next* next, absl::Status&&)
+  static auto CallFactory(Next* next, T&&) -> decltype(next->Once()) {
-      -> decltype(next->Once()) {
     return next->Once();
   }
   template <typename Result, typename RunNext>
-  static Poll<Result> CheckResultAndRunNext(absl::Status prior,
+  static Poll<Result> CheckResultAndRunNext(T prior, RunNext run_next) {
-                                            RunNext run_next) {
+    if (!IsStatusOk(prior)) return Result(std::move(prior));
-    if (!prior.ok()) return Result(prior);
     return run_next(std::move(prior));
   }
 };
 
+template <typename T>
+using TrySeqTraits = TrySeqTraitsWithSfinae<T>;
+
 template <typename... Fs>
 using TrySeq = BasicSeq<TrySeqTraits, Fs...>;
 

src/core/lib/transport/metadata_batch.h

@@ -1107,6 +1107,14 @@ class MetadataMap {
   ChunkedVector<std::pair<Slice, Slice>, 10> unknown_;
 };
 
+// Ok/not-ok check for metadata maps that contain GrpcStatusMetadata, so that
+// they can be used as result types for TrySeq.
+template <typename... Args>
+inline bool IsStatusOk(const MetadataMap<Args...>& m) {
+  return m.get(GrpcStatusMetadata()).value_or(GRPC_STATUS_UNKNOWN) ==
+         GRPC_STATUS_OK;
+}
+
 template <typename... Traits>
 MetadataMap<Traits...>::MetadataMap(Arena* arena) : unknown_(arena) {}
 

test/core/promise/BUILD

@@ -200,6 +200,20 @@ grpc_cc_test(
     ],
 )
 
+grpc_cc_test(
+    name = "try_seq_metadata_test",
+    srcs = ["try_seq_metadata_test.cc"],
+    external_deps = ["gtest"],
+    language = "c++",
+    uses_polling = False,
+    deps = [
+        "//:grpc",
+        "//:grpc_base",
+        "//:try_seq",
+        "//test/core/util:grpc_suppressions",
+    ],
+)
+
 grpc_cc_test(
     name = "activity_test",
     srcs = ["activity_test.cc"],

test/core/promise/try_seq_metadata_test.cc

@@ -0,0 +1,50 @@
+// Copyright 2021 gRPC authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <gtest/gtest.h>
+
+#include "src/core/lib/promise/try_seq.h"
+#include "src/core/lib/resource_quota/resource_quota.h"
+#include "src/core/lib/transport/metadata_batch.h"
+
+namespace grpc_core {
+
+static auto* g_memory_allocator = new MemoryAllocator(
+    ResourceQuota::Default()->memory_quota()->CreateMemoryAllocator("test"));
+
+using TestMap = MetadataMap<GrpcStatusMetadata>;
+
+TEST(PromiseTest, SucceedAndThenFail) {
+  auto arena = MakeScopedArena(1024, g_memory_allocator);
+  Poll<TestMap> r = TrySeq(
+      [&arena] {
+        TestMap m(arena.get());
+        m.Set(GrpcStatusMetadata(), GRPC_STATUS_OK);
+        return m;
+      },
+      [&arena]() {
+        TestMap m(arena.get());
+        m.Set(GrpcStatusMetadata(), GRPC_STATUS_UNAVAILABLE);
+        return m;
+      })();
+  EXPECT_EQ(absl::get<TestMap>(r).get(GrpcStatusMetadata()),
+            GRPC_STATUS_UNAVAILABLE);
+}
+
+}  // namespace grpc_core
+
+int main(int argc, char** argv) {
+  ::testing::InitGoogleTest(&argc, argv);
+  return RUN_ALL_TESTS();
+}

tools/run_tests/generated/tests.json

@@ -7117,6 +7117,30 @@
     ],
     "uses_polling": false
   },
+  {
+    "args": [],
+    "benchmark": false,
+    "ci_platforms": [
+      "linux",
+      "mac",
+      "posix",
+      "windows"
+    ],
+    "cpu_cost": 1.0,
+    "exclude_configs": [],
+    "exclude_iomgrs": [],
+    "flaky": false,
+    "gtest": true,
+    "language": "c++",
+    "name": "try_seq_metadata_test",
+    "platforms": [
+      "linux",
+      "mac",
+      "posix",
+      "windows"
+    ],
+    "uses_polling": false
+  },
   {
     "args": [],
     "benchmark": false,