Refactor GrpcStatus to implement GrpcExceptionHandlerFunction #5571

jaeseung-bae · 2024-04-05T14:09:42Z

Motivation:

Closes Make GrpcStatus implement GrpcExceptionHandlerFunction #5550

Modifications:

GrpcStatus implements GrpcExceptionHandlerFunction
Rename GrpcStatus to DefaultGrpcExceptionHandlerFunction

Result:

Closes Make GrpcStatus implement GrpcExceptionHandlerFunction #5550
GrpcClientBuilder uses DefaultGrpcExceptionHandlerFunction as default
GrpcService can use DefaultGrpcExceptionHandlerFunction for its exceptionHandler

…nction

…and client builder

github-actions · 2024-04-05T14:31:38Z

🔍 Build Scan® (commit: `0cef19b`)

Job name	Status	Build Scan®
build-windows-latest-jdk-21	✅	https://ge.armeria.dev/s/2fb7edjy7s4i2
build-self-hosted-unsafe-jdk-8	❌ (failure)	https://ge.armeria.dev/s/lk57p4j4rq2fm
build-self-hosted-unsafe-jdk-21-snapshot-blockhound	✅	https://ge.armeria.dev/s/liv7bvciab3ne
build-self-hosted-unsafe-jdk-17-min-java-17-coverage	✅	https://ge.armeria.dev/s/yyj2e5ehngce4
build-self-hosted-unsafe-jdk-17-min-java-11	✅	https://ge.armeria.dev/s/j2q4afuamf2pg
build-self-hosted-unsafe-jdk-17-leak	✅	https://ge.armeria.dev/s/2gav6xfsissok
build-self-hosted-unsafe-jdk-11	✅	https://ge.armeria.dev/s/lttaufmbodc52
build-macos-12-jdk-21	✅	https://ge.armeria.dev/s/ddb74mss4ctbs

codecov · 2024-04-05T15:02:02Z

Codecov Report

Attention: Patch coverage is 85.71429% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 74.10%. Comparing base (14c5566) to head (866a041).
Report is 5 commits behind head on main.

❗ Current head 866a041 differs from pull request most recent head 0cef19b

Please upload reports for the commit 0cef19b to get more accurate results.

Files	Patch %	Lines
...rmeria/internal/client/grpc/ArmeriaClientCall.java	83.33%	1 Missing ⚠️
...meria/internal/common/grpc/HttpStreamDeframer.java	80.00%	1 Missing ⚠️
...inecorp/armeria/server/grpc/FramedGrpcService.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #5571      +/-   ##
============================================
+ Coverage     74.05%   74.10%   +0.04%     
+ Complexity    21253    21010     -243     
============================================
  Files          1850     1819      -31     
  Lines         78600    77399    -1201     
  Branches      10032     9889     -143     
============================================
- Hits          58209    57355     -854     
+ Misses        15686    15390     -296     
+ Partials       4705     4654      -51

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

ikhoon · 2024-04-08T06:19:05Z

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

+                        DefaultGrpcExceptionHandlerFunction.fromThrowable(exceptionHandler, ctx, cause,
+                                                                          metadata)
+                                                           .withDescription(cause.getMessage())
+                                                           .asRuntimeException());


Should we set DefaultGrpcExceptionHandlerFunction as a fallback exception handler for exceptionHandler when it is set in the GrpcClientBuilder like we do in ServerBuilder?

armeria/core/src/main/java/com/linecorp/armeria/server/ServerBuilder.java

Lines 2166 to 2172 in ad3519f

final ServerErrorHandler errorHandler;

if (this.errorHandler == null) {

errorHandler = ServerErrorHandler.ofDefault();

} else {

// Ensure that ServerErrorHandler never returns null by falling back to the default.

errorHandler = this.errorHandler.orElse(ServerErrorHandler.ofDefault());

}

The rest of code should not access to DefaultGrpcExceptionHandlerFunction and use the composed one.

Okay, I'll modify to use DefaultGrpcExceptionHandlerFunction as fallback exception handler.

Global comments) If DefaultGrpcExceptionHandlerFunction is set as the fallback, should we remove DefaultGrpcExceptionHandlerFunction here and others?

Suggested change

DefaultGrpcExceptionHandlerFunction.fromThrowable(exceptionHandler, ctx, cause,

metadata)

.withDescription(cause.getMessage())

.asRuntimeException());

exceptionHandler.apply(ctx, cause, metadata)

.withDescription(cause.getMessage())

.asRuntimeException());

Applied feedback. Thanks

ikhoon · 2024-04-08T06:22:34Z

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

@@ -424,7 +425,7 @@ public void onNext(DeframedMessage message) {
                          new Metadata());
                } else {
                    GrpcWebTrailers.set(ctx, trailers);
-                    GrpcStatus.reportStatus(trailers, this);
+                    DefaultGrpcExceptionHandlerFunction.reportStatus(trailers, this);


reportStatus is unrelated to GrpcExceptionHandlerFunction. Should we leave it in GrpcStatus and only move code handling exceptions to DefaultGrpcExceptionHandlerFunction?

Thanks for the recommendation. I also think we should separate GrpcStatus and DefaultGrpcExceptionHandlerFunction.

Applied feedback. Thanks

…efaultGrpcExceptionHandlerFunction

…rpcClient and GrpcServer

…eption server interceptor added

ikhoon · 2024-04-17T03:31:54Z

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

-        if (exceptionHandler != null) {
-            option(EXCEPTION_HANDLER.newValue(exceptionHandler));
+        if (exceptionHandler == null) {
+            exceptionHandler = DefaultGrpcExceptionHandlerFunction.ofDefault();


Let's expose DefaultGrpcExceptionHandlerFunction using GrpcExceptionHandlerFunction.ofDefault().

Suggested change

exceptionHandler = DefaultGrpcExceptionHandlerFunction.ofDefault();

exceptionHandler = GrpcExceptionHandlerFunction.ofDefault();

Applied feedback. Thanks

ikhoon · 2024-04-17T03:36:08Z

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

+                        DefaultGrpcExceptionHandlerFunction.fromThrowable(exceptionHandler, ctx, cause,
+                                                                          metadata)
+                                                           .withDescription(cause.getMessage())
+                                                           .asRuntimeException());


Global comments) If DefaultGrpcExceptionHandlerFunction is set as the fallback, should we remove DefaultGrpcExceptionHandlerFunction here and others?

Suggested change

DefaultGrpcExceptionHandlerFunction.fromThrowable(exceptionHandler, ctx, cause,

metadata)

.withDescription(cause.getMessage())

.asRuntimeException());

exceptionHandler.apply(ctx, cause, metadata)

.withDescription(cause.getMessage())

.asRuntimeException());

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

...main/java/com/linecorp/armeria/internal/common/grpc/DefaultGrpcExceptionHandlerFunction.java

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

ikhoon · 2024-04-17T03:47:59Z

.../src/test/java/com/linecorp/armeria/common/grpc/GrpcExceptionHandlerFunctionBuilderTest.java

@@ -89,19 +89,24 @@ void sortExceptionHandler() {
                                 B1Exception.class);

        final GrpcExceptionHandlerFunction exceptionHandler = builder.build();
-        Status status = GrpcStatus.fromThrowable(exceptionHandler, ctx, new A3Exception(), new Metadata());
+        Status status = DefaultGrpcExceptionHandlerFunction.fromThrowable(exceptionHandler, ctx,


We don't have to use DefaultGrpcExceptionHandlerFunction to call the exceptionHandler.
If DefaultGrpcExceptionHandlerFunction is necessary, exceptionHandler.orElse(GrpcExceptionHandlerFunction.ofDefault() can be used.

Applied feedback. Thanks

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

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

trustin · 2024-04-19T06:28:18Z

grpc/src/test/java/com/linecorp/armeria/client/grpc/GrpcClientBuilderTest.java

+
+        final ClientBuilderParams clientParams = Clients.unwrap(client, ClientBuilderParams.class);
+        assertThat(clientParams.options().get(GrpcClientOptions.EXCEPTION_HANDLER))
+                .isEqualTo(GrpcExceptionHandlerFunction.ofDefault());


isSameAs() is probably a better choice in this case because we want it to be an exactly same instance.

This testcase has been removed after some modification.
But there is still test case useDefaultGrpcExceptionHandlerFunctionAsFallback

grpc/src/test/java/com/linecorp/armeria/internal/common/grpc/TestServiceImpl.java

…the most first of exception-handling

ikhoon

Overall looks good. Left minor comments on testing.

.../src/test/java/com/linecorp/armeria/common/grpc/GrpcExceptionHandlerFunctionBuilderTest.java

…ionHandlerFunctionBuilderTest.java Co-authored-by: Ikhun Um <ih.pert@gmail.com>

jaeseung-bae · 2024-04-25T23:52:53Z

Overall looks good. Left minor comments on testing.

Applied feedback, Thanks.
I've checked that test is working fine after change.

ikhoon

Very nice! Thanks, @jaeseung-bae! 🙇‍♂️🚀

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

minwoox

Basically it looks great! Left few comments. 😉

minwoox · 2024-05-07T03:37:29Z

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

-        if (exceptionHandler != null) {
-            option(EXCEPTION_HANDLER.newValue(exceptionHandler));
+        if (exceptionHandler == null) {
+            exceptionHandler = GrpcExceptionHandlerFunction.ofDefault();


I believe that we shouldn't set the property while building an instance:

GrpcExceptionHandlerFunction exceptionHandler; if (this.exceptionHandler == null) { exceptionHandler = GrpcExceptionHandlerFunction.ofDefault(); } else { exceptionHandler = this.exceptionHandler.orElse(GrpcExceptionHandlerFunction.ofDefault()); } exceptionHandler = new UnwrappingGrpcExceptionHandleFunction(exceptionHandler); option(EXCEPTION_HANDLER.newValue(exceptionHandler));

Currently, exceptionHandler is passed through ClientBuilderParams params for instantiating ArmeriaChannel. This look like a current way to pass parameters by ClientBuilderParams and so exceptionHandler should be set. I may introduce exceptionHandler as new constructor parameter but I'm not sure that is right to do. Do you have any idea on this?

armeria/grpc/src/main/java/com/linecorp/armeria/internal/client/grpc/ArmeriaChannel.java

Lines 104 to 127 in 8ab4284

ArmeriaChannel(ClientBuilderParams params,

HttpClient httpClient,

MeterRegistry meterRegistry,

SessionProtocol sessionProtocol,

SerializationFormat serializationFormat,

@Nullable GrpcJsonMarshaller jsonMarshaller,

Map<MethodDescriptor<?, ?>, String> simpleMethodNames) {

this.params = params;

this.httpClient = httpClient;

this.meterRegistry = meterRegistry;

this.sessionProtocol = sessionProtocol;

this.serializationFormat = serializationFormat;

this.jsonMarshaller = jsonMarshaller;

this.simpleMethodNames = simpleMethodNames;

final ClientOptions options = options();

maxOutboundMessageSizeBytes = options.get(GrpcClientOptions.MAX_OUTBOUND_MESSAGE_SIZE_BYTES);

maxInboundMessageSizeBytes = maxInboundMessageSizeBytes(options);

unsafeWrapResponseBuffers = options.get(GrpcClientOptions.UNSAFE_WRAP_RESPONSE_BUFFERS);

useMethodMarshaller = options.get(GrpcClientOptions.USE_METHOD_MARSHALLER);

compressor = options.get(GrpcClientOptions.COMPRESSOR);

decompressorRegistry = options.get(GrpcClientOptions.DECOMPRESSOR_REGISTRY);

credentials0 = options.get(GrpcClientOptions.CALL_CREDENTIALS);

exceptionHandler = options.get(GrpcClientOptions.EXCEPTION_HANDLER);

The exceptionHandler is retrieved from options so we don't need to add exceptionHandler to the constructor parameter:

armeria/grpc/src/main/java/com/linecorp/armeria/internal/client/grpc/ArmeriaChannel.java

Line 127 in 8ab4284

exceptionHandler = options.get(GrpcClientOptions.EXCEPTION_HANDLER);

I also realized that we have to create a new exceptionHandler only when it's set:

public <T> T build(Class<T> clientType) { requireNonNull(clientType, "clientType"); final List<ClientInterceptor> clientInterceptors = interceptors.build(); if (!clientInterceptors.isEmpty()) { option(INTERCEPTORS.newValue(clientInterceptors)); } if (exceptionHandler != null) { option(EXCEPTION_HANDLER.newValue(new UnwrappingGrpcExceptionHandleFunction( exceptionHandler.orElse(GrpcExceptionHandlerFunction.of())))); } .... } public static final ClientOption<GrpcExceptionHandlerFunction> EXCEPTION_HANDLER = ClientOption.define("EXCEPTION_HANDLER", new UnwrappingGrpcExceptionHandleFunction( GrpcExceptionHandlerFunction.of()));

minwoox · 2024-05-07T03:39:54Z

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

@@ -173,8 +172,7 @@ public final class GrpcClientOptions {
     * to a gRPC {@link Status}.
     */
    public static final ClientOption<GrpcExceptionHandlerFunction> EXCEPTION_HANDLER =
-            ClientOption.define("EXCEPTION_HANDLER",
-                                (ctx, cause, metadata) -> GrpcStatus.fromThrowable(cause));
+            ClientOption.define("EXCEPTION_HANDLER", GrpcExceptionHandlerFunction.ofDefault());


We don't use UnwrappingGrpcExceptionHandleFunction because this value is never used. Is that correct?

UnwrappingGrpcExceptionHandleFunction should be the outermost handler so we don't use it here.

Then, probably need to add a comment for that?
GrpcExceptionHandlerFunction.ofDefault() isn't used because the actual exceptionHandler is set while building a gRPC client in GrpcClientBuilder.

Added comment, @minwoox can you check it again? Thanks,

Based on this change, we have to use it now.
Sorry but could you revert it? 😓

Okay, I'll revert it

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

minwoox · 2024-05-07T03:42:41Z

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

+     * Returns the default {@link GrpcExceptionHandlerFunction}.
+     */
+    @UnstableApi
+    static GrpcExceptionHandlerFunction ofDefault() {


Recently, we just use of for the default instance.

Suggested change

static GrpcExceptionHandlerFunction ofDefault() {

static GrpcExceptionHandlerFunction of() {

Applied feedback, Thanks.

...in/java/com/linecorp/armeria/internal/common/grpc/UnwrappingGrpcExceptionHandleFunction.java

minwoox · 2024-05-07T03:43:38Z

...in/java/com/linecorp/armeria/internal/common/grpc/UnwrappingGrpcExceptionHandleFunction.java

+    private final GrpcExceptionHandlerFunction delegate;
+
+    public UnwrappingGrpcExceptionHandleFunction(GrpcExceptionHandlerFunction handlerFunction) {
+        this.delegate = handlerFunction;


nit:

Suggested change

this.delegate = handlerFunction;

delegate = handlerFunction;

Applied feedback, Thanks.

minwoox · 2024-05-07T03:43:56Z

...in/java/com/linecorp/armeria/internal/common/grpc/UnwrappingGrpcExceptionHandleFunction.java

+        return delegate.apply(ctx, t, metadata);
+    }
+
+    private Throwable peelAndUnwrap(Throwable t) {


nit:

Suggested change

private Throwable peelAndUnwrap(Throwable t) {

private static Throwable peelAndUnwrap(Throwable t) {

Applied feedback, Thanks.

grpc/src/main/java/com/linecorp/armeria/internal/server/grpc/AbstractServerCall.java

minwoox · 2024-05-07T03:46:52Z

grpc/src/main/java/com/linecorp/armeria/server/grpc/GrpcServiceBuilder.java

+        if (exceptionHandler == null) {
+            exceptionHandler = GrpcExceptionHandlerFunction.ofDefault();
+        } else {
+            exceptionHandler = exceptionHandler.orElse(GrpcExceptionHandlerFunction.ofDefault());


Is this ditto to #5571 (comment)?
I replied to #5571 (comment)?.

Yes we can probably do:

GrpcExceptionHandlerFunction grpcExceptionHandler; if (exceptionMappingsBuilder != null) { grpcExceptionHandler = exceptionMappingsBuilder.build().orElse(GrpcExceptionHandlerFunction.of()); } else if (exceptionHandler != null) { grpcExceptionHandler = exceptionHandler.orElse(GrpcExceptionHandlerFunction.of());; } else { grpcExceptionHandler = GrpcExceptionHandlerFunction.of(); } grpcExceptionHandler = new UnwrappingGrpcExceptionHandleFunction(grpcExceptionHandler); registryBuilder.setDefaultExceptionHandler(grpcExceptionHandler); if (interceptors != null) { final HandlerRegistry.Builder newRegistryBuilder = new HandlerRegistry.Builder(); final ImmutableList<ServerInterceptor> interceptors = this.interceptors.build(); for (Entry entry : registryBuilder.entries()) { final MethodDescriptor<?, ?> methodDescriptor = entry.method(); final ServerServiceDefinition intercepted = ServerInterceptors.intercept(entry.service(), interceptors); newRegistryBuilder.addService(entry.path(), intercepted, methodDescriptor, entry.type(), entry.additionalDecorators()); } newRegistryBuilder.setDefaultExceptionHandler(grpcExceptionHandler); handlerRegistry = newRegistryBuilder.build(); } else { handlerRegistry = registryBuilder.build(); }

Applied feedback, Thank.
Code gets cleaner :)

minwoox

Basically it looks great! Left a few comments. 😉

trustin

LGTM once @minwoox's comments are addressed. Thanks, @jaeseung-bae!

jrhee17

Looks good once @minwoox 's comments are addressed 👍 👍 👍

minwoox

Just two nits. 😉
Thanks a lot, @jaeseung-bae!

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

...in/java/com/linecorp/armeria/internal/common/grpc/UnwrappingGrpcExceptionHandleFunction.java

…Options.java Co-authored-by: minux <songmw725@gmail.com>

…nwrappingGrpcExceptionHandleFunction.java Co-authored-by: minux <songmw725@gmail.com>

minwoox · 2024-05-20T08:13:19Z

@jaeseung-bae, 👍 👍 👍

) Motivation: - Closes line#5550 Modifications: - `GrpcStatus` implements `GrpcExceptionHandlerFunction` - Rename `GrpcStatus` to `DefaultGrpcExceptionHandlerFunction` Result: - Closes line#5550 - GrpcClientBuilder uses `DefaultGrpcExceptionHandlerFunction` as default - GrpcService can use `DefaultGrpcExceptionHandlerFunction` for its `exceptionHandler`

jaeseung-bae added 2 commits April 5, 2024 21:25

chore: refactor GrpcStatus to use it as DefaultGrpcExceptionHandlerFu…

c33f8ea

…nction

test: can use DefaultGrpcExceptionHandlerFunction for server builder …

30dd216

…and client builder

github-actions bot requested review from ikhoon, jrhee17, minwoox and trustin April 5, 2024 14:09

chore: lint fix

866a041

jaeseung-bae force-pushed the refactor/grpc-status branch from 4152698 to 866a041 Compare April 5, 2024 15:06

jaeseung-bae marked this pull request as ready for review April 6, 2024 10:43

ikhoon added improvement sprint Issues for OSS Sprint participants labels Apr 8, 2024

ikhoon reviewed Apr 8, 2024

View reviewed changes

jaeseung-bae added 2 commits April 12, 2024 09:57

chore: refactor, move exceptionHandling features from GrpcStatus to D…

3a5585f

…efaultGrpcExceptionHandlerFunction

Merge remote-tracking branch 'upstream/main' into refactor/grpc-status

1e0a4d2

jaeseung-bae force-pushed the refactor/grpc-status branch from f838415 to 2613370 Compare April 14, 2024 12:08

chore: use DefaultGrpcExceptionHandlerFunction as fallback for both G…

0c804ef

…rpcClient and GrpcServer

jaeseung-bae force-pushed the refactor/grpc-status branch from 2613370 to 0c804ef Compare April 14, 2024 12:12

jaeseung-bae added 2 commits April 16, 2024 20:48

chore: try to add possibly missing description in case of replace_exc…

a88f3c3

…eption server interceptor added

Merge remote-tracking branch 'upstream/main' into refactor/grpc-status

cccbe7d

jaeseung-bae requested a review from ikhoon April 16, 2024 23:00

ikhoon reviewed Apr 17, 2024

View reviewed changes

chore: apply feedbacks

5c56c80

jaeseung-bae force-pushed the refactor/grpc-status branch from 756e8d0 to 5cec1bf Compare April 17, 2024 13:16

jaeseung-bae added 2 commits April 19, 2024 14:59

chore: call peelAndUnwrap when exception thrown

0d6f75c

Merge remote-tracking branch 'upstream/main' into HEAD

d38f3be

jaeseung-bae force-pushed the refactor/grpc-status branch from 5cec1bf to d38f3be Compare April 19, 2024 06:01

trustin reviewed Apr 19, 2024

View reviewed changes

chore: UnwrappingGrpcExceptionHandlerFunction calls peelAndUnwrap at …

8604cf5

…the most first of exception-handling

jaeseung-bae requested a review from ikhoon April 22, 2024 23:03

ikhoon reviewed Apr 25, 2024

View reviewed changes

ikhoon and others added 5 commits April 25, 2024 16:10

Merge branch 'main' into refactor/grpc-status

894fbba

Update grpc/src/test/java/com/linecorp/armeria/common/grpc/GrpcExcept…

7090d2c

…ionHandlerFunctionBuilderTest.java Co-authored-by: Ikhun Um <ih.pert@gmail.com>

Update grpc/src/test/java/com/linecorp/armeria/common/grpc/GrpcExcept…

a35dc16

…ionHandlerFunctionBuilderTest.java Co-authored-by: Ikhun Um <ih.pert@gmail.com>

Update grpc/src/test/java/com/linecorp/armeria/common/grpc/GrpcExcept…

a9a28ba

…ionHandlerFunctionBuilderTest.java Co-authored-by: Ikhun Um <ih.pert@gmail.com>

Update grpc/src/test/java/com/linecorp/armeria/common/grpc/GrpcExcept…

0704f71

…ionHandlerFunctionBuilderTest.java Co-authored-by: Ikhun Um <ih.pert@gmail.com>

jaeseung-bae requested a review from ikhoon April 25, 2024 23:53

ikhoon approved these changes May 2, 2024

View reviewed changes

grpc/src/main/java/com/linecorp/armeria/internal/common/grpc/HttpStreamDeframer.java Outdated Show resolved Hide resolved

chore: revert redundant wrapping code

e7b9908

minwoox reviewed May 7, 2024

View reviewed changes

trustin approved these changes May 8, 2024

View reviewed changes

jrhee17 approved these changes May 10, 2024

View reviewed changes

jaeseung-bae added 8 commits May 18, 2024 15:26

Merge remote-tracking branch 'upstream/main' into refactor/grpc-status

7244059

chore: make enum for singleton instance

10245db

chore: rename ofDefault to of to meet recent convention

c717229

chore: add comment

81f3d3a

chore: add assertion for newStatus

d1ff8b9

chore: make it final, static

9603904

chore: fix lint

5b6dba4

chore: refactoring for clean code

99d5fbb

minwoox approved these changes May 20, 2024

View reviewed changes

grpc/src/main/java/com/linecorp/armeria/client/grpc/GrpcClientOptions.java Outdated Show resolved Hide resolved

...in/java/com/linecorp/armeria/internal/common/grpc/UnwrappingGrpcExceptionHandleFunction.java Outdated Show resolved Hide resolved

jaeseung-bae and others added 2 commits May 20, 2024 11:03

Update grpc/src/main/java/com/linecorp/armeria/client/grpc/GrpcClient…

81c954d

…Options.java Co-authored-by: minux <songmw725@gmail.com>

Update grpc/src/main/java/com/linecorp/armeria/internal/common/grpc/U…

0cef19b

…nwrappingGrpcExceptionHandleFunction.java Co-authored-by: minux <songmw725@gmail.com>

minwoox merged commit 1e14396 into line:main May 20, 2024
14 of 15 checks passed

minwoox mentioned this pull request May 21, 2024

Apply gRPC exception handler function to cancellations as well #5398

Merged

	final ServerErrorHandler errorHandler;
	if (this.errorHandler == null) {
	errorHandler = ServerErrorHandler.ofDefault();
	} else {
	// Ensure that ServerErrorHandler never returns null by falling back to the default.
	errorHandler = this.errorHandler.orElse(ServerErrorHandler.ofDefault());
	}

	exceptionHandler = DefaultGrpcExceptionHandlerFunction.ofDefault();
	exceptionHandler = GrpcExceptionHandlerFunction.ofDefault();

	ArmeriaChannel(ClientBuilderParams params,
	HttpClient httpClient,
	MeterRegistry meterRegistry,
	SessionProtocol sessionProtocol,
	SerializationFormat serializationFormat,
	@Nullable GrpcJsonMarshaller jsonMarshaller,
	Map<MethodDescriptor<?, ?>, String> simpleMethodNames) {
	this.params = params;
	this.httpClient = httpClient;
	this.meterRegistry = meterRegistry;
	this.sessionProtocol = sessionProtocol;
	this.serializationFormat = serializationFormat;
	this.jsonMarshaller = jsonMarshaller;
	this.simpleMethodNames = simpleMethodNames;

	final ClientOptions options = options();
	maxOutboundMessageSizeBytes = options.get(GrpcClientOptions.MAX_OUTBOUND_MESSAGE_SIZE_BYTES);
	maxInboundMessageSizeBytes = maxInboundMessageSizeBytes(options);
	unsafeWrapResponseBuffers = options.get(GrpcClientOptions.UNSAFE_WRAP_RESPONSE_BUFFERS);
	useMethodMarshaller = options.get(GrpcClientOptions.USE_METHOD_MARSHALLER);
	compressor = options.get(GrpcClientOptions.COMPRESSOR);
	decompressorRegistry = options.get(GrpcClientOptions.DECOMPRESSOR_REGISTRY);
	credentials0 = options.get(GrpcClientOptions.CALL_CREDENTIALS);
	exceptionHandler = options.get(GrpcClientOptions.EXCEPTION_HANDLER);

	static GrpcExceptionHandlerFunction ofDefault() {
	static GrpcExceptionHandlerFunction of() {

	private Throwable peelAndUnwrap(Throwable t) {
	private static Throwable peelAndUnwrap(Throwable t) {

Refactor GrpcStatus to implement GrpcExceptionHandlerFunction #5571

Refactor GrpcStatus to implement GrpcExceptionHandlerFunction #5571

Conversation

jaeseung-bae commented Apr 5, 2024

github-actions bot commented Apr 5, 2024 • edited Loading

🔍 Build Scan® (commit: 0cef19b)

codecov bot commented Apr 5, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaeseung-bae Apr 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ikhoon left a comment

Choose a reason for hiding this comment

jaeseung-bae commented Apr 25, 2024

ikhoon left a comment

Choose a reason for hiding this comment

minwoox left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaeseung-bae May 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaeseung-bae May 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaeseung-bae May 20, 2024 • edited Loading

Choose a reason for hiding this comment

minwoox left a comment • edited Loading

Choose a reason for hiding this comment

trustin left a comment

Choose a reason for hiding this comment

jrhee17 left a comment

Choose a reason for hiding this comment

minwoox left a comment

Choose a reason for hiding this comment

minwoox commented May 20, 2024

github-actions bot commented Apr 5, 2024 •

edited

Loading

🔍 Build Scan® (commit: `0cef19b`)

codecov bot commented Apr 5, 2024 •

edited

Loading

jaeseung-bae Apr 8, 2024 •

edited

Loading

jaeseung-bae May 19, 2024 •

edited

Loading

jaeseung-bae May 19, 2024 •

edited

Loading

jaeseung-bae May 20, 2024 •

edited

Loading

minwoox left a comment •

edited

Loading