From 322333a19740ddd4d9cecafdb64d54a2d86da268 Mon Sep 17 00:00:00 2001 From: Ramasai Tadepalli Date: Sun, 7 Apr 2024 11:28:22 -0400 Subject: [PATCH 1/4] Add `StatusProto.toStatusException` overload to accept `Throwable` --- .../java/io/grpc/protobuf/StatusProto.java | 19 +++++++++++++++++++ .../io/grpc/protobuf/StatusProtoTest.java | 9 +++++++++ 2 files changed, 28 insertions(+) diff --git a/protobuf/src/main/java/io/grpc/protobuf/StatusProto.java b/protobuf/src/main/java/io/grpc/protobuf/StatusProto.java index 988e1938af0..53aac4fd258 100644 --- a/protobuf/src/main/java/io/grpc/protobuf/StatusProto.java +++ b/protobuf/src/main/java/io/grpc/protobuf/StatusProto.java @@ -103,6 +103,25 @@ public static StatusException toStatusException( return toStatus(statusProto).asException(toMetadata(statusProto, metadata)); } + /** + * Convert a {@link com.google.rpc.Status} instance to a {@link StatusException} with additional + * metadata and the root exception thrown. + * + *

The returned {@link StatusException} will wrap a {@link Status} whose code and description + * are set from the code and message in {@code statusProto}. {@code statusProto} will be + * serialized and added to {@code metadata}. {@code metadata} will be set as the metadata of the + * returned {@link StatusException}. The {@link Throwable} is the exception that is set as the + * {@code cause} of the returned {@link StatusException}. + * + * @throws IllegalArgumentException if the value of {@code statusProto.getCode()} is not a valid + * gRPC status code. + * @since 1.3.0 + */ + public static StatusException toStatusException( + com.google.rpc.Status statusProto, Metadata metadata, Throwable cause) { + return toStatus(statusProto).withCause(cause).asException(toMetadata(statusProto, metadata)); + } + private static Status toStatus(com.google.rpc.Status statusProto) { Status status = Status.fromCodeValue(statusProto.getCode()); checkArgument(status.getCode().value() == statusProto.getCode(), "invalid status code"); diff --git a/protobuf/src/test/java/io/grpc/protobuf/StatusProtoTest.java b/protobuf/src/test/java/io/grpc/protobuf/StatusProtoTest.java index cf9c2c564ab..a20a70002f7 100644 --- a/protobuf/src/test/java/io/grpc/protobuf/StatusProtoTest.java +++ b/protobuf/src/test/java/io/grpc/protobuf/StatusProtoTest.java @@ -176,6 +176,15 @@ public void fromThrowable_shouldReturnNullIfNoEmbeddedStatus() { assertNull(StatusProto.fromThrowable(nestedSe)); } + @Test + public void toStatusExceptionWithMetadataAndCause_shouldCaptureCause() { + StatusException se = StatusProto.toStatusException(STATUS_PROTO, new Metadata(), + new RuntimeException("This is a test exception.")); + + assertNotNull(se.getCause()); + assertEquals(se.getCause().getMessage(), "This is a test exception."); + } + private static final Metadata.Key METADATA_KEY = Metadata.Key.of("test-metadata", Metadata.ASCII_STRING_MARSHALLER); private static final String METADATA_VALUE = "test metadata value"; From c8400d582edc858bafd7b42b8e6b1e6c6941c96d Mon Sep 17 00:00:00 2001 From: Ramasai Tadepalli Date: Mon, 15 Apr 2024 23:22:35 -0400 Subject: [PATCH 2/4] Review comments --- protobuf/src/main/java/io/grpc/protobuf/StatusProto.java | 3 ++- .../src/test/java/io/grpc/protobuf/StatusProtoTest.java | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/protobuf/src/main/java/io/grpc/protobuf/StatusProto.java b/protobuf/src/main/java/io/grpc/protobuf/StatusProto.java index 53aac4fd258..c15c191fe07 100644 --- a/protobuf/src/main/java/io/grpc/protobuf/StatusProto.java +++ b/protobuf/src/main/java/io/grpc/protobuf/StatusProto.java @@ -105,7 +105,8 @@ public static StatusException toStatusException( /** * Convert a {@link com.google.rpc.Status} instance to a {@link StatusException} with additional - * metadata and the root exception thrown. + * metadata and the root exception thrown. The exception isn't propagated over the wire, and + * needs to be user provided. * *

The returned {@link StatusException} will wrap a {@link Status} whose code and description * are set from the code and message in {@code statusProto}. {@code statusProto} will be diff --git a/protobuf/src/test/java/io/grpc/protobuf/StatusProtoTest.java b/protobuf/src/test/java/io/grpc/protobuf/StatusProtoTest.java index a20a70002f7..771c80eabde 100644 --- a/protobuf/src/test/java/io/grpc/protobuf/StatusProtoTest.java +++ b/protobuf/src/test/java/io/grpc/protobuf/StatusProtoTest.java @@ -178,11 +178,11 @@ public void fromThrowable_shouldReturnNullIfNoEmbeddedStatus() { @Test public void toStatusExceptionWithMetadataAndCause_shouldCaptureCause() { - StatusException se = StatusProto.toStatusException(STATUS_PROTO, new Metadata(), - new RuntimeException("This is a test exception.")); + RuntimeException exc = new RuntimeException("This is a test exception."); + StatusException se = StatusProto.toStatusException(STATUS_PROTO, new Metadata(), exc); assertNotNull(se.getCause()); - assertEquals(se.getCause().getMessage(), "This is a test exception."); + assertEquals(se.getCause(), exc); } private static final Metadata.Key METADATA_KEY = From 7d7797ece8764b7665fdeb00a9ef69d77487f09b Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Wed, 24 Apr 2024 09:14:56 -0700 Subject: [PATCH 3/4] Remove "needs to be user provided" from doc Unclear what it meant. --- protobuf/src/main/java/io/grpc/protobuf/StatusProto.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/protobuf/src/main/java/io/grpc/protobuf/StatusProto.java b/protobuf/src/main/java/io/grpc/protobuf/StatusProto.java index c15c191fe07..0ebc1e714f6 100644 --- a/protobuf/src/main/java/io/grpc/protobuf/StatusProto.java +++ b/protobuf/src/main/java/io/grpc/protobuf/StatusProto.java @@ -105,8 +105,7 @@ public static StatusException toStatusException( /** * Convert a {@link com.google.rpc.Status} instance to a {@link StatusException} with additional - * metadata and the root exception thrown. The exception isn't propagated over the wire, and - * needs to be user provided. + * metadata and the root exception thrown. The exception isn't propagated over the wire. * *

The returned {@link StatusException} will wrap a {@link Status} whose code and description * are set from the code and message in {@code statusProto}. {@code statusProto} will be From bdc07587390a300f5fe3254f186f02913b6e06f5 Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Wed, 24 Apr 2024 09:15:10 -0700 Subject: [PATCH 4/4] Swap assertEquals() argument order, expected first --- protobuf/src/test/java/io/grpc/protobuf/StatusProtoTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/protobuf/src/test/java/io/grpc/protobuf/StatusProtoTest.java b/protobuf/src/test/java/io/grpc/protobuf/StatusProtoTest.java index 771c80eabde..47c045bf952 100644 --- a/protobuf/src/test/java/io/grpc/protobuf/StatusProtoTest.java +++ b/protobuf/src/test/java/io/grpc/protobuf/StatusProtoTest.java @@ -181,8 +181,7 @@ public void toStatusExceptionWithMetadataAndCause_shouldCaptureCause() { RuntimeException exc = new RuntimeException("This is a test exception."); StatusException se = StatusProto.toStatusException(STATUS_PROTO, new Metadata(), exc); - assertNotNull(se.getCause()); - assertEquals(se.getCause(), exc); + assertEquals(exc, se.getCause()); } private static final Metadata.Key METADATA_KEY =