Provide a way to send error details with ArmeriaStatusException (li…

…ne#4341) Motivation: When a user implements a gRPC service using `AbstractUnaryGrpcService`, they can throw an `ArmeriaStatusException` to send an error response. However, the user can't send the error details as defined in [status.proto](https://github.com/googleapis/googleapis/blob/3474dc892349674efda09d74b3a574765d996188/google/rpc/status.proto#L46) because `ArmeriaStatusException` doesn't have a details field. Modifications: - Add `byte[] details` to `ArmeriaStatusException` so that a user can add the serialized `details` field; - Update `AbstractUnsafeUnaryGrpcService` so it sends the `details` in an error response; - Update `UnaryGrpcClient` so it reads the details into `ArmeriaStatusException`. Result: - Closes line#4306 . - Users can add custom `details` when throwing an `ArmeriaStatusException`. --------- Co-authored-by: jrhee17 <[email protected]> Co-authored-by: minux <[email protected]> Co-authored-by: Ikhun Um <[email protected]>
munjeongho · Apr 2, 2024 · 900002d · 900002d
1 parent 25dbf13
commit 900002d
Show file tree

Hide file tree

Showing 10 changed files with 100 additions and 14 deletions.
diff --git a/grpc-protocol/src/main/java/com/linecorp/armeria/client/grpc/protocol/UnaryGrpcClient.java b/grpc-protocol/src/main/java/com/linecorp/armeria/client/grpc/protocol/UnaryGrpcClient.java
@@ -17,6 +17,7 @@
 package com.linecorp.armeria.client.grpc.protocol;
 
 import java.nio.charset.StandardCharsets;
+import java.util.Base64;
 import java.util.Set;
 import java.util.concurrent.CompletableFuture;
 
@@ -161,7 +162,14 @@ private static void checkGrpcStatus(@Nullable String grpcStatus, HttpHeaders hea
             if (grpcMessage != null) {
                 grpcMessage = StatusMessageEscaper.unescape(grpcMessage);
             }
-            throw new ArmeriaStatusException(Integer.parseInt(grpcStatus), grpcMessage);
+            final String grpcDetails = headers.get(GrpcHeaderNames.GRPC_STATUS_DETAILS_BIN);
+            final byte[] details;
+            if (grpcDetails != null) {
+                details = Base64.getDecoder().decode(grpcDetails);
+            } else {
+                details = null;
+            }
+            throw new ArmeriaStatusException(Integer.parseInt(grpcStatus), grpcMessage, details);
         }
     }
 

diff --git a/...tocol/src/main/java/com/linecorp/armeria/common/grpc/protocol/ArmeriaStatusException.java b/...tocol/src/main/java/com/linecorp/armeria/common/grpc/protocol/ArmeriaStatusException.java
@@ -29,20 +29,44 @@ public final class ArmeriaStatusException extends RuntimeException {
 
     private final int code;
 
+    @Nullable
+    private final byte[] grpcStatusDetailsBin;
+
     /**
      * Constructs an {@link ArmeriaStatusException} for the given gRPC status code and message.
      */
     public ArmeriaStatusException(int code, @Nullable String message) {
-        super(message);
-        this.code = code;
+        this(code, message, null, null);
+    }
+
+    /**
+     * Constructs an {@link ArmeriaStatusException} for the given gRPC status code, message
+     * and grpcStatusDetailsBin. {@code grpcStatusDetailsBin} may be formatted as
+     * {@code com.google.rpc.Status} to follow the
+     * <a href="https://github.com/grpc/grpc/issues/24007">unofficial specification</a>.
+     */
+    public ArmeriaStatusException(int code, @Nullable String message, @Nullable byte[] grpcStatusDetailsBin) {
+        this(code, message, grpcStatusDetailsBin, null);
     }
 
     /**
      * Constructs an {@link ArmeriaStatusException} for the given gRPC status code, message and cause.
      */
     public ArmeriaStatusException(int code, @Nullable String message, @Nullable Throwable cause) {
+        this(code, message, null, cause);
+    }
+
+    /**
+     * Constructs an {@link ArmeriaStatusException} for the given gRPC status code, message,
+     * grpcStatusDetailsBin and cause. {@code grpcStatusDetailsBin} may be formatted as
+     * {@code com.google.rpc.Status} to follow the
+     * <a href="https://github.com/grpc/grpc/issues/24007">unofficial specification</a>.
+     */
+    public ArmeriaStatusException(int code, @Nullable String message, @Nullable byte[] grpcStatusDetailsBin,
+                                  @Nullable Throwable cause) {
         super(message, cause);
         this.code = code;
+        this.grpcStatusDetailsBin = grpcStatusDetailsBin;
     }
 
     /**
@@ -51,4 +75,12 @@ public ArmeriaStatusException(int code, @Nullable String message, @Nullable Thro
     public int getCode() {
         return code;
     }
+
+    /**
+     * Returns the gRPC details binary for this {@link ArmeriaStatusException}.
+     */
+    @Nullable
+    public byte[] getGrpcStatusDetailsBin() {
+        return grpcStatusDetailsBin;
+    }
 }
diff --git a/...ol/src/main/java/com/linecorp/armeria/internal/common/grpc/protocol/GrpcTrailersUtil.java b/...ol/src/main/java/com/linecorp/armeria/internal/common/grpc/protocol/GrpcTrailersUtil.java
@@ -18,6 +18,7 @@
 
 import static io.netty.util.AsciiString.c2b;
 
+import java.util.Base64;
 import java.util.Map;
 
 import com.linecorp.armeria.common.HttpHeaders;
@@ -43,12 +44,17 @@ public final class GrpcTrailersUtil {
      * as {@code true}.
      */
     public static void addStatusMessageToTrailers(
-            HttpHeadersBuilder trailersBuilder, int code, @Nullable String message) {
+            HttpHeadersBuilder trailersBuilder, int code, @Nullable String message,
+            @Nullable byte[] details) {
         trailersBuilder.endOfStream(true);
         trailersBuilder.add(GrpcHeaderNames.GRPC_STATUS, StringUtil.toString(code));
         if (message != null) {
             trailersBuilder.add(GrpcHeaderNames.GRPC_MESSAGE, StatusMessageEscaper.escape(message));
         }
+        if (details != null && details.length > 0) {
+            final String encodedDetails = Base64.getEncoder().encodeToString(details);
+            trailersBuilder.add(GrpcHeaderNames.GRPC_STATUS_DETAILS_BIN, encodedDetails);
+        }
     }
 
     /**

diff --git a/...c/main/java/com/linecorp/armeria/server/grpc/protocol/AbstractUnsafeUnaryGrpcService.java b/...c/main/java/com/linecorp/armeria/server/grpc/protocol/AbstractUnsafeUnaryGrpcService.java
@@ -133,7 +133,8 @@ protected final HttpResponse doPost(ServiceRequestContext ctx, HttpRequest req)
                     if (cause == null) {
                         try {
                             final HttpHeadersBuilder trailersBuilder = HttpHeaders.builder();
-                            GrpcTrailersUtil.addStatusMessageToTrailers(trailersBuilder, StatusCodes.OK, null);
+                            GrpcTrailersUtil.addStatusMessageToTrailers(trailersBuilder, StatusCodes.OK,
+                                                                        null, null);
                             final HttpHeaders trailers = trailersBuilder.build();
                             GrpcWebTrailers.set(ctx, trailers);
                             final ArmeriaMessageFramer framer = new ArmeriaMessageFramer(
@@ -162,10 +163,11 @@ protected final HttpResponse doPost(ServiceRequestContext ctx, HttpRequest req)
                     if (cause instanceof ArmeriaStatusException) {
                         final ArmeriaStatusException statusException = (ArmeriaStatusException) cause;
                         GrpcTrailersUtil.addStatusMessageToTrailers(
-                                trailersBuilder, statusException.getCode(), statusException.getMessage());
+                                trailersBuilder, statusException.getCode(), statusException.getMessage(),
+                                statusException.getGrpcStatusDetailsBin());
                     } else {
                         GrpcTrailersUtil.addStatusMessageToTrailers(
-                                trailersBuilder, StatusCodes.INTERNAL, cause.getMessage());
+                                trailersBuilder, StatusCodes.INTERNAL, cause.getMessage(), null);
                     }
                     final ResponseHeaders trailers = trailersBuilder.build();
                     GrpcWebTrailers.set(ctx, trailers);

diff --git a/grpc/src/main/java/com/linecorp/armeria/internal/server/grpc/AbstractServerCall.java b/grpc/src/main/java/com/linecorp/armeria/internal/server/grpc/AbstractServerCall.java
@@ -562,7 +562,7 @@ public static HttpHeaders statusToTrailers(
                     .build();
         }
         GrpcTrailersUtil.addStatusMessageToTrailers(
-                trailersBuilder, status.getCode().value(), status.getDescription());
+                trailersBuilder, status.getCode().value(), status.getDescription(), null);
 
         if (ctx.config().verboseResponses() && status.getCause() != null) {
             final ThrowableProto proto = GrpcStatus.serializeThrowable(status.getCause());

diff --git a/grpc/src/test/java/com/linecorp/armeria/client/grpc/GrpcWebTextTest.java b/grpc/src/test/java/com/linecorp/armeria/client/grpc/GrpcWebTextTest.java
@@ -142,7 +142,7 @@ private static void writeEncodedMessageAcrossFrames(
 
         private static void writeTrailers(ServiceRequestContext ctx, HttpResponseWriter streaming) {
             final HttpHeadersBuilder trailersBuilder = HttpHeaders.builder();
-            GrpcTrailersUtil.addStatusMessageToTrailers(trailersBuilder, StatusCodes.OK, null);
+            GrpcTrailersUtil.addStatusMessageToTrailers(trailersBuilder, StatusCodes.OK, null, null);
             final ByteBuf serializedTrailers =
                     GrpcTrailersUtil.serializeTrailersAsMessage(ctx.alloc(), trailersBuilder.build());
             final HttpData httpdataTrailers = HttpData.wrap(

diff --git a/grpc/src/test/java/com/linecorp/armeria/client/grpc/protocol/UnaryGrpcClientTest.java b/grpc/src/test/java/com/linecorp/armeria/client/grpc/protocol/UnaryGrpcClientTest.java
@@ -20,6 +20,7 @@
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
+import java.net.URI;
 import java.nio.charset.StandardCharsets;
 import java.util.concurrent.CompletionException;
 import java.util.stream.Stream;
@@ -81,8 +82,8 @@ private static SimpleRequest buildRequest(String payload) {
                             .build();
     }
 
-    private static String getUri(SerializationFormat serializationFormat) {
-        return String.format("%s+%s", serializationFormat, server.httpUri());
+    private static URI getUri(SerializationFormat serializationFormat) {
+        return server.httpUri(serializationFormat);
     }
 
     @ParameterizedTest

diff --git a/grpc/src/test/java/com/linecorp/armeria/common/grpc/protocol/ArmeriaMessageFramerTest.java b/grpc/src/test/java/com/linecorp/armeria/common/grpc/protocol/ArmeriaMessageFramerTest.java
@@ -209,7 +209,7 @@ void tooLargeCompressed() {
     private static ByteBuf serializedTrailers() {
         final ResponseHeadersBuilder trailersBuilder = ResponseHeaders.builder(200).contentType(
                 GrpcSerializationFormats.PROTO.mediaType());
-        GrpcTrailersUtil.addStatusMessageToTrailers(trailersBuilder, StatusCodes.OK, null);
+        GrpcTrailersUtil.addStatusMessageToTrailers(trailersBuilder, StatusCodes.OK, null, null);
         return serializeTrailersAsMessage(ByteBufAllocator.DEFAULT, trailersBuilder.build());
     }
 }
diff --git a/grpc/src/test/java/com/linecorp/armeria/internal/common/grpc/MetadataUtilTest.java b/grpc/src/test/java/com/linecorp/armeria/internal/common/grpc/MetadataUtilTest.java
@@ -60,7 +60,9 @@ void fillHeadersTest() {
                                .add(HttpHeaderNames.STATUS, HttpStatus.OK.codeAsText())
                                .add(HttpHeaderNames.CONTENT_TYPE, "application/grpc+proto")
                                .add(GrpcHeaderNames.GRPC_STATUS, "3")
-                               .add(GrpcHeaderNames.GRPC_MESSAGE, "test_grpc_message");
+                               .add(GrpcHeaderNames.GRPC_MESSAGE, "test_grpc_message")
+                               .add(GrpcHeaderNames.GRPC_STATUS_DETAILS_BIN,
+                                    Base64.getEncoder().encodeToString("test_grpc_details".getBytes()));
 
         final Metadata metadata = new Metadata();
         // be copied into HttpHeaderBuilder trailers
@@ -81,6 +83,8 @@ void fillHeadersTest() {
         assertThat(trailers.getAll(HttpHeaderNames.CONTENT_TYPE)).containsExactly("application/grpc+proto");
         assertThat(trailers.getAll(GrpcHeaderNames.GRPC_STATUS)).containsExactly("3");
         assertThat(trailers.getAll(GrpcHeaderNames.GRPC_MESSAGE)).containsOnly("test_grpc_message");
+        assertThat(Base64.getDecoder().decode(trailers.get(GrpcHeaderNames.GRPC_STATUS_DETAILS_BIN)))
+                .containsExactly("test_grpc_details".getBytes());
         assertThat(trailers.getAll(GrpcHeaderNames.ARMERIA_GRPC_THROWABLEPROTO_BIN)).isEmpty();
     }
 

diff --git a/...src/test/java/com/linecorp/armeria/server/grpc/protocol/AbstractUnaryGrpcServiceTest.java b/...src/test/java/com/linecorp/armeria/server/grpc/protocol/AbstractUnaryGrpcServiceTest.java
@@ -20,6 +20,7 @@
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 import java.io.UncheckedIOException;
+import java.util.concurrent.CompletionException;
 import java.util.concurrent.CompletionStage;
 import java.util.stream.Stream;
 
@@ -34,8 +35,12 @@
 import com.google.protobuf.ByteString;
 import com.google.protobuf.InvalidProtocolBufferException;
 
+import com.linecorp.armeria.client.ClientRequestContext;
+import com.linecorp.armeria.client.ClientRequestContextCaptor;
+import com.linecorp.armeria.client.Clients;
 import com.linecorp.armeria.client.WebClient;
 import com.linecorp.armeria.client.grpc.GrpcClients;
+import com.linecorp.armeria.client.grpc.protocol.UnaryGrpcClient;
 import com.linecorp.armeria.common.AggregatedHttpResponse;
 import com.linecorp.armeria.common.HttpHeaderNames;
 import com.linecorp.armeria.common.HttpHeaders;
@@ -100,7 +105,9 @@ protected CompletionStage<byte[]> handleMessage(ServiceRequestContext ctx, byte[
                 // For statusExceptionUpstream() and statusExceptionDownstream()
                 assertThat(request).isEqualTo(EXCEPTION_REQUEST_MESSAGE);
                 final Messages.EchoStatus resStatus = request.getResponseStatus();
-                throw new ArmeriaStatusException(resStatus.getCode(), resStatus.getMessage());
+                throw new ArmeriaStatusException(resStatus.getCode(),
+                                                 resStatus.getMessage(),
+                                                 "TestDetails".getBytes());
             } else {
                 // For normalUpstream() and normalDownstream()
                 assertThat(request).isEqualTo(REQUEST_MESSAGE);
@@ -200,6 +207,32 @@ void unsupportedMediaType() {
                 HttpStatus.UNSUPPORTED_MEDIA_TYPE.codeAsText());
     }
 
+    @Test
+    void exceptionWithDetails() throws Exception {
+        final UnaryGrpcClient client = Clients.newClient(
+            server.httpUri(UnaryGrpcSerializationFormats.PROTO),
+                           UnaryGrpcClient.class);
+
+        try (ClientRequestContextCaptor captor = Clients.newContextCaptor()) {
+            assertThatThrownBy(
+                    () -> client.execute("/armeria.grpc.testing.TestService/UnaryCall",
+                                         EXCEPTION_REQUEST_MESSAGE.toByteArray()).join())
+                    .isInstanceOf(CompletionException.class)
+                    .hasMessageContaining("not for your eyes")
+                    .hasCauseInstanceOf(ArmeriaStatusException.class)
+                    .cause()
+                    .satisfies(ex -> {
+                            final ArmeriaStatusException cause = (ArmeriaStatusException) ex;
+                            assertThat(cause.getGrpcStatusDetailsBin()).isNotEmpty();
+                            assertThat(cause.getGrpcStatusDetailsBin()).isEqualTo("TestDetails".getBytes());
+                        });
+            final ClientRequestContext ctx = captor.get();
+            final HttpHeaders trailers = GrpcWebTrailers.get(ctx);
+            assertThat(trailers).isNotNull();
+            assertThat(trailers.getInt(GrpcHeaderNames.GRPC_STATUS)).isEqualTo(StatusCodes.PERMISSION_DENIED);
+        }
+    }
+
     @ParameterizedTest
     @ArgumentsSource(UnaryGrpcSerializationFormatArgumentsProvider.class)
     void invalidPayload(SerializationFormat serializationFormat) throws Exception {