chore: use DefaultGrpcExceptionHandlerFunction as fallback for both G…

…rpcClient and GrpcServer
line · Apr 14, 2024 · 0c804ef · 0c804ef
1 parent 1e0a4d2
commit 0c804ef
Show file tree

Hide file tree

Showing 8 changed files with 146 additions and 23 deletions.
diff --git a/grpc/src/main/java/com/linecorp/armeria/client/grpc/GrpcClientBuilder.java b/grpc/src/main/java/com/linecorp/armeria/client/grpc/GrpcClientBuilder.java
@@ -74,6 +74,7 @@
 import com.linecorp.armeria.common.grpc.GrpcSerializationFormats;
 import com.linecorp.armeria.common.grpc.protocol.ArmeriaMessageDeframer;
 import com.linecorp.armeria.common.grpc.protocol.ArmeriaMessageFramer;
+import com.linecorp.armeria.internal.common.grpc.DefaultGrpcExceptionHandlerFunction;
 import com.linecorp.armeria.unsafe.grpc.GrpcUnsafeBufferUtil;
 
 import io.grpc.CallCredentials;
@@ -393,9 +394,12 @@ public <T> T build(Class<T> clientType) {
         if (!clientInterceptors.isEmpty()) {
             option(INTERCEPTORS.newValue(clientInterceptors));
         }
-        if (exceptionHandler != null) {
-            option(EXCEPTION_HANDLER.newValue(exceptionHandler));
+        if (exceptionHandler == null) {
+            exceptionHandler = DefaultGrpcExceptionHandlerFunction.ofDefault();
+        } else {
+            exceptionHandler = exceptionHandler.orElse(DefaultGrpcExceptionHandlerFunction.ofDefault());
         }
+        option(EXCEPTION_HANDLER.newValue(exceptionHandler));
 
         final Object client;
         final ClientOptions options = buildOptions();

diff --git a/grpc/src/main/java/com/linecorp/armeria/client/grpc/GrpcClientOptions.java b/grpc/src/main/java/com/linecorp/armeria/client/grpc/GrpcClientOptions.java
@@ -173,7 +173,7 @@ public final class GrpcClientOptions {
      * to a gRPC {@link Status}.
      */
     public static final ClientOption<GrpcExceptionHandlerFunction> EXCEPTION_HANDLER =
-            ClientOption.define("EXCEPTION_HANDLER", DefaultGrpcExceptionHandlerFunction.INSTANCE);
+            ClientOption.define("EXCEPTION_HANDLER", DefaultGrpcExceptionHandlerFunction.ofDefault());
 
     private GrpcClientOptions() {}
 }
diff --git a/...n/java/com/linecorp/armeria/internal/common/grpc/DefaultGrpcExceptionHandlerFunction.java b/...n/java/com/linecorp/armeria/internal/common/grpc/DefaultGrpcExceptionHandlerFunction.java
@@ -1,3 +1,19 @@
+/*
+ * Copyright 2024 LINE Corporation
+ *
+ * LINE Corporation licenses this file to you under the Apache License,
+ * version 2.0 (the "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at:
+ *
+ *   https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+
 package com.linecorp.armeria.internal.common.grpc;
 
 import static java.util.Objects.requireNonNull;
@@ -6,9 +22,11 @@
 import java.nio.channels.ClosedChannelException;
 
 import com.google.common.base.Strings;
+import com.google.protobuf.InvalidProtocolBufferException;
 
 import com.linecorp.armeria.client.UnprocessedRequestException;
 import com.linecorp.armeria.client.circuitbreaker.FailFastException;
+import com.linecorp.armeria.client.grpc.GrpcClientBuilder;
 import com.linecorp.armeria.common.ClosedSessionException;
 import com.linecorp.armeria.common.ContentTooLargeException;
 import com.linecorp.armeria.common.RequestContext;
@@ -23,15 +41,40 @@
 import com.linecorp.armeria.common.stream.ClosedStreamException;
 import com.linecorp.armeria.common.util.Exceptions;
 import com.linecorp.armeria.server.RequestTimeoutException;
+import com.linecorp.armeria.server.grpc.GrpcServiceBuilder;
 
 import io.grpc.Metadata;
 import io.grpc.Status;
 import io.grpc.Status.Code;
 import io.netty.handler.codec.http2.Http2Error;
 import io.netty.handler.codec.http2.Http2Exception;
 
-public class DefaultGrpcExceptionHandlerFunction implements GrpcExceptionHandlerFunction {
-    public static final GrpcExceptionHandlerFunction INSTANCE = new DefaultGrpcExceptionHandlerFunction();
+public final class DefaultGrpcExceptionHandlerFunction implements GrpcExceptionHandlerFunction {
+    private static final GrpcExceptionHandlerFunction INSTANCE = new DefaultGrpcExceptionHandlerFunction();
+
+    /**
+     * Returns the default {@link GrpcExceptionHandlerFunction}. This handler is also used as the final
+     * fallback when the handler customized
+     * with either {@link GrpcClientBuilder#exceptionHandler(GrpcExceptionHandlerFunction)}
+     * or {@link GrpcServiceBuilder#exceptionHandler(GrpcExceptionHandlerFunction)} returns {@code null}.
+     * For example, the following handler basically delegates all error handling to the default handler:
+     * <pre>{@code
+     * // For GrpcClient
+     * GrpcClients
+     *   .builder("http://foo.com")
+     *   .exceptionHandler((ctx, cause, metadata) -> null)
+     *   ...
+     *
+     * // For GrpcServer
+     * GrpcService
+     *   .builder()
+     *   .exceptionHandler((ctx, cause, metadata) -> null)
+     *   ...
+     * }</pre>
+     */
+    public static GrpcExceptionHandlerFunction ofDefault() {
+        return DefaultGrpcExceptionHandlerFunction.INSTANCE;
+    }
 
     @Override
     public @Nullable Status apply(RequestContext ctx, Throwable cause, Metadata metadata) {
@@ -95,6 +138,9 @@ private static Status statusFromThrowable(Throwable t) {
         if (t instanceof ClosedStreamException || t instanceof RequestTimeoutException) {
             return Status.CANCELLED.withCause(t);
         }
+        if (t instanceof InvalidProtocolBufferException) {
+            return Status.INVALID_ARGUMENT.withCause(t);
+        }
         if (t instanceof UnprocessedRequestException ||
             t instanceof IOException ||
             t instanceof FailFastException) {
@@ -163,7 +209,6 @@ private static Throwable peelAndUnwrap(Throwable t) {
         return t;
     }
 
-
     /**
      * Fills the information from the {@link Throwable} into a {@link ThrowableProto} for
      * returning to a client.

diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/GrpcServiceBuilder.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/GrpcServiceBuilder.java
@@ -51,6 +51,7 @@
 import com.linecorp.armeria.common.grpc.GrpcStatusFunction;
 import com.linecorp.armeria.common.grpc.protocol.AbstractMessageDeframer;
 import com.linecorp.armeria.common.grpc.protocol.ArmeriaMessageFramer;
+import com.linecorp.armeria.internal.common.grpc.DefaultGrpcExceptionHandlerFunction;
 import com.linecorp.armeria.server.HttpService;
 import com.linecorp.armeria.server.HttpServiceWithRoutes;
 import com.linecorp.armeria.server.Server;
@@ -974,6 +975,11 @@ public GrpcService build() {
             registryBuilder.addService(grpcHealthCheckService.bindService(), null, ImmutableList.of());
         }
 
+        if (exceptionHandler == null) {
+            exceptionHandler = DefaultGrpcExceptionHandlerFunction.ofDefault();
+        } else {
+            exceptionHandler = exceptionHandler.orElse(DefaultGrpcExceptionHandlerFunction.ofDefault());
+        }
         final GrpcExceptionHandlerFunction grpcExceptionHandler;
         if (exceptionMappingsBuilder != null) {
             grpcExceptionHandler = exceptionMappingsBuilder.build();

diff --git a/grpc/src/test/java/com/linecorp/armeria/client/grpc/GrpcClientBuilderTest.java b/grpc/src/test/java/com/linecorp/armeria/client/grpc/GrpcClientBuilderTest.java
@@ -18,21 +18,39 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.util.concurrent.Executors;
 
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
 
 import com.linecorp.armeria.client.ClientBuilderParams;
 import com.linecorp.armeria.client.Clients;
 import com.linecorp.armeria.client.endpoint.EndpointGroup;
+import com.linecorp.armeria.common.ContentTooLargeException;
 import com.linecorp.armeria.common.SerializationFormat;
+import com.linecorp.armeria.common.grpc.GrpcExceptionHandlerFunction;
 import com.linecorp.armeria.common.grpc.GrpcSerializationFormats;
 import com.linecorp.armeria.internal.common.grpc.DefaultGrpcExceptionHandlerFunction;
+import com.linecorp.armeria.internal.common.grpc.TestServiceImpl;
+import com.linecorp.armeria.server.ServerBuilder;
+import com.linecorp.armeria.server.grpc.GrpcService;
+import com.linecorp.armeria.testing.junit5.server.ServerExtension;
 
 import io.grpc.CallOptions;
 import io.grpc.Channel;
 import io.grpc.ClientCall;
 import io.grpc.ClientInterceptor;
 import io.grpc.MethodDescriptor;
+import io.grpc.Status;
+import io.grpc.Status.Code;
+import io.grpc.StatusRuntimeException;
+import testing.grpc.Messages.SimpleRequest;
 import testing.grpc.TestServiceGrpc.TestServiceBlockingStub;
 
 class GrpcClientBuilderTest {
@@ -156,13 +174,49 @@ public <I, O> ClientCall<I, O> interceptCall(MethodDescriptor<I, O> method,
                 .containsExactly(interceptorA, interceptorB);
     }
 
+    @RegisterExtension
+    static ServerExtension server = new ServerExtension() {
+        @Override
+        protected void configure(ServerBuilder sb) {
+            sb.service(
+                    GrpcService.builder()
+                               .addService(new TestServiceImpl(Executors.newSingleThreadScheduledExecutor()))
+                               .build());
+        }
+    };
+
     @Test
     void useDefaultGrpcExceptionHandlerFunction() {
-        final TestServiceBlockingStub client = GrpcClients.builder("http://foo.com").build(
-                TestServiceBlockingStub.class);
+        final TestServiceBlockingStub client = GrpcClients.builder(server.httpUri())
+                                                          .build(TestServiceBlockingStub.class);
 
         final ClientBuilderParams clientParams = Clients.unwrap(client, ClientBuilderParams.class);
         assertThat(clientParams.options().get(GrpcClientOptions.EXCEPTION_HANDLER))
-                .isEqualTo(DefaultGrpcExceptionHandlerFunction.INSTANCE);
+                .isEqualTo(DefaultGrpcExceptionHandlerFunction.ofDefault());
+    }
+
+    @Test
+    void useDefaultGrpcExceptionHandlerFunctionAsFallback() {
+        final GrpcExceptionHandlerFunction mockExceptionHandler = mock(GrpcExceptionHandlerFunction.class);
+        when(mockExceptionHandler.apply(any(), any(), any())).thenReturn(null);
+
+        final GrpcExceptionHandlerFunction exceptionHandler =
+                GrpcExceptionHandlerFunction.builder()
+                                            .on(ContentTooLargeException.class, mockExceptionHandler)
+                                            .build();
+        final TestServiceBlockingStub client = GrpcClients.builder(server.httpUri())
+                                                          .maxResponseLength(1)
+                                                          .exceptionHandler(exceptionHandler)
+                                                          .build(TestServiceBlockingStub.class);
+
+        // Fallback exception handler expected to return RESOURCE_EXHAUSTED for the ContentTooLargeException
+        assertThatThrownBy(() -> client.unaryCall(SimpleRequest.getDefaultInstance()))
+                .isInstanceOf(StatusRuntimeException.class)
+                .extracting(e -> ((StatusRuntimeException) e).getStatus())
+                .extracting(Status::getCode)
+                .isEqualTo(Code.RESOURCE_EXHAUSTED);
+
+        // mockExceptionHandler is supposed to be called once.
+        verify(mockExceptionHandler, times(1)).apply(any(), any(), any());
     }
 }
diff --git a/...va/com/linecorp/armeria/internal/common/grpc/DefaultGrpcExceptionHandlerFunctionTest.java b/...va/com/linecorp/armeria/internal/common/grpc/DefaultGrpcExceptionHandlerFunctionTest.java
@@ -1,9 +1,27 @@
+/*
+ * Copyright 2024 LINE Corporation
+ *
+ * LINE Corporation licenses this file to you under the Apache License,
+ * version 2.0 (the "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at:
+ *
+ *   https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+
 package com.linecorp.armeria.internal.common.grpc;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
 import org.junit.jupiter.api.Test;
 
+import com.google.protobuf.InvalidProtocolBufferException;
+
 import com.linecorp.armeria.client.circuitbreaker.CircuitBreaker;
 import com.linecorp.armeria.client.circuitbreaker.FailFastException;
 
@@ -18,4 +36,12 @@ void failFastExceptionToUnavailableCode() {
                            .getCode())
                 .isEqualTo(Status.Code.UNAVAILABLE);
     }
-}
+
+    @Test
+    void invalidProtocolBufferExceptionToInvalidArgumentCode() {
+        assertThat(DefaultGrpcExceptionHandlerFunction
+                           .fromThrowable(new InvalidProtocolBufferException("Failed to parse message"))
+                           .getCode())
+                .isEqualTo(Status.Code.INVALID_ARGUMENT);
+    }
+}
diff --git a/grpc/src/test/java/com/linecorp/armeria/internal/common/grpc/GrpcStatusTest.java b/grpc/src/test/java/com/linecorp/armeria/internal/common/grpc/GrpcStatusTest.java
@@ -36,10 +36,6 @@
 import org.junit.jupiter.api.Test;
 
 import com.google.api.gax.grpc.GrpcStatusCode;
-import com.google.protobuf.InvalidProtocolBufferException;
-
-import com.linecorp.armeria.client.circuitbreaker.CircuitBreaker;
-import com.linecorp.armeria.client.circuitbreaker.FailFastException;
 
 import io.grpc.Status;
 
@@ -52,11 +48,4 @@ void grpcCodeToHttpStatus() {
                     .isEqualTo(GrpcStatusCode.of(code).getCode().getHttpStatusCode());
         }
     }
-
-    @Test
-    void invalidProtocolBufferExceptionToInvalidArgumentCode() {
-        assertThat(DefaultGrpcExceptionHandlerFunction.fromThrowable(new InvalidProtocolBufferException("Failed to parse message"))
-                             .getCode())
-                .isEqualTo(Status.Code.INVALID_ARGUMENT);
-    }
 }
diff --git a/grpc/src/test/java/com/linecorp/armeria/server/grpc/GrpcExceptionHandlerTest.java b/grpc/src/test/java/com/linecorp/armeria/server/grpc/GrpcExceptionHandlerTest.java
@@ -33,7 +33,6 @@
 import com.linecorp.armeria.common.RequestContext;
 import com.linecorp.armeria.common.annotation.Nullable;
 import com.linecorp.armeria.common.grpc.GrpcExceptionHandlerFunction;
-import com.linecorp.armeria.internal.common.grpc.DefaultGrpcExceptionHandlerFunction;
 import com.linecorp.armeria.server.ServerBuilder;
 import com.linecorp.armeria.server.logging.LoggingService;
 import com.linecorp.armeria.testing.junit5.server.ServerExtension;
@@ -595,7 +594,7 @@ public void unaryCall(SimpleRequest request, StreamObserver<SimpleResponse> resp
         }
     }
 
-    @GrpcExceptionHandler(DefaultGrpcExceptionHandlerFunction.class)
+    // TestServiceIOException has DefaultGRPCExceptionHandlerFunction as fallback exception handler
     private static class TestServiceIOException extends TestServiceImpl {
         @Override
         public void unaryCall(SimpleRequest request, StreamObserver<SimpleResponse> responseObserver) {