From 544f2a5abbee840c5b7a28b41e75b3ff5c0f3b60 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Wed, 21 Nov 2018 14:00:16 -0800 Subject: [PATCH 1/2] Necessary change after #17219 --- include/grpcpp/impl/codegen/client_unary_call.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/include/grpcpp/impl/codegen/client_unary_call.h b/include/grpcpp/impl/codegen/client_unary_call.h index b1c80764f23..18ee5b34bfa 100644 --- a/include/grpcpp/impl/codegen/client_unary_call.h +++ b/include/grpcpp/impl/codegen/client_unary_call.h @@ -75,7 +75,12 @@ class BlockingUnaryCallImpl { "No message returned for unary request"); } } else { - GPR_CODEGEN_ASSERT(!status_.ok()); + // Some of the ops failed. For example, this can happen if deserialization + // of the message fails. gRPC Core guarantees that the op + // GRPC_OP_RECV_STATUS_ON_CLIENT always succeeds, so status would still be + // filled. + // TODO(yashykt): If deserialization fails, but the status received is OK, + // then it might be a good idea to change the status to reflect this. } } Status status() { return status_; } From 8fb11e6d5e2228523f79c3ed8ee1b4d8fb4b7174 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Wed, 21 Nov 2018 14:11:59 -0800 Subject: [PATCH 2/2] Apply the conversion on the status irrespective of whether Pluck returned true --- .../grpcpp/impl/codegen/client_unary_call.h | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/include/grpcpp/impl/codegen/client_unary_call.h b/include/grpcpp/impl/codegen/client_unary_call.h index 18ee5b34bfa..5151839412b 100644 --- a/include/grpcpp/impl/codegen/client_unary_call.h +++ b/include/grpcpp/impl/codegen/client_unary_call.h @@ -69,18 +69,17 @@ class BlockingUnaryCallImpl { ops.ClientSendClose(); ops.ClientRecvStatus(context, &status_); call.PerformOps(&ops); - if (cq.Pluck(&ops)) { - if (!ops.got_message && status_.ok()) { - status_ = Status(StatusCode::UNIMPLEMENTED, - "No message returned for unary request"); - } - } else { - // Some of the ops failed. For example, this can happen if deserialization - // of the message fails. gRPC Core guarantees that the op - // GRPC_OP_RECV_STATUS_ON_CLIENT always succeeds, so status would still be - // filled. - // TODO(yashykt): If deserialization fails, but the status received is OK, - // then it might be a good idea to change the status to reflect this. + cq.Pluck(&ops); + // Some of the ops might fail. If the ops fail in the core layer, status + // would reflect the error. But, if the ops fail in the C++ layer, the + // status would still be the same as the one returned by gRPC Core. This can + // happen if deserialization of the message fails. + // TODO(yashykt): If deserialization fails, but the status received is OK, + // then it might be a good idea to change the status to something better + // than StatusCode::UNIMPLEMENTED to reflect this. + if (!ops.got_message && status_.ok()) { + status_ = Status(StatusCode::UNIMPLEMENTED, + "No message returned for unary request"); } } Status status() { return status_; }