Skip to content

Commit 72ebfe1

Browse files
minwooxminwoox
authored
Fix handling of exceptions in GrpcExceptionHandlerFunction. (#5796)
Motivation: This PR addresses two issues with the `GrpcExceptionHandlerFunction`: - Peel the exception when calling `apply(RequestContext ctx, Status status, Throwable cause, Metadata metadata)`. - Convert the cause to a `Status` via `Status.fromThrowable(cause)` before calling the `apply()` method. The reasoning behind the second change is to resolve the behavior discrepancy between a `StatusRuntimeException` being thrown and a `StatusRuntimeException` being handled with `onError`. When the exception is handled with `onError`, it is converted to a `Status` by the gRPC upstream, and `status.getCause()` is specified when calling the `apply()` method. However, when a `StatusRuntimeException` is thrown, the `StatusRuntimeException` is passed as is. Modifications: - Fixed to pass the peeled exception in `UnwrappingGrpcExceptionHandlerFunction`. - Converted the cause to a `Status` using `Status.fromThrowable(cause)` before calling the `apply()` method. Result: - The peeled exception is passed for `GrpcExceptionHandlerFunction.apply(RequestContext ctx, Status status, Throwable cause, Metadata metadata)` method.
1 parent 608d61c commit 72ebfe1

23 files changed

Lines changed: 210 additions & 95 deletions

‎examples/tutorials/grpc/src/main/java/example/armeria/server/blog/grpc/GrpcExceptionHandler.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ class GrpcExceptionHandler implements GrpcExceptionHandlerFunction {
1111

1212
@Nullable
1313
@Override
14-
public Status apply(RequestContext ctx, @Nullable Status status, Throwable cause, Metadata metadata) {
14+
public Status apply(RequestContext ctx, Status status, Throwable cause, Metadata metadata) {
1515
if (cause instanceof IllegalArgumentException) {
1616
return Status.INVALID_ARGUMENT.withCause(cause);
1717
}

‎grpc-kotlin/src/test/kotlin/com/linecorp/armeria/server/grpc/kotlin/CoroutineServerInterceptorTest.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ internal class CoroutineServerInterceptorTest {
230230
val exceptionHandler =
231231
GrpcExceptionHandlerFunction {
232232
_: RequestContext,
233-
_: Status?,
233+
_: Status,
234234
throwable: Throwable,
235235
_: Metadata,
236236
->

‎grpc/src/main/java/com/linecorp/armeria/client/grpc/GrpcClientBuilder.java‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,6 @@
7575
import com.linecorp.armeria.common.grpc.GrpcSerializationFormats;
7676
import com.linecorp.armeria.common.grpc.protocol.ArmeriaMessageDeframer;
7777
import com.linecorp.armeria.common.grpc.protocol.ArmeriaMessageFramer;
78-
import com.linecorp.armeria.internal.common.grpc.UnwrappingGrpcExceptionHandleFunction;
7978
import com.linecorp.armeria.unsafe.grpc.GrpcUnsafeBufferUtil;
8079

8180
import io.grpc.CallCredentials;
@@ -419,8 +418,7 @@ public <T> T build(Class<T> clientType) {
419418
option(INTERCEPTORS.newValue(clientInterceptors));
420419
}
421420
if (exceptionHandler != null) {
422-
option(EXCEPTION_HANDLER.newValue(new UnwrappingGrpcExceptionHandleFunction(exceptionHandler.orElse(
423-
GrpcExceptionHandlerFunction.of()))));
421+
option(EXCEPTION_HANDLER.newValue(exceptionHandler.orElse(GrpcExceptionHandlerFunction.of())));
424422
}
425423

426424
final Object client;

‎grpc/src/main/java/com/linecorp/armeria/client/grpc/GrpcClientOptions.java‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@
3737
import com.linecorp.armeria.common.grpc.protocol.ArmeriaMessageFramer;
3838
import com.linecorp.armeria.internal.client.grpc.NullCallCredentials;
3939
import com.linecorp.armeria.internal.client.grpc.NullGrpcClientStubFactory;
40-
import com.linecorp.armeria.internal.common.grpc.UnwrappingGrpcExceptionHandleFunction;
4140
import com.linecorp.armeria.unsafe.grpc.GrpcUnsafeBufferUtil;
4241

4342
import io.grpc.CallCredentials;
@@ -174,8 +173,7 @@ public final class GrpcClientOptions {
174173
* to a gRPC {@link Status}.
175174
*/
176175
public static final ClientOption<GrpcExceptionHandlerFunction> EXCEPTION_HANDLER =
177-
ClientOption.define("EXCEPTION_HANDLER", new UnwrappingGrpcExceptionHandleFunction(
178-
GrpcExceptionHandlerFunction.of()));
176+
ClientOption.define("EXCEPTION_HANDLER", GrpcExceptionHandlerFunction.of());
179177

180178
/**
181179
* Sets whether to respect the marshaller specified in gRPC {@link MethodDescriptor}.

‎grpc/src/main/java/com/linecorp/armeria/common/grpc/DefaultGrpcExceptionHandlerFunction.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
import com.linecorp.armeria.common.ContentTooLargeException;
2828
import com.linecorp.armeria.common.RequestContext;
2929
import com.linecorp.armeria.common.TimeoutException;
30-
import com.linecorp.armeria.common.annotation.Nullable;
3130
import com.linecorp.armeria.common.stream.ClosedStreamException;
3231
import com.linecorp.armeria.server.RequestTimeoutException;
3332
import com.linecorp.armeria.server.ServiceRequestContext;
@@ -46,8 +45,8 @@ enum DefaultGrpcExceptionHandlerFunction implements GrpcExceptionHandlerFunction
4645
* well and the protocol package.
4746
*/
4847
@Override
49-
public Status apply(RequestContext ctx, @Nullable Status status, Throwable cause, Metadata metadata) {
50-
if (status != null && status.getCode() != Code.UNKNOWN) {
48+
public Status apply(RequestContext ctx, Status status, Throwable cause, Metadata metadata) {
49+
if (status.getCode() != Code.UNKNOWN) {
5150
return status;
5251
}
5352
final Status s = Status.fromThrowable(cause);

‎grpc/src/main/java/com/linecorp/armeria/common/grpc/GoogleGrpcExceptionHandlerFunction.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ public interface GoogleGrpcExceptionHandlerFunction extends GrpcExceptionHandler
4040

4141
@Nullable
4242
@Override
43-
default Status apply(RequestContext ctx, @Nullable Status status, Throwable throwable, Metadata metadata) {
43+
default Status apply(RequestContext ctx, Status status, Throwable throwable, Metadata metadata) {
4444
return handleException(ctx, throwable, metadata, this::applyStatusProto);
4545
}
4646

‎grpc/src/main/java/com/linecorp/armeria/common/grpc/GrpcExceptionHandlerFunction.java‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,13 +51,11 @@ static GrpcExceptionHandlerFunction of() {
5151
* Maps the specified {@link Throwable} to a gRPC {@link Status} and mutates the specified {@link Metadata}.
5252
* If {@code null} is returned, {@link #of()} will be used to return {@link Status} as the default.
5353
*
54-
* <p>The {@link Status} may also be specified as a parameter if it is created by
55-
* the upstream gRPC framework.
56-
* You can return the {@link Status} or any other {@link Status} as needed. If the exception is raised
57-
* internally in Armeria, no {@link Status} created, so {@code null} will be specified.
54+
* <p>The specified {@link Status} parameter was created via {@link Status#fromThrowable(Throwable)}.
55+
* You can return the {@link Status} or any other {@link Status} as needed.
5856
*/
5957
@Nullable
60-
Status apply(RequestContext ctx, @Nullable Status status, Throwable cause, Metadata metadata);
58+
Status apply(RequestContext ctx, Status status, Throwable cause, Metadata metadata);
6159

6260
/**
6361
* Returns a {@link GrpcExceptionHandlerFunction} that returns the result of this function

‎grpc/src/main/java/com/linecorp/armeria/internal/client/grpc/ArmeriaClientCall.java‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717

1818
import static com.linecorp.armeria.internal.client.ClientUtil.initContextAndExecuteWithFallback;
1919
import static com.linecorp.armeria.internal.client.grpc.protocol.InternalGrpcWebUtil.messageBuf;
20+
import static com.linecorp.armeria.internal.common.grpc.GrpcExceptionHandlerFunctionUtil.fromThrowable;
21+
import static com.linecorp.armeria.internal.common.grpc.GrpcExceptionHandlerFunctionUtil.generateMetadataFromThrowable;
2022
import static java.util.Objects.requireNonNull;
2123
import static java.util.concurrent.TimeUnit.MILLISECONDS;
2224

@@ -248,9 +250,14 @@ public void start(Listener<O> responseListener, Metadata metadata) {
248250
prepareHeaders(compressor, metadata, remainingNanos);
249251

250252
final BiFunction<ClientRequestContext, Throwable, HttpResponse> errorResponseFactory =
251-
(unused, cause) -> HttpResponse.ofFailure(exceptionHandler.apply(ctx, null, cause, metadata)
252-
.withDescription(cause.getMessage())
253-
.asRuntimeException());
253+
(unused, cause) -> {
254+
final Metadata responseMetadata = generateMetadataFromThrowable(cause);
255+
Status status = fromThrowable(ctx, exceptionHandler, cause, responseMetadata);
256+
if (status.getDescription() == null) {
257+
status = status.withDescription(cause.getMessage());
258+
}
259+
return HttpResponse.ofFailure(status.asRuntimeException());
260+
};
254261
final HttpResponse res = initContextAndExecuteWithFallback(
255262
httpClient, ctx, endpointGroup, HttpResponse::of, errorResponseFactory);
256263

@@ -453,8 +460,8 @@ public void onNext(DeframedMessage message) {
453460
}
454461
});
455462
} catch (Throwable t) {
456-
final Metadata metadata = new Metadata();
457-
close(exceptionHandler.apply(ctx, null, t, metadata), metadata);
463+
final Metadata metadata = generateMetadataFromThrowable(t);
464+
close(fromThrowable(ctx, exceptionHandler, t, metadata), metadata);
458465
}
459466
}
460467

@@ -510,8 +517,8 @@ private void prepareHeaders(Compressor compressor, Metadata metadata, long remai
510517
}
511518

512519
private void closeWhenListenerThrows(Throwable t) {
513-
final Metadata metadata = new Metadata();
514-
closeWhenEos(exceptionHandler.apply(ctx, null, t, metadata), metadata);
520+
final Metadata metadata = generateMetadataFromThrowable(t);
521+
closeWhenEos(fromThrowable(ctx, exceptionHandler, t, metadata), metadata);
515522
}
516523

517524
private void closeWhenEos(Status status, Metadata metadata) {

grpc/src/main/java/com/linecorp/armeria/internal/common/grpc/UnwrappingGrpcExceptionHandleFunction.java renamed to grpc/src/main/java/com/linecorp/armeria/internal/common/grpc/GrpcExceptionHandlerFunctionUtil.java

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,37 +13,48 @@
1313
* License for the specific language governing permissions and limitations
1414
* under the License.
1515
*/
16-
1716
package com.linecorp.armeria.internal.common.grpc;
1817

1918
import static java.util.Objects.requireNonNull;
2019

2120
import com.linecorp.armeria.common.RequestContext;
22-
import com.linecorp.armeria.common.annotation.Nullable;
2321
import com.linecorp.armeria.common.grpc.GrpcExceptionHandlerFunction;
2422
import com.linecorp.armeria.common.grpc.protocol.ArmeriaStatusException;
2523
import com.linecorp.armeria.common.util.Exceptions;
2624

2725
import io.grpc.Metadata;
2826
import io.grpc.Status;
2927

30-
public final class UnwrappingGrpcExceptionHandleFunction implements GrpcExceptionHandlerFunction {
31-
private final GrpcExceptionHandlerFunction delegate;
28+
public final class GrpcExceptionHandlerFunctionUtil {
3229

33-
public UnwrappingGrpcExceptionHandleFunction(GrpcExceptionHandlerFunction handlerFunction) {
34-
delegate = handlerFunction;
30+
public static Metadata generateMetadataFromThrowable(Throwable exception) {
31+
final Metadata metadata = Status.trailersFromThrowable(peelAndUnwrap(exception));
32+
return metadata != null ? metadata : new Metadata();
3533
}
3634

37-
@Nullable
38-
@Override
39-
public Status apply(RequestContext ctx, @Nullable Status status, Throwable cause, Metadata metadata) {
40-
final Throwable t = peelAndUnwrap(cause);
41-
return delegate.apply(ctx, status, t, metadata);
35+
public static Status fromThrowable(RequestContext ctx, GrpcExceptionHandlerFunction exceptionHandler,
36+
Throwable t, Metadata metadata) {
37+
final Status status = Status.fromThrowable(peelAndUnwrap(t));
38+
final Throwable cause = status.getCause();
39+
if (cause == null) {
40+
return status;
41+
}
42+
return applyExceptionHandler(ctx, exceptionHandler, status, cause, metadata);
43+
}
44+
45+
public static Status applyExceptionHandler(RequestContext ctx,
46+
GrpcExceptionHandlerFunction exceptionHandler,
47+
Status status, Throwable cause, Metadata metadata) {
48+
final Throwable peeled = peelAndUnwrap(cause);
49+
status = exceptionHandler.apply(ctx, status, peeled, metadata);
50+
assert status != null;
51+
return status;
4252
}
4353

4454
private static Throwable peelAndUnwrap(Throwable t) {
4555
requireNonNull(t, "t");
46-
Throwable cause = Exceptions.peel(t);
56+
t = Exceptions.peel(t);
57+
Throwable cause = t;
4758
while (cause != null) {
4859
if (cause instanceof ArmeriaStatusException) {
4960
return StatusExceptionConverter.toGrpc((ArmeriaStatusException) cause);
@@ -52,4 +63,6 @@ private static Throwable peelAndUnwrap(Throwable t) {
5263
}
5364
return t;
5465
}
66+
67+
private GrpcExceptionHandlerFunctionUtil() {}
5568
}

‎grpc/src/main/java/com/linecorp/armeria/internal/common/grpc/HttpStreamDeframer.java‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
package com.linecorp.armeria.internal.common.grpc;
1818

1919
import static com.google.common.base.Preconditions.checkState;
20+
import static com.linecorp.armeria.internal.common.grpc.GrpcExceptionHandlerFunctionUtil.fromThrowable;
21+
import static com.linecorp.armeria.internal.common.grpc.GrpcExceptionHandlerFunctionUtil.generateMetadataFromThrowable;
2022
import static java.util.Objects.requireNonNull;
2123

2224
import com.linecorp.armeria.common.HttpHeaderNames;
@@ -119,9 +121,9 @@ public void processHeaders(HttpHeaders headers, StreamDecoderOutput<DeframedMess
119121
try {
120122
decompressor(ForwardingDecompressor.forGrpc(decompressor));
121123
} catch (Throwable t) {
122-
final Metadata metadata = new Metadata();
123-
transportStatusListener.transportReportStatus(exceptionHandler.apply(ctx, null, t, metadata),
124-
metadata);
124+
final Metadata metadata = generateMetadataFromThrowable(t);
125+
transportStatusListener.transportReportStatus(
126+
fromThrowable(ctx, exceptionHandler, t, metadata), metadata);
125127
return;
126128
}
127129
}
@@ -147,9 +149,9 @@ public void processTrailers(HttpHeaders headers, StreamDecoderOutput<DeframedMes
147149

148150
@Override
149151
public void processOnError(Throwable cause) {
150-
final Metadata metadata = new Metadata();
152+
final Metadata metadata = generateMetadataFromThrowable(cause);
151153
transportStatusListener.transportReportStatus(
152-
exceptionHandler.apply(ctx, null, cause, metadata), metadata);
154+
fromThrowable(ctx, exceptionHandler, cause, metadata), metadata);
153155
}
154156

155157
@Override

0 commit comments

Comments
 (0)