Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow overriding the default gRPC status to HTTP status rules for an unframed gRPC response #3683

Closed
ikhoon opened this issue Jul 9, 2021 · 5 comments · Fixed by #3948
Closed

Comments

@ikhoon
Copy link
Contributor

ikhoon commented Jul 9, 2021

An unframed gRPC service statically maps gRPC status to HTTP status based on https://github.com/googleapis/googleapis/blob/master/google/rpc/code.proto.
I got a question from Armera slack channel. https://line-armeria.slack.com/archives/C1NGPBUH2/p1625735446194500

How can I map StatusRuntimeException to http status codes?

Unfortunately, the answer is no. We don't provide a way to customize the default mapping rules at the moment.
It would be nice to take a custom mapping function using GrpcServiceBuilder.

@ikhoon
Copy link
Contributor Author

ikhoon commented Nov 21, 2021

We can introduce an interface to let users override the mapping from gRPC status to HTTP status for unframed gRPC requests.

interface UnframedGrpcStatusFunction {
    // The default mapping.
	static of() {
        return (ctx, status, cause) -> GrpcStatus.grpcStatusToHttpStatus(status);
    }

    // If null is returned, fallback to the default mapping.
    // 'cause' could be captured from `RequestLog.responseCause()`
    @Nullable
	HttpStatus apply(ServiceRequestContext ctx, Status status, @Nullable Throwable cause);

    default UnframedGrpcStatusFunction orElse(UnframedGrpcStatusFunction other) {
        return (ctx, status, cause) -> {
            // apply 'other' if the current status function returns null.
        };
    }
}


GrpcService.builder()
           .unframedGrpcStatusMapping(unframedGrpcStatusFunction);

The UnframedGrpcStatusFunction set via GrpcServiceBuilder could be used in UnframedGrpcErrorHandler with a breaking change.
As UnframedGrpcErrorHandler is an unstable API, we can add UnframedGrpcStatusFunction as the 4th parameter.

HttpResponse handle(ServiceRequestContext ctx, Status status, AggregatedHttpResponse response);

@jupiny
Copy link
Contributor

jupiny commented Nov 21, 2021

Thanks for the detailed guide. Let me take this issue 😄

@jupiny
Copy link
Contributor

jupiny commented Nov 21, 2021

As UnframedGrpcErrorHandler is an unstable API,

One question, could you explain about this part? 😭
Currently, unframedGrpcErrorHandler can be set in GrpcServiceBuilder, can't it?

@ikhoon
Copy link
Contributor Author

ikhoon commented Nov 21, 2021

As UnframedGrpcErrorHandler is an unstable API,

One question, could you explain about this part? 😭 Currently, unframedGrpcErrorHandler can be set in GrpcServiceBuilder, can't it?

Yes, it is. Currently, the status mapping is hard-coded.

final HttpStatus httpStatus = GrpcStatus.grpcStatusToHttpStatus(status);

We need to pass UnframedGrpcStatusFunction to UnframedGrpcErrorHandler so that UnframedGrpcErrorHandler uses UnframedGrpcStatusFunction when building HttpResponse.

 public interface UnframedGrpcErrorHandler {
-    HttpResponse handle(ServiceRequestContext ctx, Status status, AggregatedHttpResponse response);
+    HttpResponse handle(ServiceRequestContext ctx, Status status, AggregatedHttpResponse response, UnframedGrpcStatusFunction statusFunction);

@jupiny
Copy link
Contributor

jupiny commented Nov 21, 2021

Ah, sorry for the misunderstanding the meaning of UnstableApi;
I checked a description about UnstableApi.

/**
 * Indicates the API of the target is not mature enough to guarantee the compatibility between releases.
 * Its behavior, signature or even existence might change without a prior notice at any point.
 */

So you said I could add the breaking change simply. Thank you for answer 🙇

minwoox pushed a commit that referenced this issue Dec 28, 2021
…vice (#3948)

Motivation:

- #3683

Modifications:

- Introduce a `UnframedGrpcStatusMappingFunction `
- Add a `UnframedGrpcStatusMappingFunction ` parameter in `UnframedGrpcErrorHandler#of`

Result:

- Closes #3683
- Users can map a gRPC status to an HTTP status that they want through `UnframedGrpcErrorHandler.of(UnframedGrpcStatusMappingFunction unframedGrpcStatusMappingFunction)`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants