chore: call peelAndUnwrap when exception thrown

line · Apr 17, 2024 · d4929fe · d4929fe
1 parent 5c56c80
commit d4929fe
Show file tree

Hide file tree

Showing 7 changed files with 45 additions and 44 deletions.
diff --git a/grpc/src/main/java/com/linecorp/armeria/common/grpc/DefaultGrpcExceptionHandlerFunction.java b/grpc/src/main/java/com/linecorp/armeria/common/grpc/DefaultGrpcExceptionHandlerFunction.java
@@ -16,8 +16,6 @@
 
 package com.linecorp.armeria.common.grpc;
 
-import static java.util.Objects.requireNonNull;
-
 import java.io.IOException;
 import java.nio.channels.ClosedChannelException;
 
@@ -29,11 +27,7 @@
 import com.linecorp.armeria.common.ContentTooLargeException;
 import com.linecorp.armeria.common.RequestContext;
 import com.linecorp.armeria.common.TimeoutException;
-import com.linecorp.armeria.common.annotation.Nullable;
-import com.linecorp.armeria.common.grpc.protocol.ArmeriaStatusException;
 import com.linecorp.armeria.common.stream.ClosedStreamException;
-import com.linecorp.armeria.common.util.Exceptions;
-import com.linecorp.armeria.internal.common.grpc.StatusExceptionConverter;
 import com.linecorp.armeria.server.RequestTimeoutException;
 
 import io.grpc.Metadata;
@@ -47,18 +41,13 @@ final class DefaultGrpcExceptionHandlerFunction implements GrpcExceptionHandlerF
 
     @Override
     public Status apply(RequestContext ctx, Throwable cause, Metadata metadata) {
-        return fromThrowable(cause);
+        return statusFromThrowable(cause);
     }
 
     /**
      * Converts the {@link Throwable} to a {@link Status}, taking into account exceptions specific to Armeria as
      * well and the protocol package.
      */
-    static Status fromThrowable(Throwable t) {
-        t = peelAndUnwrap(requireNonNull(t, "t"));
-        return statusFromThrowable(t);
-    }
-
     private static Status statusFromThrowable(Throwable t) {
         final Status s = Status.fromThrowable(t);
         if (s.getCode() != Code.UNKNOWN) {
@@ -98,18 +87,5 @@ private static Status statusFromThrowable(Throwable t) {
         return s;
     }
 
-    private static Throwable peelAndUnwrap(Throwable t) {
-        t = Exceptions.peel(t);
-        Throwable cause = t;
-        while (cause != null) {
-            if (cause instanceof ArmeriaStatusException) {
-                t = StatusExceptionConverter.toGrpc((ArmeriaStatusException) cause);
-                break;
-            }
-            cause = cause.getCause();
-        }
-        return t;
-    }
-
     private DefaultGrpcExceptionHandlerFunction() {}
 }
diff --git a/grpc/src/main/java/com/linecorp/armeria/internal/client/grpc/ArmeriaClientCall.java b/grpc/src/main/java/com/linecorp/armeria/internal/client/grpc/ArmeriaClientCall.java
@@ -17,6 +17,7 @@
 
 import static com.linecorp.armeria.internal.client.ClientUtil.initContextAndExecuteWithFallback;
 import static com.linecorp.armeria.internal.client.grpc.protocol.InternalGrpcWebUtil.messageBuf;
+import static com.linecorp.armeria.internal.common.grpc.GrpcStatus.peelAndUnwrap;
 import static java.util.Objects.requireNonNull;
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
 
@@ -247,7 +248,8 @@ public void start(Listener<O> responseListener, Metadata metadata) {
         prepareHeaders(compressor, metadata, remainingNanos);
 
         final BiFunction<ClientRequestContext, Throwable, HttpResponse> errorResponseFactory =
-                (unused, cause) -> HttpResponse.ofFailure(exceptionHandler.apply(ctx, cause, metadata)
+                (unused, cause) -> HttpResponse.ofFailure(exceptionHandler.apply(ctx, peelAndUnwrap(cause),
+                                                                                 metadata)
                                                                           .withDescription(cause.getMessage())
                                                                           .asRuntimeException());
         final HttpResponse res = initContextAndExecuteWithFallback(
@@ -453,7 +455,7 @@ public void onNext(DeframedMessage message) {
             });
         } catch (Throwable t) {
             final Metadata metadata = new Metadata();
-            close(exceptionHandler.apply(ctx, t, metadata), metadata);
+            close(exceptionHandler.apply(ctx, peelAndUnwrap(t), metadata), metadata);
         }
     }
 
@@ -510,7 +512,7 @@ private void prepareHeaders(Compressor compressor, Metadata metadata, long remai
 
     private void closeWhenListenerThrows(Throwable t) {
         final Metadata metadata = new Metadata();
-        closeWhenEos(exceptionHandler.apply(ctx, t, metadata), metadata);
+        closeWhenEos(exceptionHandler.apply(ctx, peelAndUnwrap(t), metadata), metadata);
     }
 
     private void closeWhenEos(Status status, Metadata metadata) {

diff --git a/grpc/src/main/java/com/linecorp/armeria/internal/common/grpc/GrpcStatus.java b/grpc/src/main/java/com/linecorp/armeria/internal/common/grpc/GrpcStatus.java
@@ -31,6 +31,8 @@
 
 package com.linecorp.armeria.internal.common.grpc;
 
+import static java.util.Objects.requireNonNull;
+
 import java.net.HttpURLConnection;
 import java.util.Base64;
 
@@ -45,10 +47,12 @@
 import com.linecorp.armeria.common.grpc.StackTraceElementProto;
 import com.linecorp.armeria.common.grpc.StatusCauseException;
 import com.linecorp.armeria.common.grpc.ThrowableProto;
+import com.linecorp.armeria.common.grpc.protocol.ArmeriaStatusException;
 import com.linecorp.armeria.common.grpc.protocol.DeframedMessage;
 import com.linecorp.armeria.common.grpc.protocol.GrpcHeaderNames;
 import com.linecorp.armeria.common.grpc.protocol.StatusMessageEscaper;
 import com.linecorp.armeria.common.stream.StreamMessage;
+import com.linecorp.armeria.common.util.Exceptions;
 
 import io.grpc.Metadata;
 import io.grpc.Status;
@@ -61,6 +65,20 @@ public final class GrpcStatus {
 
     private static final Logger logger = LoggerFactory.getLogger(GrpcStatus.class);
 
+    public static Throwable peelAndUnwrap(Throwable t) {
+        requireNonNull(t, "t");
+        t = Exceptions.peel(t);
+        Throwable cause = t;
+        while (cause != null) {
+            if (cause instanceof ArmeriaStatusException) {
+                t = StatusExceptionConverter.toGrpc((ArmeriaStatusException) cause);
+                break;
+            }
+            cause = cause.getCause();
+        }
+        return t;
+    }
+
     /**
      * Maps gRPC {@link Status} to {@link HttpStatus}. If there is no matched rule for the specified
      * {@link Status}, the mapping rules defined in upstream Google APIs

diff --git a/grpc/src/main/java/com/linecorp/armeria/internal/common/grpc/HttpStreamDeframer.java b/grpc/src/main/java/com/linecorp/armeria/internal/common/grpc/HttpStreamDeframer.java
@@ -17,6 +17,7 @@
 package com.linecorp.armeria.internal.common.grpc;
 
 import static com.google.common.base.Preconditions.checkState;
+import static com.linecorp.armeria.internal.common.grpc.GrpcStatus.peelAndUnwrap;
 import static java.util.Objects.requireNonNull;
 
 import com.linecorp.armeria.common.HttpHeaderNames;
@@ -120,8 +121,8 @@ public void processHeaders(HttpHeaders headers, StreamDecoderOutput<DeframedMess
                 decompressor(ForwardingDecompressor.forGrpc(decompressor));
             } catch (Throwable t) {
                 final Metadata metadata = new Metadata();
-                transportStatusListener.transportReportStatus(exceptionHandler.apply(ctx, t, metadata),
-                                                              metadata);
+                transportStatusListener.transportReportStatus(
+                        exceptionHandler.apply(ctx, peelAndUnwrap(t), metadata), metadata);
                 return;
             }
         }
@@ -148,7 +149,8 @@ public void processTrailers(HttpHeaders headers, StreamDecoderOutput<DeframedMes
     @Override
     public void processOnError(Throwable cause) {
         final Metadata metadata = new Metadata();
-        transportStatusListener.transportReportStatus(exceptionHandler.apply(ctx, cause, metadata), metadata);
+        transportStatusListener.transportReportStatus(
+                exceptionHandler.apply(ctx, peelAndUnwrap(cause), metadata), metadata);
     }
 
     @Override

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
@@ -18,6 +18,7 @@
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkState;
+import static com.linecorp.armeria.internal.common.grpc.GrpcStatus.peelAndUnwrap;
 import static com.linecorp.armeria.internal.common.grpc.protocol.GrpcTrailersUtil.serializeTrailersAsMessage;
 import static java.util.Objects.requireNonNull;
 
@@ -212,16 +213,17 @@ public final void close(Throwable exception) {
     public final void close(Throwable exception, boolean cancelled) {
         exception = Exceptions.peel(exception);
         final Metadata metadata = generateMetadataFromThrowable(exception);
-        final Status status = exceptionHandler.apply(ctx, exception, metadata);
+        final Status status = exceptionHandler.apply(ctx, peelAndUnwrap(exception), metadata);
         close(new ServerStatusAndMetadata(status, metadata, false, cancelled), exception);
     }
 
     @Override
     public final void close(Status status, Metadata metadata) {
+        final Throwable t = status.getCause();
         final ServerStatusAndMetadata statusAndMetadata =
-                new ServerStatusAndMetadata(
-                        exceptionHandler.apply(ctx, status.getCause(), metadata),
-                        metadata, false);
+                t == null ? new ServerStatusAndMetadata(status, metadata, false)
+                          : new ServerStatusAndMetadata(exceptionHandler.apply(ctx, peelAndUnwrap(t), metadata),
+                                                        metadata, false);
         close(statusAndMetadata);
     }
 

diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/FramedGrpcService.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/FramedGrpcService.java
@@ -19,6 +19,7 @@
 import static com.google.common.base.MoreObjects.firstNonNull;
 import static com.google.common.collect.ImmutableMap.toImmutableMap;
 import static com.linecorp.armeria.internal.common.grpc.GrpcExchangeTypeUtil.toExchangeType;
+import static com.linecorp.armeria.internal.common.grpc.GrpcStatus.peelAndUnwrap;
 import static java.util.Objects.requireNonNull;
 
 import java.time.Duration;
@@ -238,7 +239,7 @@ protected HttpResponse doPost(ServiceRequestContext ctx, HttpRequest req) throws
                     return HttpResponse.of(
                             (ResponseHeaders) AbstractServerCall.statusToTrailers(
                                     ctx, defaultHeaders.get(serializationFormat).toBuilder(),
-                                    exceptionHandler.apply(ctx, e, metadata), metadata));
+                                    exceptionHandler.apply(ctx, peelAndUnwrap(e), metadata), metadata));
                 }
             } else {
                 if (Boolean.TRUE.equals(ctx.attr(AbstractUnframedGrpcService.IS_UNFRAMED_GRPC))) {

diff --git a/...c/test/java/com/linecorp/armeria/common/grpc/DefaultGrpcExceptionHandlerFunctionTest.java b/...c/test/java/com/linecorp/armeria/common/grpc/DefaultGrpcExceptionHandlerFunctionTest.java
@@ -31,17 +31,17 @@ class DefaultGrpcExceptionHandlerFunctionTest {
 
     @Test
     void failFastExceptionToUnavailableCode() {
-        assertThat(DefaultGrpcExceptionHandlerFunction
-                           .fromThrowable(new FailFastException(CircuitBreaker.ofDefaultName()))
-                           .getCode())
-                .isEqualTo(Status.Code.UNAVAILABLE);
+        assertThat(GrpcExceptionHandlerFunction
+                           .ofDefault()
+                           .apply(null, new FailFastException(CircuitBreaker.ofDefaultName()), null)
+                           .getCode()).isEqualTo(Status.Code.UNAVAILABLE);
     }
 
     @Test
     void invalidProtocolBufferExceptionToInvalidArgumentCode() {
-        assertThat(DefaultGrpcExceptionHandlerFunction
-                           .fromThrowable(new InvalidProtocolBufferException("Failed to parse message"))
-                           .getCode())
-                .isEqualTo(Status.Code.INVALID_ARGUMENT);
+        assertThat(GrpcExceptionHandlerFunction
+                           .ofDefault()
+                           .apply(null, new InvalidProtocolBufferException("Failed to parse message"), null)
+                           .getCode()).isEqualTo(Status.Code.INVALID_ARGUMENT);
     }
 }