From bc089b0fbe1ff25dfa099e1ced9acec945f74aec Mon Sep 17 00:00:00 2001 From: Jihun Cho Date: Mon, 14 Oct 2019 10:15:51 -0700 Subject: [PATCH 1/3] protobuf: StatusProto#fromStatusAndTrailers fall-back use status --- .../java/io/grpc/protobuf/StatusProto.java | 12 +++++++++- .../io/grpc/protobuf/StatusProtoTest.java | 23 +++++++++++-------- 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/protobuf/src/main/java/io/grpc/protobuf/StatusProto.java b/protobuf/src/main/java/io/grpc/protobuf/StatusProto.java index 2648138b93b..7018b245c56 100644 --- a/protobuf/src/main/java/io/grpc/protobuf/StatusProto.java +++ b/protobuf/src/main/java/io/grpc/protobuf/StatusProto.java @@ -148,7 +148,8 @@ public static com.google.rpc.Status fromThrowable(Throwable t) { /** * Extracts the google.rpc.Status from trailers, and makes sure they match the gRPC - * {@code status}. + * {@code status}. If the trailers doesn't contain status entry, it uses {@code status} param to + * generate status. * * @return the embedded google.rpc.Status or {@code null} if it is not present. * @since 1.11.0 @@ -164,6 +165,15 @@ public static com.google.rpc.Status fromStatusAndTrailers(Status status, Metadat return statusProto; } } + // fall-back to status, this is useful if the error is local. e.g. Server is unavailable. + if (status != null) { + com.google.rpc.Status.Builder statusBuilder = com.google.rpc.Status.newBuilder() + .setCode(status.getCode().value()); + if (status.getDescription() != null) { + statusBuilder.setMessage(status.getDescription()); + } + return statusBuilder.build(); + } return null; } } diff --git a/protobuf/src/test/java/io/grpc/protobuf/StatusProtoTest.java b/protobuf/src/test/java/io/grpc/protobuf/StatusProtoTest.java index f981236523a..cf9c2c564ab 100644 --- a/protobuf/src/test/java/io/grpc/protobuf/StatusProtoTest.java +++ b/protobuf/src/test/java/io/grpc/protobuf/StatusProtoTest.java @@ -126,20 +126,25 @@ public void toStatusException_shouldThrowIfStatusCodeInvalid() throws Exception } @Test - public void fromThrowable_shouldReturnNullIfTrailersAreNull() { - Status status = Status.fromCodeValue(0); + public void fromThrowable_runtimeException_shouldReturnDerivedStatusIfTrailersAreNull() { + Status status = Status.UNAVAILABLE.withDescription("not available"); - assertNull(StatusProto.fromThrowable(status.asRuntimeException())); - assertNull(StatusProto.fromThrowable(status.asException())); + com.google.rpc.Status statusFromThrowable = + StatusProto.fromThrowable(status.asRuntimeException()); + + assertEquals(statusFromThrowable.getCode(), status.getCode().value()); + assertEquals(statusFromThrowable.getMessage(), status.getDescription()); } @Test - public void fromThrowable_shouldReturnNullIfStatusDetailsKeyIsMissing() { - Status status = Status.fromCodeValue(0); - Metadata emptyMetadata = new Metadata(); + public void fromThrowable_exception_shouldReturnDerivedStatusIfTrailersAreNull() { + Status status = Status.UNAVAILABLE.withDescription("not available"); + + com.google.rpc.Status statusFromThrowable = + StatusProto.fromThrowable(status.asException()); - assertNull(StatusProto.fromThrowable(status.asRuntimeException(emptyMetadata))); - assertNull(StatusProto.fromThrowable(status.asException(emptyMetadata))); + assertEquals(statusFromThrowable.getCode(), status.getCode().value()); + assertEquals(statusFromThrowable.getMessage(), status.getDescription()); } @Test From b339684b7862844e74c5b26cb07d01a8f1189c4e Mon Sep 17 00:00:00 2001 From: Jihun Cho Date: Mon, 14 Oct 2019 11:41:08 -0700 Subject: [PATCH 2/3] apply review --- protobuf/src/main/java/io/grpc/protobuf/StatusProto.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/protobuf/src/main/java/io/grpc/protobuf/StatusProto.java b/protobuf/src/main/java/io/grpc/protobuf/StatusProto.java index 7018b245c56..bb6e5a969b2 100644 --- a/protobuf/src/main/java/io/grpc/protobuf/StatusProto.java +++ b/protobuf/src/main/java/io/grpc/protobuf/StatusProto.java @@ -147,9 +147,9 @@ public static com.google.rpc.Status fromThrowable(Throwable t) { } /** - * Extracts the google.rpc.Status from trailers, and makes sure they match the gRPC - * {@code status}. If the trailers doesn't contain status entry, it uses {@code status} param to - * generate status. + * Extracts the {@code google.rpc.Status} from trailers, and makes sure they match the gRPC + * {@code status}. If the trailers do not contain a {@code google.rpc.Status}, it uses + * {@code status} param to generate a {@code google.rpc.Status}. * * @return the embedded google.rpc.Status or {@code null} if it is not present. * @since 1.11.0 From dd356ea94cb456b1b7c152b8aefd324d3644c17f Mon Sep 17 00:00:00 2001 From: Jihun Cho Date: Mon, 14 Oct 2019 13:02:52 -0700 Subject: [PATCH 3/3] statusproto: status now cannot be null --- .../java/io/grpc/protobuf/StatusProto.java | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/protobuf/src/main/java/io/grpc/protobuf/StatusProto.java b/protobuf/src/main/java/io/grpc/protobuf/StatusProto.java index bb6e5a969b2..988e1938af0 100644 --- a/protobuf/src/main/java/io/grpc/protobuf/StatusProto.java +++ b/protobuf/src/main/java/io/grpc/protobuf/StatusProto.java @@ -151,11 +151,12 @@ public static com.google.rpc.Status fromThrowable(Throwable t) { * {@code status}. If the trailers do not contain a {@code google.rpc.Status}, it uses * {@code status} param to generate a {@code google.rpc.Status}. * - * @return the embedded google.rpc.Status or {@code null} if it is not present. + * @return the embedded google.rpc.Status * @since 1.11.0 */ - @Nullable - public static com.google.rpc.Status fromStatusAndTrailers(Status status, Metadata trailers) { + public static com.google.rpc.Status fromStatusAndTrailers( + Status status, @Nullable Metadata trailers) { + checkNotNull(status, "status"); if (trailers != null) { com.google.rpc.Status statusProto = trailers.get(STATUS_DETAILS_KEY); if (statusProto != null) { @@ -166,14 +167,11 @@ public static com.google.rpc.Status fromStatusAndTrailers(Status status, Metadat } } // fall-back to status, this is useful if the error is local. e.g. Server is unavailable. - if (status != null) { - com.google.rpc.Status.Builder statusBuilder = com.google.rpc.Status.newBuilder() - .setCode(status.getCode().value()); - if (status.getDescription() != null) { - statusBuilder.setMessage(status.getDescription()); - } - return statusBuilder.build(); + com.google.rpc.Status.Builder statusBuilder = com.google.rpc.Status.newBuilder() + .setCode(status.getCode().value()); + if (status.getDescription() != null) { + statusBuilder.setMessage(status.getDescription()); } - return null; + return statusBuilder.build(); } }