Skip to content

Commit

Permalink
Revert "Expose trailers-only response status through C++ callback API (
Browse files Browse the repository at this point in the history
…grpc#26249)" (grpc#26363)

This reverts commit 393bae7.
  • Loading branch information
yashykt authored May 25, 2021
1 parent 05aa736 commit 05c3b30
Show file tree
Hide file tree
Showing 8 changed files with 6 additions and 75 deletions.
8 changes: 4 additions & 4 deletions include/grpcpp/impl/codegen/client_callback.h
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,7 @@ class ClientCallbackReaderWriterImpl
start_tag_.Set(
call_.call(),
[this](bool ok) {
reactor_->OnReadInitialMetadataDone(ok && !context_->trailers_only());
reactor_->OnReadInitialMetadataDone(ok);
MaybeFinish(/*from_reaction=*/true);
},
&start_ops_, /*can_inline=*/false);
Expand Down Expand Up @@ -737,7 +737,7 @@ class ClientCallbackReaderImpl : public ClientCallbackReader<Response> {
start_tag_.Set(
call_.call(),
[this](bool ok) {
reactor_->OnReadInitialMetadataDone(ok && !context_->trailers_only());
reactor_->OnReadInitialMetadataDone(ok);
MaybeFinish(/*from_reaction=*/true);
},
&start_ops_, /*can_inline=*/false);
Expand Down Expand Up @@ -995,7 +995,7 @@ class ClientCallbackWriterImpl : public ClientCallbackWriter<Request> {
start_tag_.Set(
call_.call(),
[this](bool ok) {
reactor_->OnReadInitialMetadataDone(ok && !context_->trailers_only());
reactor_->OnReadInitialMetadataDone(ok);
MaybeFinish(/*from_reaction=*/true);
},
&start_ops_, /*can_inline=*/false);
Expand Down Expand Up @@ -1121,7 +1121,7 @@ class ClientCallbackUnaryImpl final : public ClientCallbackUnary {
start_tag_.Set(
call_.call(),
[this](bool ok) {
reactor_->OnReadInitialMetadataDone(ok && !context_->trailers_only());
reactor_->OnReadInitialMetadataDone(ok);
MaybeFinish();
},
&start_ops_, /*can_inline=*/false);
Expand Down
2 changes: 0 additions & 2 deletions include/grpcpp/impl/codegen/client_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -493,8 +493,6 @@ class ClientContext {
const grpc::ServerContextBase& server_context,
PropagationOptions options);

bool trailers_only() const;

bool initial_metadata_received_;
bool wait_for_ready_;
bool wait_for_ready_explicitly_set_;
Expand Down
11 changes: 1 addition & 10 deletions src/core/lib/surface/call.cc
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,6 @@ struct grpc_call {
bool destroy_called = false;
/** flag indicating that cancellation is inherited */
bool cancellation_is_inherited = false;
// Trailers-only response status
bool is_trailers_only = false;
/** which ops are in-flight */
bool sent_initial_metadata = false;
bool sending_message = false;
Expand Down Expand Up @@ -1827,10 +1825,7 @@ static grpc_call_error call_start_batch(grpc_call* call, const grpc_op* ops,
&call->metadata_batch[1 /* is_receiving */][0 /* is_trailing */];
stream_op_payload->recv_initial_metadata.recv_initial_metadata_ready =
&call->receiving_initial_metadata_ready;
if (call->is_client) {
stream_op_payload->recv_initial_metadata.trailing_metadata_available =
&call->is_trailers_only;
} else {
if (!call->is_client) {
stream_op_payload->recv_initial_metadata.peer_string =
&call->peer_string;
}
Expand Down Expand Up @@ -2022,10 +2017,6 @@ grpc_compression_algorithm grpc_call_compression_for_level(
return algo;
}

bool grpc_call_is_trailers_only(const grpc_call* call) {
return call->is_trailers_only;
}

bool grpc_call_failed_before_recv_message(const grpc_call* c) {
return c->call_failed_before_recv_message;
}
Expand Down
5 changes: 0 additions & 5 deletions src/core/lib/surface/call.h
Original file line number Diff line number Diff line change
Expand Up @@ -120,11 +120,6 @@ size_t grpc_call_get_initial_size_estimate();
grpc_compression_algorithm grpc_call_compression_for_level(
grpc_call* call, grpc_compression_level level);

/* Did this client call receive a trailers-only response */
/* TODO(markdroth): This is currently available only to the C++ API.
Move to surface API if requested by other languages. */
bool grpc_call_is_trailers_only(const grpc_call* call);

/* Returns whether or not the call's receive message operation failed because of
* an error (as opposed to a graceful end-of-stream) */
/* TODO(markdroth): This is currently available only to the C++ API.
Expand Down
8 changes: 0 additions & 8 deletions src/cpp/client/client_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@
#include <grpcpp/server_context.h>
#include <grpcpp/support/time.h>

#include "src/core/lib/surface/call.h"

namespace grpc {

class Channel;
Expand Down Expand Up @@ -180,10 +178,4 @@ void ClientContext::SetGlobalCallbacks(GlobalCallbacks* client_callbacks) {
g_client_callbacks = client_callbacks;
}

bool ClientContext::trailers_only() const {
bool result = initial_metadata_received_ && grpc_call_is_trailers_only(call_);
GPR_DEBUG_ASSERT(!result || recv_initial_metadata_.arr()->count == 0);
return result;
}

} // namespace grpc
1 change: 0 additions & 1 deletion src/proto/grpc/testing/echo.proto
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ service EchoTestService {
rpc ResponseStream(EchoRequest) returns (stream EchoResponse);
rpc BidiStream(stream EchoRequest) returns (stream EchoResponse);
rpc Unimplemented(EchoRequest) returns (EchoResponse);
rpc UnimplementedBidi(stream EchoRequest) returns (stream EchoResponse);
}

service EchoTest1Service {
Expand Down
41 changes: 0 additions & 41 deletions test/cpp/end2end/client_callback_end2end_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1403,47 +1403,6 @@ TEST_P(ClientCallbackEnd2endTest, UnimplementedRpc) {
}
}

TEST_P(ClientCallbackEnd2endTest, TestTrailersOnlyOnError) {
// Note that trailers-only is an HTTP/2 concept so we shouldn't do this test
// for any other transport such as inproc.
if (GetParam().protocol != Protocol::TCP) {
return;
}

ResetStub();
class Reactor : public grpc::experimental::ClientBidiReactor<EchoRequest,
EchoResponse> {
public:
explicit Reactor(grpc::testing::EchoTestService::Stub* stub) {
stub->experimental_async()->UnimplementedBidi(&context_, this);
StartCall();
}
void Await() {
std::unique_lock<std::mutex> l(mu_);
while (!done_) {
done_cv_.wait(l);
}
}

private:
void OnReadInitialMetadataDone(bool ok) override { EXPECT_FALSE(ok); }
void OnDone(const Status& s) override {
EXPECT_EQ(s.error_code(), grpc::StatusCode::UNIMPLEMENTED);
EXPECT_EQ(s.error_message(), "");
std::unique_lock<std::mutex> l(mu_);
done_ = true;
done_cv_.notify_one();
}

ClientContext context_;
std::mutex mu_;
std::condition_variable done_cv_;
bool done_ = false;
} client(stub_.get());

client.Await();
}

TEST_P(ClientCallbackEnd2endTest,
ResponseStreamExtraReactionFlowReadsUntilDone) {
ResetStub();
Expand Down
5 changes: 1 addition & 4 deletions test/cpp/util/grpc_tool_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,7 @@ using grpc::testing::EchoResponse;
"RequestStream\n" \
"ResponseStream\n" \
"BidiStream\n" \
"Unimplemented\n" \
"UnimplementedBidi\n"
"Unimplemented\n"

#define ECHO_TEST_SERVICE_DESCRIPTION \
"filename: src/proto/grpc/testing/echo.proto\n" \
Expand All @@ -89,8 +88,6 @@ using grpc::testing::EchoResponse;
"grpc.testing.EchoResponse) {}\n" \
" rpc Unimplemented(grpc.testing.EchoRequest) returns " \
"(grpc.testing.EchoResponse) {}\n" \
" rpc UnimplementedBidi(stream grpc.testing.EchoRequest) returns (stream " \
"grpc.testing.EchoResponse) {}\n" \
"}\n" \
"\n"

Expand Down

0 comments on commit 05c3b30

Please sign in to comment.