From 171a58c42ac9d453ce63632c5afc04bb940e0ef9 Mon Sep 17 00:00:00 2001 From: Hu Guang Date: Tue, 26 Apr 2022 21:26:12 +0800 Subject: [PATCH 01/36] add rich error mode --- .../common/grpc/protocol/GrpcHeaderNames.java | 5 + .../server/grpc/UnframedGrpcErrorHandler.java | 84 ++++++++++ .../grpc/UnframedGrpcErrorHandlerUtils.java | 89 +++++++++++ .../grpc/UnframedGrpcErrorHandlerTest.java | 68 +++++++- .../UnframedGrpcErrorHandlerUtilsTest.java | 145 ++++++++++++++++++ 5 files changed, 386 insertions(+), 5 deletions(-) create mode 100644 grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerUtils.java create mode 100644 grpc/src/test/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerUtilsTest.java diff --git a/grpc-protocol/src/main/java/com/linecorp/armeria/common/grpc/protocol/GrpcHeaderNames.java b/grpc-protocol/src/main/java/com/linecorp/armeria/common/grpc/protocol/GrpcHeaderNames.java index bdbc6900bdb..ccd5cc62367 100644 --- a/grpc-protocol/src/main/java/com/linecorp/armeria/common/grpc/protocol/GrpcHeaderNames.java +++ b/grpc-protocol/src/main/java/com/linecorp/armeria/common/grpc/protocol/GrpcHeaderNames.java @@ -52,5 +52,10 @@ public final class GrpcHeaderNames { public static final AsciiString ARMERIA_GRPC_THROWABLEPROTO_BIN = HttpHeaderNames.of("armeria.grpc.ThrowableProto-bin"); + /** + * {@code "grpc-status-details-bin"}. + */ + public static final AsciiString GRPC_STATUS_DETAILS_BIN = HttpHeaderNames.of("grpc-status-details-bin"); + private GrpcHeaderNames() {} } diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java index 5d73d085a53..115b7cd3289 100644 --- a/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java @@ -13,14 +13,25 @@ * License for the specific language governing permissions and limitations * under the License. */ + package com.linecorp.armeria.server.grpc; import static java.util.Objects.requireNonNull; +import java.io.IOException; +import java.util.Map; + +import org.curioswitch.common.protobuf.json.MessageMarshaller; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.common.base.Strings; import com.google.common.collect.ImmutableMap; +import com.google.protobuf.InvalidProtocolBufferException; import com.linecorp.armeria.common.AggregatedHttpResponse; import com.linecorp.armeria.common.HttpData; +import com.linecorp.armeria.common.HttpHeaders; import com.linecorp.armeria.common.HttpResponse; import com.linecorp.armeria.common.HttpStatus; import com.linecorp.armeria.common.MediaType; @@ -170,6 +181,79 @@ static UnframedGrpcErrorHandler ofPlainText(UnframedGrpcStatusMappingFunction st }; } + /** + * Returns a rich json response. + */ + static UnframedGrpcErrorHandler ofRichJson() { + return ofRichJson(UnframedGrpcStatusMappingFunction.of()); + } + + /** + * Returns a rich json response. + * + * @param statusMappingFunction The function which maps the {@link Throwable} or gRPC {@link Status} code + * to an {@link HttpStatus} code. + */ + static UnframedGrpcErrorHandler ofRichJson(UnframedGrpcStatusMappingFunction statusMappingFunction) { + // Ensure that unframedGrpcStatusMappingFunction never returns null + // by falling back to the default. + final UnframedGrpcStatusMappingFunction mappingFunction = + requireNonNull(statusMappingFunction, "statusMappingFunction") + .orElse(UnframedGrpcStatusMappingFunction.of()); + final MessageMarshaller errorDetailsMarshaller + = UnframedGrpcErrorHandlerUtils.getErrorDetailsMarshaller(); + return (ctx, status, response) -> { + final Logger logger = LoggerFactory.getLogger(UnframedGrpcErrorHandler.class); + final Code grpcCode = status.getCode(); + final String grpcMessage = status.getDescription(); + final RequestLogAccess log = ctx.log(); + final Throwable cause; + if (log.isAvailable(RequestLogProperty.RESPONSE_CAUSE)) { + cause = log.partial().responseCause(); + } else { + cause = null; + } + final HttpHeaders trailers = !response.trailers().isEmpty() ? + response.trailers() : response.headers(); + final String grpcStatusDetailsBin = trailers.get(GrpcHeaderNames.GRPC_STATUS_DETAILS_BIN); + final HttpStatus httpStatus = mappingFunction.apply(ctx, status, cause); + final ResponseHeaders responseHeaders = ResponseHeaders.builder(httpStatus) + .contentType(MediaType.JSON_UTF_8) + .addInt(GrpcHeaderNames.GRPC_STATUS, + grpcCode.value()) + .build(); + final ImmutableMap.Builder messageBuilder = ImmutableMap.builder(); + messageBuilder.put("code", grpcCode.name()); + if (grpcMessage != null) { + messageBuilder.put("message", grpcMessage); + } + if (cause != null && ctx.config().verboseResponses()) { + messageBuilder.put("stack-trace", Exceptions.traceText(cause)); + } + if (!Strings.isNullOrEmpty(grpcStatusDetailsBin)) { + com.google.rpc.Status rpcStatus = null; + try { + rpcStatus = UnframedGrpcErrorHandlerUtils.decodeGrpcStatusDetailsBin(grpcStatusDetailsBin); + } catch (InvalidProtocolBufferException e) { + logger.warn("invalid protobuf exception happens when decode grpc-status-details-bin {}", + grpcStatusDetailsBin); + } + if (rpcStatus != null) { + try { + messageBuilder.put("details", UnframedGrpcErrorHandlerUtils + .convertErrorDetailToJsonNode(rpcStatus.getDetailsList(), + errorDetailsMarshaller)); + } catch (IOException e) { + logger.warn("error happens when convert error converting rpc status {} to strings", + rpcStatus); + } + } + } + final Map errorObject = ImmutableMap.of("error", messageBuilder.build()); + return HttpResponse.ofJson(responseHeaders, errorObject); + }; + } + /** * Maps the gRPC error response to the {@link HttpResponse}. * diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerUtils.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerUtils.java new file mode 100644 index 00000000000..d2848483630 --- /dev/null +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerUtils.java @@ -0,0 +1,89 @@ +/* + * Copyright 2021 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.server.grpc; + +import java.io.IOException; +import java.io.StringWriter; +import java.util.Base64; +import java.util.List; + +import org.curioswitch.common.protobuf.json.MessageMarshaller; + +import com.fasterxml.jackson.core.JsonGenerator; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.protobuf.Any; +import com.google.protobuf.InvalidProtocolBufferException; +import com.google.rpc.BadRequest; +import com.google.rpc.DebugInfo; +import com.google.rpc.ErrorInfo; +import com.google.rpc.Help; +import com.google.rpc.LocalizedMessage; +import com.google.rpc.PreconditionFailure; +import com.google.rpc.QuotaFailure; +import com.google.rpc.RequestInfo; +import com.google.rpc.ResourceInfo; +import com.google.rpc.RetryInfo; +import com.google.rpc.Status; + +import com.linecorp.armeria.internal.common.JacksonUtil; + +/** + * An Utility class which contains methods related to Unframed grpc error handler. + */ +public final class UnframedGrpcErrorHandlerUtils { + + static MessageMarshaller getErrorDetailsMarshaller() { + return MessageMarshaller.builder() + .omittingInsignificantWhitespace(true) + .register(RetryInfo.getDefaultInstance()) + .register(ErrorInfo.getDefaultInstance()) + .register(QuotaFailure.getDefaultInstance()) + .register(DebugInfo.getDefaultInstance()) + .register(PreconditionFailure.getDefaultInstance()) + .register(BadRequest.getDefaultInstance()) + .register(RequestInfo.getDefaultInstance()) + .register(ResourceInfo.getDefaultInstance()) + .register(Help.getDefaultInstance()) + .register(LocalizedMessage.getDefaultInstance()) + .build(); + } + + private static final ObjectMapper mapper = JacksonUtil.newDefaultObjectMapper(); + + private UnframedGrpcErrorHandlerUtils() {} + + static JsonNode convertErrorDetailToJsonNode(List details, MessageMarshaller messageMarshaller) + throws IOException { + final StringWriter jsonObjectWriter = new StringWriter(); + try (JsonGenerator jsonGenerator = mapper.createGenerator(jsonObjectWriter)) { + jsonGenerator.writeStartArray(); + for (Any detail : details) { + messageMarshaller.writeValue(detail, jsonGenerator); + } + jsonGenerator.writeEndArray(); + jsonGenerator.flush(); + return mapper.readTree(jsonObjectWriter.toString()); + } + } + + static Status decodeGrpcStatusDetailsBin(String grpcStatusDetailsBin) + throws InvalidProtocolBufferException { + final byte[] result = Base64.getDecoder().decode(grpcStatusDetailsBin); + return Status.parseFrom(result); + } +} diff --git a/grpc/src/test/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerTest.java b/grpc/src/test/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerTest.java index bf50bb95cae..fe1a4132911 100644 --- a/grpc/src/test/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerTest.java +++ b/grpc/src/test/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerTest.java @@ -16,22 +16,31 @@ package com.linecorp.armeria.server.grpc; +import static net.javacrumbs.jsonunit.fluent.JsonFluentAssert.assertThatJson; import static org.assertj.core.api.Assertions.assertThat; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.protobuf.Any; +import com.google.rpc.Code; +import com.google.rpc.ErrorInfo; + import com.linecorp.armeria.client.BlockingWebClient; import com.linecorp.armeria.common.AggregatedHttpResponse; import com.linecorp.armeria.common.HttpStatus; import com.linecorp.armeria.common.MediaType; import com.linecorp.armeria.grpc.testing.TestServiceGrpc; import com.linecorp.armeria.grpc.testing.TestServiceGrpc.TestServiceImplBase; +import com.linecorp.armeria.internal.common.JacksonUtil; import com.linecorp.armeria.protobuf.EmptyProtos.Empty; import com.linecorp.armeria.server.ServerBuilder; import com.linecorp.armeria.testing.junit5.server.ServerExtension; import io.grpc.Status; +import io.grpc.protobuf.StatusProto; import io.grpc.stub.StreamObserver; public class UnframedGrpcErrorHandlerTest { @@ -39,7 +48,7 @@ public class UnframedGrpcErrorHandlerTest { static ServerExtension nonVerboseServer = new ServerExtension() { @Override protected void configure(ServerBuilder sb) { - configureServer(sb, false, UnframedGrpcErrorHandler.of()); + configureServer(sb, false, UnframedGrpcErrorHandler.of(), testService); } }; @@ -47,7 +56,7 @@ protected void configure(ServerBuilder sb) { static ServerExtension verbosePlainTextResServer = new ServerExtension() { @Override protected void configure(ServerBuilder sb) { - configureServer(sb, true, UnframedGrpcErrorHandler.ofPlainText()); + configureServer(sb, true, UnframedGrpcErrorHandler.ofPlainText(), testService); } }; @@ -55,7 +64,15 @@ protected void configure(ServerBuilder sb) { static ServerExtension verboseJsonResServer = new ServerExtension() { @Override protected void configure(ServerBuilder sb) { - configureServer(sb, true, UnframedGrpcErrorHandler.ofJson()); + configureServer(sb, true, UnframedGrpcErrorHandler.ofJson(), testService); + } + }; + + @RegisterExtension + static ServerExtension testServerGrpcStatus = new ServerExtension() { + @Override + protected void configure(ServerBuilder sb) { + configureServer(sb, false, UnframedGrpcErrorHandler.ofRichJson(), testServiceGrpcStatus); } }; @@ -67,13 +84,39 @@ public void emptyCall(Empty request, StreamObserver responseObserver) { } } + private static class TestServiceGrpcStatus extends TestServiceImplBase { + + @Override + public void emptyCall(Empty request, StreamObserver responseObserver) { + final ErrorInfo errorInfo = ErrorInfo.newBuilder() + .setDomain("test") + .setReason("Unknown Exception").build(); + + final com.google.rpc.Status + status = com.google.rpc.Status.newBuilder() + .setCode( + Code.UNKNOWN.getNumber()) + .setMessage("Unknown Exceptions Test") + .addDetails( + Any.pack(errorInfo)) + .build(); + + responseObserver.onError(StatusProto.toStatusRuntimeException(status)); + } + } + + private final ObjectMapper mapper = JacksonUtil.newDefaultObjectMapper(); + private static final TestService testService = new TestService(); + private static final TestServiceGrpcStatus testServiceGrpcStatus = new TestServiceGrpcStatus(); + private static void configureServer(ServerBuilder sb, boolean verboseResponses, - UnframedGrpcErrorHandler errorHandler) { + UnframedGrpcErrorHandler errorHandler, + TestServiceImplBase testServiceImplBase) { sb.verboseResponses(verboseResponses); sb.service(GrpcService.builder() - .addService(testService) + .addService(testServiceImplBase) .enableUnframedRequests(true) .unframedGrpcErrorHandler(errorHandler) .build()); @@ -122,4 +165,19 @@ void jsonWithStackTrace() { "\"stack-trace\":\"io.grpc.StatusException"); assertThat(response.trailers()).isEmpty(); } + + @Test + void richJson() throws JsonProcessingException { + final BlockingWebClient client = testServerGrpcStatus.webClient().blocking(); + final AggregatedHttpResponse response = + client.prepare() + .post(TestServiceGrpc.getEmptyCallMethod().getFullMethodName()) + .content(MediaType.PROTOBUF, Empty.getDefaultInstance().toByteArray()) + .execute(); + assertThat(response.status()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR); + assertThatJson(mapper.readTree(response.contentUtf8())).isEqualTo( + "{\"error\":{\"code\":\"UNKNOWN\",\"message\":\"Unknown Exceptions Test\"," + + "\"details\":[{\"@type\":\"type.googleapis.com/google.rpc.ErrorInfo\"," + + "\"reason\":\"Unknown Exception\",\"domain\":\"test\"}]}}"); + } } diff --git a/grpc/src/test/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerUtilsTest.java b/grpc/src/test/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerUtilsTest.java new file mode 100644 index 00000000000..2b5fab450a8 --- /dev/null +++ b/grpc/src/test/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerUtilsTest.java @@ -0,0 +1,145 @@ +/* + * Copyright 2022 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.server.grpc; + +import static net.javacrumbs.jsonunit.fluent.JsonFluentAssert.assertThatJson; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.io.IOException; + +import org.junit.jupiter.api.Test; + +import com.fasterxml.jackson.databind.JsonNode; +import com.google.protobuf.Any; +import com.google.protobuf.Duration; +import com.google.protobuf.Empty; +import com.google.rpc.BadRequest; +import com.google.rpc.BadRequest.FieldViolation; +import com.google.rpc.Code; +import com.google.rpc.DebugInfo; +import com.google.rpc.ErrorInfo; +import com.google.rpc.Help; +import com.google.rpc.Help.Link; +import com.google.rpc.LocalizedMessage; +import com.google.rpc.PreconditionFailure; +import com.google.rpc.QuotaFailure; +import com.google.rpc.QuotaFailure.Violation; +import com.google.rpc.RequestInfo; +import com.google.rpc.ResourceInfo; +import com.google.rpc.RetryInfo; +import com.google.rpc.Status; + +class UnframedGrpcErrorHandlerUtilsTest { + + @Test + void convertErrorDetailToJsonNodeTest() throws IOException { + final ErrorInfo errorInfo = ErrorInfo.newBuilder() + .setDomain("test") + .setReason("Unknown Exception") + .putMetadata("key", "value") + .build(); + final RetryInfo retryInfo = RetryInfo.newBuilder() + .setRetryDelay(Duration.newBuilder() + .setSeconds(100) + .build()) + .build(); + final DebugInfo debugInfo = DebugInfo.newBuilder() + .setDetail("debug info") + .addStackEntries("stack1") + .addStackEntries("stack2") + .build(); + final QuotaFailure quotaFailure = + QuotaFailure.newBuilder() + .addViolations(Violation.newBuilder().setDescription("quota").build()) + .build(); + + final PreconditionFailure preconditionFailure = + PreconditionFailure.newBuilder() + .addViolations( + PreconditionFailure.Violation.newBuilder() + .setDescription("violation") + .build()) + .build(); + final BadRequest badRequest = + BadRequest.newBuilder() + .addFieldViolations( + FieldViolation.newBuilder() + .setDescription("field violation") + .build()) + .build(); + final RequestInfo requestInfo = RequestInfo.newBuilder() + .setRequestId("requestid") + .build(); + final ResourceInfo resourceInfo = ResourceInfo.newBuilder() + .setDescription("description") + .build(); + final Help help = Help.newBuilder() + .addLinks(Link.newBuilder() + .setDescription("descrption") + .build()) + .build(); + final LocalizedMessage localizedMessage = LocalizedMessage.newBuilder() + .setMessage("message") + .build(); + + final Status status = Status.newBuilder() + .setCode( + Code.UNKNOWN.getNumber()) + .setMessage("Unknown Exceptions Test") + .addDetails(Any.pack(errorInfo)) + .addDetails(Any.pack(retryInfo)) + .addDetails(Any.pack(debugInfo)) + .addDetails(Any.pack(quotaFailure)) + .addDetails(Any.pack(preconditionFailure)) + .addDetails(Any.pack(badRequest)) + .addDetails(Any.pack(requestInfo)) + .addDetails(Any.pack(resourceInfo)) + .addDetails(Any.pack(help)) + .addDetails(Any.pack(localizedMessage)) + .build(); + + final JsonNode jsonNode = UnframedGrpcErrorHandlerUtils.convertErrorDetailToJsonNode( + status.getDetailsList(), UnframedGrpcErrorHandlerUtils.getErrorDetailsMarshaller()); + final String expectedJsonString = + "[{\"@type\":\"type.googleapis.com/google.rpc.ErrorInfo\"," + + "\"reason\":\"Unknown Exception\",\"domain\":\"test\",\"metadata\":{\"key\":\"value\"}}," + + "{\"@type\":\"type.googleapis.com/google.rpc.RetryInfo\",\"retryDelay\":\"100s\"}," + + "{\"@type\":\"type.googleapis.com/google.rpc.DebugInfo\"," + + "\"stackEntries\":[\"stack1\",\"stack2\"],\"detail\":\"debug info\"}," + + "{\"@type\":\"type.googleapis.com/google.rpc.QuotaFailure\"," + + "\"violations\":[{\"description\":\"quota\"}]}," + + "{\"@type\":\"type.googleapis.com/google.rpc.PreconditionFailure\"," + + "\"violations\":[{\"description\":\"violation\"}]}," + + "{\"@type\":\"type.googleapis.com/google.rpc.BadRequest\"," + + "\"fieldViolations\":[{\"description\":\"field violation\"}]}," + + "{\"@type\":\"type.googleapis.com/google.rpc.RequestInfo\",\"requestId\":\"requestid\"}," + + "{\"@type\":\"type.googleapis.com/google.rpc.ResourceInfo\",\"description\":\"description\"}," + + "{\"@type\":\"type.googleapis.com/google.rpc.Help\"," + + "\"links\":[{\"description\":\"descrption\"}]}," + + "{\"@type\":\"type.googleapis.com/google.rpc.LocalizedMessage\",\"message\":\"message\"}]"; + assertThatJson(jsonNode).isEqualTo(expectedJsonString); + } + + @Test + void shouldThrowIOException() { + final Empty empty = Empty.getDefaultInstance(); + final Status status = Status.newBuilder().addDetails(Any.pack(empty)).build(); + assertThatThrownBy(() -> UnframedGrpcErrorHandlerUtils.convertErrorDetailToJsonNode( + status.getDetailsList(), UnframedGrpcErrorHandlerUtils.getErrorDetailsMarshaller())) + .isInstanceOf(IOException.class); + } +} From 665741a5e3434d32417750d9d148874937e6c389 Mon Sep 17 00:00:00 2001 From: Hu Guang Date: Tue, 26 Apr 2022 21:42:56 +0800 Subject: [PATCH 02/36] some conflict --- .../armeria/common/grpc/protocol/GrpcHeaderNames.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/grpc-protocol/src/main/java/com/linecorp/armeria/common/grpc/protocol/GrpcHeaderNames.java b/grpc-protocol/src/main/java/com/linecorp/armeria/common/grpc/protocol/GrpcHeaderNames.java index 2ed44217c0e..3075626ee1c 100644 --- a/grpc-protocol/src/main/java/com/linecorp/armeria/common/grpc/protocol/GrpcHeaderNames.java +++ b/grpc-protocol/src/main/java/com/linecorp/armeria/common/grpc/protocol/GrpcHeaderNames.java @@ -56,10 +56,5 @@ public final class GrpcHeaderNames { */ public static final AsciiString GRPC_STATUS_DETAILS_BIN = HttpHeaderNames.of("grpc-status-details-bin"); - /** - * {@code "grpc-status-details-bin"}. - */ - public static final AsciiString GRPC_STATUS_DETAILS_BIN = HttpHeaderNames.of("grpc-status-details-bin"); - private GrpcHeaderNames() {} } From c1993e3087b8ed63d67dc294be7d03a1694b3a1c Mon Sep 17 00:00:00 2001 From: Hu Guang Date: Wed, 27 Apr 2022 11:06:19 +0800 Subject: [PATCH 03/36] add status to response --- .../armeria/server/grpc/UnframedGrpcErrorHandler.java | 3 ++- .../server/grpc/GrpcStatusDetailsBinHeaderTest.java | 9 +-------- .../server/grpc/UnframedGrpcErrorHandlerTest.java | 4 +++- 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java index 115b7cd3289..c3cfbf74c30 100644 --- a/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java @@ -223,7 +223,8 @@ static UnframedGrpcErrorHandler ofRichJson(UnframedGrpcStatusMappingFunction sta grpcCode.value()) .build(); final ImmutableMap.Builder messageBuilder = ImmutableMap.builder(); - messageBuilder.put("code", grpcCode.name()); + messageBuilder.put("code", httpStatus.code()); + messageBuilder.put("status", grpcCode.name()); if (grpcMessage != null) { messageBuilder.put("message", grpcMessage); } diff --git a/grpc/src/test/java/com/linecorp/armeria/server/grpc/GrpcStatusDetailsBinHeaderTest.java b/grpc/src/test/java/com/linecorp/armeria/server/grpc/GrpcStatusDetailsBinHeaderTest.java index f37a4f6d909..d5b11cc4754 100644 --- a/grpc/src/test/java/com/linecorp/armeria/server/grpc/GrpcStatusDetailsBinHeaderTest.java +++ b/grpc/src/test/java/com/linecorp/armeria/server/grpc/GrpcStatusDetailsBinHeaderTest.java @@ -16,10 +16,9 @@ package com.linecorp.armeria.server.grpc; +import static com.linecorp.armeria.server.grpc.UnframedGrpcErrorHandlerUtils.decodeGrpcStatusDetailsBin; import static org.assertj.core.api.Assertions.assertThat; -import java.util.Base64; - import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; @@ -92,12 +91,6 @@ private static void configureServer(ServerBuilder sb, .build()); } - private static Status decodeGrpcStatusDetailsBin(String grpcStatusDetailsBin) - throws InvalidProtocolBufferException { - final byte[] result = Base64.getDecoder().decode(grpcStatusDetailsBin); - return Status.parseFrom(result); - } - @Test void googleRpcErrorDetail() throws InvalidProtocolBufferException { final BlockingWebClient client = testServer.webClient().blocking(); diff --git a/grpc/src/test/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerTest.java b/grpc/src/test/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerTest.java index fe1a4132911..f1647424dd0 100644 --- a/grpc/src/test/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerTest.java +++ b/grpc/src/test/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerTest.java @@ -176,8 +176,10 @@ void richJson() throws JsonProcessingException { .execute(); assertThat(response.status()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR); assertThatJson(mapper.readTree(response.contentUtf8())).isEqualTo( - "{\"error\":{\"code\":\"UNKNOWN\",\"message\":\"Unknown Exceptions Test\"," + + "{\"error\":{\"code\":500,\"status\":\"UNKNOWN\"," + + "\"message\":\"Unknown Exceptions Test\"," + "\"details\":[{\"@type\":\"type.googleapis.com/google.rpc.ErrorInfo\"," + "\"reason\":\"Unknown Exception\",\"domain\":\"test\"}]}}"); + assertThat(response.trailers()).isEmpty(); } } From 68669219c6ca6849835be7cde164f75c108866d9 Mon Sep 17 00:00:00 2001 From: Hu Guang Date: Wed, 27 Apr 2022 11:08:27 +0800 Subject: [PATCH 04/36] change license header --- .../armeria/server/grpc/UnframedGrpcErrorHandlerUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerUtils.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerUtils.java index d2848483630..f1db5c3a81f 100644 --- a/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerUtils.java +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerUtils.java @@ -1,5 +1,5 @@ /* - * Copyright 2021 LINE Corporation + * Copyright 2022 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 From 659aa8db9d736a9afd71e5ac3f5886edde9c2dae Mon Sep 17 00:00:00 2001 From: Hu Guang Date: Wed, 27 Apr 2022 11:42:25 +0800 Subject: [PATCH 05/36] some minor change --- .../grpc/UnframedGrpcErrorHandlerUtils.java | 29 ++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerUtils.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerUtils.java index f1db5c3a81f..89ffa7fc24b 100644 --- a/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerUtils.java +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerUtils.java @@ -47,20 +47,23 @@ */ public final class UnframedGrpcErrorHandlerUtils { + private static final MessageMarshaller ERROR_DETAILS_MARSHALLER = + MessageMarshaller.builder() + .omittingInsignificantWhitespace(true) + .register(RetryInfo.getDefaultInstance()) + .register(ErrorInfo.getDefaultInstance()) + .register(QuotaFailure.getDefaultInstance()) + .register(DebugInfo.getDefaultInstance()) + .register(PreconditionFailure.getDefaultInstance()) + .register(BadRequest.getDefaultInstance()) + .register(RequestInfo.getDefaultInstance()) + .register(ResourceInfo.getDefaultInstance()) + .register(Help.getDefaultInstance()) + .register(LocalizedMessage.getDefaultInstance()) + .build(); + static MessageMarshaller getErrorDetailsMarshaller() { - return MessageMarshaller.builder() - .omittingInsignificantWhitespace(true) - .register(RetryInfo.getDefaultInstance()) - .register(ErrorInfo.getDefaultInstance()) - .register(QuotaFailure.getDefaultInstance()) - .register(DebugInfo.getDefaultInstance()) - .register(PreconditionFailure.getDefaultInstance()) - .register(BadRequest.getDefaultInstance()) - .register(RequestInfo.getDefaultInstance()) - .register(ResourceInfo.getDefaultInstance()) - .register(Help.getDefaultInstance()) - .register(LocalizedMessage.getDefaultInstance()) - .build(); + return ERROR_DETAILS_MARSHALLER; } private static final ObjectMapper mapper = JacksonUtil.newDefaultObjectMapper(); From 4ab4a28f9069713c0338f229f1d143e6318598fc Mon Sep 17 00:00:00 2001 From: Hu Guang Date: Wed, 27 Apr 2022 11:55:33 +0800 Subject: [PATCH 06/36] some comment --- .../linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java index c3cfbf74c30..91ff8bdb9da 100644 --- a/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java @@ -182,7 +182,8 @@ static UnframedGrpcErrorHandler ofPlainText(UnframedGrpcStatusMappingFunction st } /** - * Returns a rich json response. + * Returns a rich json response based on google api. + * Please refer google error_model */ static UnframedGrpcErrorHandler ofRichJson() { return ofRichJson(UnframedGrpcStatusMappingFunction.of()); From 69980c19e9e2b5987e19f0c40bc62f93908cad92 Mon Sep 17 00:00:00 2001 From: Hu Guang Date: Mon, 16 May 2022 20:09:05 +0800 Subject: [PATCH 07/36] tmp --- .../grpc/DefaultUnframedGrpcErrorHandler.java | 255 ++++++++++++++++++ .../server/grpc/UnframedGrpcErrorHandler.java | 52 ++-- .../grpc/UnframedGrpcErrorHandlerUtils.java | 2 +- 3 files changed, 282 insertions(+), 27 deletions(-) create mode 100644 grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java new file mode 100644 index 00000000000..6a71de6a627 --- /dev/null +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java @@ -0,0 +1,255 @@ +package com.linecorp.armeria.server.grpc; + +import static java.util.Objects.requireNonNull; + +import java.io.IOException; +import java.util.Map; + +import javax.annotation.Nonnull; + +import org.curioswitch.common.protobuf.json.MessageMarshaller; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.google.common.base.Strings; +import com.google.common.collect.ImmutableMap; +import com.google.protobuf.InvalidProtocolBufferException; + +import com.linecorp.armeria.common.HttpData; +import com.linecorp.armeria.common.HttpHeaders; +import com.linecorp.armeria.common.HttpResponse; +import com.linecorp.armeria.common.HttpStatus; +import com.linecorp.armeria.common.MediaType; +import com.linecorp.armeria.common.ResponseHeaders; +import com.linecorp.armeria.common.ResponseHeadersBuilder; +import com.linecorp.armeria.common.grpc.protocol.GrpcHeaderNames; +import com.linecorp.armeria.common.logging.RequestLogAccess; +import com.linecorp.armeria.common.logging.RequestLogProperty; +import com.linecorp.armeria.common.util.Exceptions; +import com.linecorp.armeria.internal.common.util.TemporaryThreadLocals; +import com.linecorp.armeria.server.ServiceRequestContext; + +import io.grpc.Status; +import io.grpc.Status.Code; + +class DefaultUnframedGrpcErrorHandler { + + static final Logger logger = LoggerFactory.getLogger(DefaultUnframedGrpcErrorHandler.class); + +// /** +// * Returns a plain text or json response based on the content type. +// */ +// static UnframedGrpcErrorHandler of() { +// return of(UnframedGrpcStatusMappingFunction.of()); +// } + + private static UnframedGrpcStatusMappingFunction get(UnframedGrpcStatusMappingFunction statusMappingFunction) { + return requireNonNull(statusMappingFunction, "statusMappingFunction") + .orElse(UnframedGrpcStatusMappingFunction.of()); + } + + private static ResponseHeadersBuilder buildResponseHeaders(ServiceRequestContext ctx, Status status, UnframedGrpcStatusMappingFunction mappingFunction) { + final Code grpcCode = status.getCode(); + final RequestLogAccess log = ctx.log(); + final Throwable cause; + if (log.isAvailable(RequestLogProperty.RESPONSE_CAUSE)) { + cause = log.partial().responseCause(); + } else { + cause = null; + } + final HttpStatus httpStatus = mappingFunction.apply(ctx, status, cause); + return ResponseHeaders.builder(httpStatus) + .addInt(GrpcHeaderNames.GRPC_STATUS, + grpcCode.value()); + } + + /** + * Returns a plain text or json response based on the content type. + * + * @param statusMappingFunction The function which maps the {@link Throwable} or gRPC {@link Status} code + * to an {@link HttpStatus} code. + */ + static UnframedGrpcErrorHandler of(UnframedGrpcStatusMappingFunction statusMappingFunction) { + // Ensure that unframedGrpcStatusMappingFunction never returns null + // by falling back to the default. + final UnframedGrpcStatusMappingFunction mappingFunction = get(statusMappingFunction); + return (ctx, status, response) -> { + final MediaType grpcMediaType = response.contentType(); + if (grpcMediaType != null && grpcMediaType.isJson()) { + return ofJson(mappingFunction).handle(ctx, status, response); + } else { + return ofPlainText(mappingFunction).handle(ctx, status, response); + } + }; + } + +// /** +// * Returns a json response. +// */ +// static UnframedGrpcErrorHandler ofJson() { +// return ofJson(UnframedGrpcStatusMappingFunction.of()); +// } + + + /** + * Returns a json response. + * + * @param statusMappingFunction The function which maps the {@link Throwable} or gRPC {@link Status} code + * to an {@link HttpStatus} code. + */ + static UnframedGrpcErrorHandler ofJson(UnframedGrpcStatusMappingFunction statusMappingFunction) { + // Ensure that unframedGrpcStatusMappingFunction never returns null + // by falling back to the default. + final UnframedGrpcStatusMappingFunction mappingFunction = get(statusMappingFunction); + return (ctx, status, response) -> { + final String grpcMessage = status.getDescription(); + final RequestLogAccess log = ctx.log(); + final Throwable cause; + if (log.isAvailable(RequestLogProperty.RESPONSE_CAUSE)) { + cause = log.partial().responseCause(); + } else { + cause = null; + } + final HttpStatus httpStatus = mappingFunction.apply(ctx, status, cause); + final ResponseHeaders responseHeaders = ResponseHeaders.builder(httpStatus) + .contentType(MediaType.JSON_UTF_8) + .addInt(GrpcHeaderNames.GRPC_STATUS, + status.getCode().value()) + .build(); + final ImmutableMap.Builder messageBuilder = ImmutableMap.builder(); + messageBuilder.put("grpc-code", grpcCode.name()); + if (grpcMessage != null) { + messageBuilder.put("message", grpcMessage); + } + if (cause != null && ctx.config().verboseResponses()) { + messageBuilder.put("stack-trace", Exceptions.traceText(cause)); + } + return HttpResponse.ofJson(responseHeaders, messageBuilder.build()); + }; + } + + /** + * Returns a plain text response. + */ + static UnframedGrpcErrorHandler ofPlainText() { + return ofPlainText(UnframedGrpcStatusMappingFunction.of()); + } + + /** + * Returns a plain text response. + * + * @param statusMappingFunction The function which maps the {@link Throwable} or gRPC {@link Status} code + * to an {@link HttpStatus} code. + */ + static UnframedGrpcErrorHandler ofPlainText(UnframedGrpcStatusMappingFunction statusMappingFunction) { + // Ensure that unframedGrpcStatusMappingFunction never returns null + // by falling back to the default. + final UnframedGrpcStatusMappingFunction mappingFunction = get(statusMappingFunction); + return (ctx, status, response) -> { + final Code grpcCode = status.getCode(); + final RequestLogAccess log = ctx.log(); + final Throwable cause; + if (log.isAvailable(RequestLogProperty.RESPONSE_CAUSE)) { + cause = log.partial().responseCause(); + } else { + cause = null; + } + final HttpStatus httpStatus = mappingFunction.apply(ctx, status, cause); + final ResponseHeaders responseHeaders = ResponseHeaders.builder(httpStatus) + .contentType(MediaType.PLAIN_TEXT_UTF_8) + .addInt(GrpcHeaderNames.GRPC_STATUS, + grpcCode.value()) + .build(); + final HttpData content; + try (TemporaryThreadLocals ttl = TemporaryThreadLocals.acquire()) { + final StringBuilder msg = ttl.stringBuilder(); + msg.append("grpc-code: ").append(grpcCode.name()); + final String grpcMessage = status.getDescription(); + if (grpcMessage != null) { + msg.append(", ").append(grpcMessage); + } + if (cause != null && ctx.config().verboseResponses()) { + msg.append("\nstack-trace:\n").append(Exceptions.traceText(cause)); + } + content = HttpData.ofUtf8(msg); + } + return HttpResponse.of(responseHeaders, content); + }; + } + + /** + * Returns a rich error JSON response based on Google APIs. + * Please refer Google error model + */ + static UnframedGrpcErrorHandler ofRichJson() { + return ofRichJson(UnframedGrpcStatusMappingFunction.of()); + } + + /** + * Returns a rich error JSON response based on Google APIs. + * Please refer Google error model + * @param statusMappingFunction The function which maps the {@link Throwable} or gRPC {@link Status} code + * to an {@link HttpStatus} code. + */ + static UnframedGrpcErrorHandler ofRichJson(UnframedGrpcStatusMappingFunction statusMappingFunction) { + // Ensure that unframedGrpcStatusMappingFunction never returns null + // by falling back to the default. + final UnframedGrpcStatusMappingFunction mappingFunction = + requireNonNull(statusMappingFunction, "statusMappingFunction") + .orElse(UnframedGrpcStatusMappingFunction.of()); + final MessageMarshaller errorDetailsMarshaller + = UnframedGrpcErrorHandlerUtils.getErrorDetailsMarshaller(); + return (ctx, status, response) -> { + //final Logger logger = LoggerFactory.getLogger(UnframedGrpcErrorHandler.class); + final Code grpcCode = status.getCode(); + final String grpcMessage = status.getDescription(); + final RequestLogAccess log = ctx.log(); + final Throwable cause; + if (log.isAvailable(RequestLogProperty.RESPONSE_CAUSE)) { + cause = log.partial().responseCause(); + } else { + cause = null; + } + final HttpHeaders trailers = !response.trailers().isEmpty() ? + response.trailers() : response.headers(); + final String grpcStatusDetailsBin = trailers.get(GrpcHeaderNames.GRPC_STATUS_DETAILS_BIN); + final HttpStatus httpStatus = mappingFunction.apply(ctx, status, cause); + final ResponseHeaders responseHeaders = ResponseHeaders.builder(httpStatus) + .contentType(MediaType.JSON_UTF_8) + .addInt(GrpcHeaderNames.GRPC_STATUS, + grpcCode.value()) + .build(); + final ImmutableMap.Builder messageBuilder = ImmutableMap.builder(); + messageBuilder.put("code", httpStatus.code()); + messageBuilder.put("status", grpcCode.name()); + if (grpcMessage != null) { + messageBuilder.put("message", grpcMessage); + } + if (cause != null && ctx.config().verboseResponses()) { + messageBuilder.put("stack-trace", Exceptions.traceText(cause)); + } + if (!Strings.isNullOrEmpty(grpcStatusDetailsBin)) { + com.google.rpc.Status rpcStatus = null; + try { + rpcStatus = UnframedGrpcErrorHandlerUtils.decodeGrpcStatusDetailsBin(grpcStatusDetailsBin); + } catch (InvalidProtocolBufferException e) { + logger.warn("invalid protobuf exception happens when decode grpc-status-details-bin {}", + grpcStatusDetailsBin); + } + if (rpcStatus != null) { + try { + messageBuilder.put("details", UnframedGrpcErrorHandlerUtils + .convertErrorDetailToJsonNode(rpcStatus.getDetailsList(), + errorDetailsMarshaller)); + } catch (IOException e) { + logger.warn("error happens when convert error converting rpc status {} to strings", + rpcStatus); + } + } + } + final Map errorObject = ImmutableMap.of("error", messageBuilder.build()); + return HttpResponse.ofJson(responseHeaders, errorObject); + }; + } + +} diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java index 91ff8bdb9da..1d3b8f8ea1b 100644 --- a/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java @@ -58,30 +58,30 @@ public interface UnframedGrpcErrorHandler { * Returns a plain text or json response based on the content type. */ static UnframedGrpcErrorHandler of() { - return of(UnframedGrpcStatusMappingFunction.of()); + return DefaultUnframedGrpcErrorHandler.of(UnframedGrpcStatusMappingFunction.of()); } - /** - * Returns a plain text or json response based on the content type. - * - * @param statusMappingFunction The function which maps the {@link Throwable} or gRPC {@link Status} code - * to an {@link HttpStatus} code. - */ - static UnframedGrpcErrorHandler of(UnframedGrpcStatusMappingFunction statusMappingFunction) { - // Ensure that unframedGrpcStatusMappingFunction never returns null - // by falling back to the default. - final UnframedGrpcStatusMappingFunction mappingFunction = - requireNonNull(statusMappingFunction, "statusMappingFunction") - .orElse(UnframedGrpcStatusMappingFunction.of()); - return (ctx, status, response) -> { - final MediaType grpcMediaType = response.contentType(); - if (grpcMediaType != null && grpcMediaType.isJson()) { - return ofJson(mappingFunction).handle(ctx, status, response); - } else { - return ofPlainText(mappingFunction).handle(ctx, status, response); - } - }; - } +// /** +// * Returns a plain text or json response based on the content type. +// * +// * @param statusMappingFunction The function which maps the {@link Throwable} or gRPC {@link Status} code +// * to an {@link HttpStatus} code. +// */ +// static UnframedGrpcErrorHandler of(UnframedGrpcStatusMappingFunction statusMappingFunction) { +// // Ensure that unframedGrpcStatusMappingFunction never returns null +// // by falling back to the default. +// final UnframedGrpcStatusMappingFunction mappingFunction = +// requireNonNull(statusMappingFunction, "statusMappingFunction") +// .orElse(UnframedGrpcStatusMappingFunction.of()); +// return (ctx, status, response) -> { +// final MediaType grpcMediaType = response.contentType(); +// if (grpcMediaType != null && grpcMediaType.isJson()) { +// return ofJson(mappingFunction).handle(ctx, status, response); +// } else { +// return ofPlainText(mappingFunction).handle(ctx, status, response); +// } +// }; +// } /** * Returns a json response. @@ -182,16 +182,16 @@ static UnframedGrpcErrorHandler ofPlainText(UnframedGrpcStatusMappingFunction st } /** - * Returns a rich json response based on google api. - * Please refer google error_model + * Returns a rich error JSON response based on Google APIs. + * Please refer Google error model */ static UnframedGrpcErrorHandler ofRichJson() { return ofRichJson(UnframedGrpcStatusMappingFunction.of()); } /** - * Returns a rich json response. - * + * Returns a rich error JSON response based on Google APIs. + * Please refer Google error model * @param statusMappingFunction The function which maps the {@link Throwable} or gRPC {@link Status} code * to an {@link HttpStatus} code. */ diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerUtils.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerUtils.java index 89ffa7fc24b..75e78b65b94 100644 --- a/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerUtils.java +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerUtils.java @@ -45,7 +45,7 @@ /** * An Utility class which contains methods related to Unframed grpc error handler. */ -public final class UnframedGrpcErrorHandlerUtils { +final class UnframedGrpcErrorHandlerUtils { private static final MessageMarshaller ERROR_DETAILS_MARSHALLER = MessageMarshaller.builder() From a67121d090d319a19d7c61df7fe4fac1339f8376 Mon Sep 17 00:00:00 2001 From: Hu Guang Date: Mon, 16 May 2022 21:29:30 +0800 Subject: [PATCH 08/36] some code cleanup --- .../grpc/DefaultUnframedGrpcErrorHandler.java | 182 ++++++++-------- .../server/grpc/UnframedGrpcErrorHandler.java | 194 ++---------------- .../grpc/UnframedGrpcErrorHandlerUtils.java | 92 --------- .../grpc/GrpcStatusDetailsBinHeaderTest.java | 3 +- .../UnframedGrpcErrorHandlerUtilsTest.java | 9 +- 5 files changed, 119 insertions(+), 361 deletions(-) delete mode 100644 grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerUtils.java diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java index 6a71de6a627..b216354ac8d 100644 --- a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java @@ -1,19 +1,50 @@ +/* + * Copyright 2021 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.server.grpc; import static java.util.Objects.requireNonNull; import java.io.IOException; +import java.io.StringWriter; +import java.util.Base64; +import java.util.List; import java.util.Map; -import javax.annotation.Nonnull; - import org.curioswitch.common.protobuf.json.MessageMarshaller; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.fasterxml.jackson.core.JsonGenerator; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.base.Strings; import com.google.common.collect.ImmutableMap; +import com.google.protobuf.Any; import com.google.protobuf.InvalidProtocolBufferException; +import com.google.rpc.BadRequest; +import com.google.rpc.DebugInfo; +import com.google.rpc.ErrorInfo; +import com.google.rpc.Help; +import com.google.rpc.LocalizedMessage; +import com.google.rpc.PreconditionFailure; +import com.google.rpc.QuotaFailure; +import com.google.rpc.RequestInfo; +import com.google.rpc.ResourceInfo; +import com.google.rpc.RetryInfo; import com.linecorp.armeria.common.HttpData; import com.linecorp.armeria.common.HttpHeaders; @@ -21,35 +52,70 @@ import com.linecorp.armeria.common.HttpStatus; import com.linecorp.armeria.common.MediaType; import com.linecorp.armeria.common.ResponseHeaders; -import com.linecorp.armeria.common.ResponseHeadersBuilder; +import com.linecorp.armeria.common.annotation.Nullable; import com.linecorp.armeria.common.grpc.protocol.GrpcHeaderNames; import com.linecorp.armeria.common.logging.RequestLogAccess; import com.linecorp.armeria.common.logging.RequestLogProperty; import com.linecorp.armeria.common.util.Exceptions; +import com.linecorp.armeria.internal.common.JacksonUtil; import com.linecorp.armeria.internal.common.util.TemporaryThreadLocals; import com.linecorp.armeria.server.ServiceRequestContext; import io.grpc.Status; import io.grpc.Status.Code; -class DefaultUnframedGrpcErrorHandler { +final class DefaultUnframedGrpcErrorHandler { + + private static final Logger logger = LoggerFactory.getLogger(DefaultUnframedGrpcErrorHandler.class); - static final Logger logger = LoggerFactory.getLogger(DefaultUnframedGrpcErrorHandler.class); + private static final MessageMarshaller ERROR_DETAILS_MARSHALLER = + MessageMarshaller.builder() + .omittingInsignificantWhitespace(true) + .register(RetryInfo.getDefaultInstance()) + .register(ErrorInfo.getDefaultInstance()) + .register(QuotaFailure.getDefaultInstance()) + .register(DebugInfo.getDefaultInstance()) + .register(PreconditionFailure.getDefaultInstance()) + .register(BadRequest.getDefaultInstance()) + .register(RequestInfo.getDefaultInstance()) + .register(ResourceInfo.getDefaultInstance()) + .register(Help.getDefaultInstance()) + .register(LocalizedMessage.getDefaultInstance()) + .build(); -// /** -// * Returns a plain text or json response based on the content type. -// */ -// static UnframedGrpcErrorHandler of() { -// return of(UnframedGrpcStatusMappingFunction.of()); -// } + private static final ObjectMapper mapper = JacksonUtil.newDefaultObjectMapper(); - private static UnframedGrpcStatusMappingFunction get(UnframedGrpcStatusMappingFunction statusMappingFunction) { + /** + * Ensure that unframedGrpcStatusMappingFunction never returns null by falling back to the default. + */ + private static UnframedGrpcStatusMappingFunction ofStatusMappingFunction( + UnframedGrpcStatusMappingFunction statusMappingFunction) { return requireNonNull(statusMappingFunction, "statusMappingFunction") .orElse(UnframedGrpcStatusMappingFunction.of()); } - private static ResponseHeadersBuilder buildResponseHeaders(ServiceRequestContext ctx, Status status, UnframedGrpcStatusMappingFunction mappingFunction) { - final Code grpcCode = status.getCode(); + static JsonNode convertErrorDetailToJsonNode(List details) + throws IOException { + final StringWriter jsonObjectWriter = new StringWriter(); + try (JsonGenerator jsonGenerator = mapper.createGenerator(jsonObjectWriter)) { + jsonGenerator.writeStartArray(); + for (Any detail : details) { + ERROR_DETAILS_MARSHALLER.writeValue(detail, jsonGenerator); + } + jsonGenerator.writeEndArray(); + jsonGenerator.flush(); + return mapper.readTree(jsonObjectWriter.toString()); + } + } + + static com.google.rpc.Status decodeGrpcStatusDetailsBin(String grpcStatusDetailsBin) + throws InvalidProtocolBufferException { + final byte[] result = Base64.getDecoder().decode(grpcStatusDetailsBin); + return com.google.rpc.Status.parseFrom(result); + } + + @Nullable + private static Throwable getThrowableFromContext(ServiceRequestContext ctx) { final RequestLogAccess log = ctx.log(); final Throwable cause; if (log.isAvailable(RequestLogProperty.RESPONSE_CAUSE)) { @@ -57,10 +123,7 @@ private static ResponseHeadersBuilder buildResponseHeaders(ServiceRequestContext } else { cause = null; } - final HttpStatus httpStatus = mappingFunction.apply(ctx, status, cause); - return ResponseHeaders.builder(httpStatus) - .addInt(GrpcHeaderNames.GRPC_STATUS, - grpcCode.value()); + return cause; } /** @@ -70,9 +133,8 @@ private static ResponseHeadersBuilder buildResponseHeaders(ServiceRequestContext * to an {@link HttpStatus} code. */ static UnframedGrpcErrorHandler of(UnframedGrpcStatusMappingFunction statusMappingFunction) { - // Ensure that unframedGrpcStatusMappingFunction never returns null - // by falling back to the default. - final UnframedGrpcStatusMappingFunction mappingFunction = get(statusMappingFunction); + final UnframedGrpcStatusMappingFunction mappingFunction + = ofStatusMappingFunction(statusMappingFunction); return (ctx, status, response) -> { final MediaType grpcMediaType = response.contentType(); if (grpcMediaType != null && grpcMediaType.isJson()) { @@ -83,14 +145,6 @@ static UnframedGrpcErrorHandler of(UnframedGrpcStatusMappingFunction statusMappi }; } -// /** -// * Returns a json response. -// */ -// static UnframedGrpcErrorHandler ofJson() { -// return ofJson(UnframedGrpcStatusMappingFunction.of()); -// } - - /** * Returns a json response. * @@ -98,18 +152,13 @@ static UnframedGrpcErrorHandler of(UnframedGrpcStatusMappingFunction statusMappi * to an {@link HttpStatus} code. */ static UnframedGrpcErrorHandler ofJson(UnframedGrpcStatusMappingFunction statusMappingFunction) { - // Ensure that unframedGrpcStatusMappingFunction never returns null - // by falling back to the default. - final UnframedGrpcStatusMappingFunction mappingFunction = get(statusMappingFunction); + final UnframedGrpcStatusMappingFunction mappingFunction + = ofStatusMappingFunction(statusMappingFunction); return (ctx, status, response) -> { + final Code grpcCode = status.getCode(); final String grpcMessage = status.getDescription(); - final RequestLogAccess log = ctx.log(); - final Throwable cause; - if (log.isAvailable(RequestLogProperty.RESPONSE_CAUSE)) { - cause = log.partial().responseCause(); - } else { - cause = null; - } + @Nullable + final Throwable cause = getThrowableFromContext(ctx); final HttpStatus httpStatus = mappingFunction.apply(ctx, status, cause); final ResponseHeaders responseHeaders = ResponseHeaders.builder(httpStatus) .contentType(MediaType.JSON_UTF_8) @@ -128,13 +177,6 @@ static UnframedGrpcErrorHandler ofJson(UnframedGrpcStatusMappingFunction statusM }; } - /** - * Returns a plain text response. - */ - static UnframedGrpcErrorHandler ofPlainText() { - return ofPlainText(UnframedGrpcStatusMappingFunction.of()); - } - /** * Returns a plain text response. * @@ -142,18 +184,13 @@ static UnframedGrpcErrorHandler ofPlainText() { * to an {@link HttpStatus} code. */ static UnframedGrpcErrorHandler ofPlainText(UnframedGrpcStatusMappingFunction statusMappingFunction) { - // Ensure that unframedGrpcStatusMappingFunction never returns null - // by falling back to the default. - final UnframedGrpcStatusMappingFunction mappingFunction = get(statusMappingFunction); + final UnframedGrpcStatusMappingFunction mappingFunction + = ofStatusMappingFunction(statusMappingFunction); return (ctx, status, response) -> { final Code grpcCode = status.getCode(); - final RequestLogAccess log = ctx.log(); - final Throwable cause; - if (log.isAvailable(RequestLogProperty.RESPONSE_CAUSE)) { - cause = log.partial().responseCause(); - } else { - cause = null; - } + final String grpcMessage = status.getDescription(); + @Nullable + final Throwable cause = getThrowableFromContext(ctx); final HttpStatus httpStatus = mappingFunction.apply(ctx, status, cause); final ResponseHeaders responseHeaders = ResponseHeaders.builder(httpStatus) .contentType(MediaType.PLAIN_TEXT_UTF_8) @@ -164,7 +201,6 @@ static UnframedGrpcErrorHandler ofPlainText(UnframedGrpcStatusMappingFunction st try (TemporaryThreadLocals ttl = TemporaryThreadLocals.acquire()) { final StringBuilder msg = ttl.stringBuilder(); msg.append("grpc-code: ").append(grpcCode.name()); - final String grpcMessage = status.getDescription(); if (grpcMessage != null) { msg.append(", ").append(grpcMessage); } @@ -177,14 +213,6 @@ static UnframedGrpcErrorHandler ofPlainText(UnframedGrpcStatusMappingFunction st }; } - /** - * Returns a rich error JSON response based on Google APIs. - * Please refer Google error model - */ - static UnframedGrpcErrorHandler ofRichJson() { - return ofRichJson(UnframedGrpcStatusMappingFunction.of()); - } - /** * Returns a rich error JSON response based on Google APIs. * Please refer Google error model @@ -192,28 +220,19 @@ static UnframedGrpcErrorHandler ofRichJson() { * to an {@link HttpStatus} code. */ static UnframedGrpcErrorHandler ofRichJson(UnframedGrpcStatusMappingFunction statusMappingFunction) { - // Ensure that unframedGrpcStatusMappingFunction never returns null - // by falling back to the default. final UnframedGrpcStatusMappingFunction mappingFunction = requireNonNull(statusMappingFunction, "statusMappingFunction") .orElse(UnframedGrpcStatusMappingFunction.of()); - final MessageMarshaller errorDetailsMarshaller - = UnframedGrpcErrorHandlerUtils.getErrorDetailsMarshaller(); return (ctx, status, response) -> { - //final Logger logger = LoggerFactory.getLogger(UnframedGrpcErrorHandler.class); final Code grpcCode = status.getCode(); final String grpcMessage = status.getDescription(); - final RequestLogAccess log = ctx.log(); - final Throwable cause; - if (log.isAvailable(RequestLogProperty.RESPONSE_CAUSE)) { - cause = log.partial().responseCause(); - } else { - cause = null; - } + @Nullable + final Throwable cause = getThrowableFromContext(ctx); + final HttpStatus httpStatus = mappingFunction.apply(ctx, status, cause); final HttpHeaders trailers = !response.trailers().isEmpty() ? response.trailers() : response.headers(); + @Nullable final String grpcStatusDetailsBin = trailers.get(GrpcHeaderNames.GRPC_STATUS_DETAILS_BIN); - final HttpStatus httpStatus = mappingFunction.apply(ctx, status, cause); final ResponseHeaders responseHeaders = ResponseHeaders.builder(httpStatus) .contentType(MediaType.JSON_UTF_8) .addInt(GrpcHeaderNames.GRPC_STATUS, @@ -231,16 +250,14 @@ static UnframedGrpcErrorHandler ofRichJson(UnframedGrpcStatusMappingFunction sta if (!Strings.isNullOrEmpty(grpcStatusDetailsBin)) { com.google.rpc.Status rpcStatus = null; try { - rpcStatus = UnframedGrpcErrorHandlerUtils.decodeGrpcStatusDetailsBin(grpcStatusDetailsBin); + rpcStatus = decodeGrpcStatusDetailsBin(grpcStatusDetailsBin); } catch (InvalidProtocolBufferException e) { logger.warn("invalid protobuf exception happens when decode grpc-status-details-bin {}", grpcStatusDetailsBin); } if (rpcStatus != null) { try { - messageBuilder.put("details", UnframedGrpcErrorHandlerUtils - .convertErrorDetailToJsonNode(rpcStatus.getDetailsList(), - errorDetailsMarshaller)); + messageBuilder.put("details", convertErrorDetailToJsonNode(rpcStatus.getDetailsList())); } catch (IOException e) { logger.warn("error happens when convert error converting rpc status {} to strings", rpcStatus); @@ -252,4 +269,5 @@ static UnframedGrpcErrorHandler ofRichJson(UnframedGrpcStatusMappingFunction sta }; } + private DefaultUnframedGrpcErrorHandler() {} } diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java index 1d3b8f8ea1b..eea28c1663a 100644 --- a/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java @@ -16,36 +16,13 @@ package com.linecorp.armeria.server.grpc; -import static java.util.Objects.requireNonNull; - -import java.io.IOException; -import java.util.Map; - -import org.curioswitch.common.protobuf.json.MessageMarshaller; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import com.google.common.base.Strings; -import com.google.common.collect.ImmutableMap; -import com.google.protobuf.InvalidProtocolBufferException; - import com.linecorp.armeria.common.AggregatedHttpResponse; -import com.linecorp.armeria.common.HttpData; -import com.linecorp.armeria.common.HttpHeaders; import com.linecorp.armeria.common.HttpResponse; import com.linecorp.armeria.common.HttpStatus; -import com.linecorp.armeria.common.MediaType; -import com.linecorp.armeria.common.ResponseHeaders; import com.linecorp.armeria.common.annotation.UnstableApi; -import com.linecorp.armeria.common.grpc.protocol.GrpcHeaderNames; -import com.linecorp.armeria.common.logging.RequestLogAccess; -import com.linecorp.armeria.common.logging.RequestLogProperty; -import com.linecorp.armeria.common.util.Exceptions; -import com.linecorp.armeria.internal.common.util.TemporaryThreadLocals; import com.linecorp.armeria.server.ServiceRequestContext; import io.grpc.Status; -import io.grpc.Status.Code; /** * Error handler which maps a gRPC response to an {@link HttpResponse}. @@ -61,33 +38,21 @@ static UnframedGrpcErrorHandler of() { return DefaultUnframedGrpcErrorHandler.of(UnframedGrpcStatusMappingFunction.of()); } -// /** -// * Returns a plain text or json response based on the content type. -// * -// * @param statusMappingFunction The function which maps the {@link Throwable} or gRPC {@link Status} code -// * to an {@link HttpStatus} code. -// */ -// static UnframedGrpcErrorHandler of(UnframedGrpcStatusMappingFunction statusMappingFunction) { -// // Ensure that unframedGrpcStatusMappingFunction never returns null -// // by falling back to the default. -// final UnframedGrpcStatusMappingFunction mappingFunction = -// requireNonNull(statusMappingFunction, "statusMappingFunction") -// .orElse(UnframedGrpcStatusMappingFunction.of()); -// return (ctx, status, response) -> { -// final MediaType grpcMediaType = response.contentType(); -// if (grpcMediaType != null && grpcMediaType.isJson()) { -// return ofJson(mappingFunction).handle(ctx, status, response); -// } else { -// return ofPlainText(mappingFunction).handle(ctx, status, response); -// } -// }; -// } + /** + * Returns a plain text or json response based on the content type. + * + * @param statusMappingFunction The function which maps the {@link Throwable} or gRPC {@link Status} code + * to an {@link HttpStatus} code. + */ + static UnframedGrpcErrorHandler of(UnframedGrpcStatusMappingFunction statusMappingFunction) { + return DefaultUnframedGrpcErrorHandler.of(statusMappingFunction); + } /** * Returns a json response. */ static UnframedGrpcErrorHandler ofJson() { - return ofJson(UnframedGrpcStatusMappingFunction.of()); + return DefaultUnframedGrpcErrorHandler.ofJson(UnframedGrpcStatusMappingFunction.of()); } /** @@ -97,44 +62,14 @@ static UnframedGrpcErrorHandler ofJson() { * to an {@link HttpStatus} code. */ static UnframedGrpcErrorHandler ofJson(UnframedGrpcStatusMappingFunction statusMappingFunction) { - // Ensure that unframedGrpcStatusMappingFunction never returns null - // by falling back to the default. - final UnframedGrpcStatusMappingFunction mappingFunction = - requireNonNull(statusMappingFunction, "statusMappingFunction") - .orElse(UnframedGrpcStatusMappingFunction.of()); - return (ctx, status, response) -> { - final Code grpcCode = status.getCode(); - final String grpcMessage = status.getDescription(); - final RequestLogAccess log = ctx.log(); - final Throwable cause; - if (log.isAvailable(RequestLogProperty.RESPONSE_CAUSE)) { - cause = log.partial().responseCause(); - } else { - cause = null; - } - final HttpStatus httpStatus = mappingFunction.apply(ctx, status, cause); - final ResponseHeaders responseHeaders = ResponseHeaders.builder(httpStatus) - .contentType(MediaType.JSON_UTF_8) - .addInt(GrpcHeaderNames.GRPC_STATUS, - grpcCode.value()) - .build(); - final ImmutableMap.Builder messageBuilder = ImmutableMap.builder(); - messageBuilder.put("grpc-code", grpcCode.name()); - if (grpcMessage != null) { - messageBuilder.put("message", grpcMessage); - } - if (cause != null && ctx.config().verboseResponses()) { - messageBuilder.put("stack-trace", Exceptions.traceText(cause)); - } - return HttpResponse.ofJson(responseHeaders, messageBuilder.build()); - }; + return DefaultUnframedGrpcErrorHandler.ofJson(statusMappingFunction); } /** * Returns a plain text response. */ static UnframedGrpcErrorHandler ofPlainText() { - return ofPlainText(UnframedGrpcStatusMappingFunction.of()); + return DefaultUnframedGrpcErrorHandler.ofPlainText(UnframedGrpcStatusMappingFunction.of()); } /** @@ -144,41 +79,7 @@ static UnframedGrpcErrorHandler ofPlainText() { * to an {@link HttpStatus} code. */ static UnframedGrpcErrorHandler ofPlainText(UnframedGrpcStatusMappingFunction statusMappingFunction) { - // Ensure that unframedGrpcStatusMappingFunction never returns null - // by falling back to the default. - final UnframedGrpcStatusMappingFunction mappingFunction = - requireNonNull(statusMappingFunction, "statusMappingFunction") - .orElse(UnframedGrpcStatusMappingFunction.of()); - return (ctx, status, response) -> { - final Code grpcCode = status.getCode(); - final RequestLogAccess log = ctx.log(); - final Throwable cause; - if (log.isAvailable(RequestLogProperty.RESPONSE_CAUSE)) { - cause = log.partial().responseCause(); - } else { - cause = null; - } - final HttpStatus httpStatus = mappingFunction.apply(ctx, status, cause); - final ResponseHeaders responseHeaders = ResponseHeaders.builder(httpStatus) - .contentType(MediaType.PLAIN_TEXT_UTF_8) - .addInt(GrpcHeaderNames.GRPC_STATUS, - grpcCode.value()) - .build(); - final HttpData content; - try (TemporaryThreadLocals ttl = TemporaryThreadLocals.acquire()) { - final StringBuilder msg = ttl.stringBuilder(); - msg.append("grpc-code: ").append(grpcCode.name()); - final String grpcMessage = status.getDescription(); - if (grpcMessage != null) { - msg.append(", ").append(grpcMessage); - } - if (cause != null && ctx.config().verboseResponses()) { - msg.append("\nstack-trace:\n").append(Exceptions.traceText(cause)); - } - content = HttpData.ofUtf8(msg); - } - return HttpResponse.of(responseHeaders, content); - }; + return DefaultUnframedGrpcErrorHandler.ofPlainText(statusMappingFunction); } /** @@ -186,74 +87,7 @@ static UnframedGrpcErrorHandler ofPlainText(UnframedGrpcStatusMappingFunction st * Please refer Google error model */ static UnframedGrpcErrorHandler ofRichJson() { - return ofRichJson(UnframedGrpcStatusMappingFunction.of()); - } - - /** - * Returns a rich error JSON response based on Google APIs. - * Please refer Google error model - * @param statusMappingFunction The function which maps the {@link Throwable} or gRPC {@link Status} code - * to an {@link HttpStatus} code. - */ - static UnframedGrpcErrorHandler ofRichJson(UnframedGrpcStatusMappingFunction statusMappingFunction) { - // Ensure that unframedGrpcStatusMappingFunction never returns null - // by falling back to the default. - final UnframedGrpcStatusMappingFunction mappingFunction = - requireNonNull(statusMappingFunction, "statusMappingFunction") - .orElse(UnframedGrpcStatusMappingFunction.of()); - final MessageMarshaller errorDetailsMarshaller - = UnframedGrpcErrorHandlerUtils.getErrorDetailsMarshaller(); - return (ctx, status, response) -> { - final Logger logger = LoggerFactory.getLogger(UnframedGrpcErrorHandler.class); - final Code grpcCode = status.getCode(); - final String grpcMessage = status.getDescription(); - final RequestLogAccess log = ctx.log(); - final Throwable cause; - if (log.isAvailable(RequestLogProperty.RESPONSE_CAUSE)) { - cause = log.partial().responseCause(); - } else { - cause = null; - } - final HttpHeaders trailers = !response.trailers().isEmpty() ? - response.trailers() : response.headers(); - final String grpcStatusDetailsBin = trailers.get(GrpcHeaderNames.GRPC_STATUS_DETAILS_BIN); - final HttpStatus httpStatus = mappingFunction.apply(ctx, status, cause); - final ResponseHeaders responseHeaders = ResponseHeaders.builder(httpStatus) - .contentType(MediaType.JSON_UTF_8) - .addInt(GrpcHeaderNames.GRPC_STATUS, - grpcCode.value()) - .build(); - final ImmutableMap.Builder messageBuilder = ImmutableMap.builder(); - messageBuilder.put("code", httpStatus.code()); - messageBuilder.put("status", grpcCode.name()); - if (grpcMessage != null) { - messageBuilder.put("message", grpcMessage); - } - if (cause != null && ctx.config().verboseResponses()) { - messageBuilder.put("stack-trace", Exceptions.traceText(cause)); - } - if (!Strings.isNullOrEmpty(grpcStatusDetailsBin)) { - com.google.rpc.Status rpcStatus = null; - try { - rpcStatus = UnframedGrpcErrorHandlerUtils.decodeGrpcStatusDetailsBin(grpcStatusDetailsBin); - } catch (InvalidProtocolBufferException e) { - logger.warn("invalid protobuf exception happens when decode grpc-status-details-bin {}", - grpcStatusDetailsBin); - } - if (rpcStatus != null) { - try { - messageBuilder.put("details", UnframedGrpcErrorHandlerUtils - .convertErrorDetailToJsonNode(rpcStatus.getDetailsList(), - errorDetailsMarshaller)); - } catch (IOException e) { - logger.warn("error happens when convert error converting rpc status {} to strings", - rpcStatus); - } - } - } - final Map errorObject = ImmutableMap.of("error", messageBuilder.build()); - return HttpResponse.ofJson(responseHeaders, errorObject); - }; + return DefaultUnframedGrpcErrorHandler.ofRichJson(UnframedGrpcStatusMappingFunction.of()); } /** diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerUtils.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerUtils.java deleted file mode 100644 index 75e78b65b94..00000000000 --- a/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerUtils.java +++ /dev/null @@ -1,92 +0,0 @@ -/* - * Copyright 2022 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.server.grpc; - -import java.io.IOException; -import java.io.StringWriter; -import java.util.Base64; -import java.util.List; - -import org.curioswitch.common.protobuf.json.MessageMarshaller; - -import com.fasterxml.jackson.core.JsonGenerator; -import com.fasterxml.jackson.databind.JsonNode; -import com.fasterxml.jackson.databind.ObjectMapper; -import com.google.protobuf.Any; -import com.google.protobuf.InvalidProtocolBufferException; -import com.google.rpc.BadRequest; -import com.google.rpc.DebugInfo; -import com.google.rpc.ErrorInfo; -import com.google.rpc.Help; -import com.google.rpc.LocalizedMessage; -import com.google.rpc.PreconditionFailure; -import com.google.rpc.QuotaFailure; -import com.google.rpc.RequestInfo; -import com.google.rpc.ResourceInfo; -import com.google.rpc.RetryInfo; -import com.google.rpc.Status; - -import com.linecorp.armeria.internal.common.JacksonUtil; - -/** - * An Utility class which contains methods related to Unframed grpc error handler. - */ -final class UnframedGrpcErrorHandlerUtils { - - private static final MessageMarshaller ERROR_DETAILS_MARSHALLER = - MessageMarshaller.builder() - .omittingInsignificantWhitespace(true) - .register(RetryInfo.getDefaultInstance()) - .register(ErrorInfo.getDefaultInstance()) - .register(QuotaFailure.getDefaultInstance()) - .register(DebugInfo.getDefaultInstance()) - .register(PreconditionFailure.getDefaultInstance()) - .register(BadRequest.getDefaultInstance()) - .register(RequestInfo.getDefaultInstance()) - .register(ResourceInfo.getDefaultInstance()) - .register(Help.getDefaultInstance()) - .register(LocalizedMessage.getDefaultInstance()) - .build(); - - static MessageMarshaller getErrorDetailsMarshaller() { - return ERROR_DETAILS_MARSHALLER; - } - - private static final ObjectMapper mapper = JacksonUtil.newDefaultObjectMapper(); - - private UnframedGrpcErrorHandlerUtils() {} - - static JsonNode convertErrorDetailToJsonNode(List details, MessageMarshaller messageMarshaller) - throws IOException { - final StringWriter jsonObjectWriter = new StringWriter(); - try (JsonGenerator jsonGenerator = mapper.createGenerator(jsonObjectWriter)) { - jsonGenerator.writeStartArray(); - for (Any detail : details) { - messageMarshaller.writeValue(detail, jsonGenerator); - } - jsonGenerator.writeEndArray(); - jsonGenerator.flush(); - return mapper.readTree(jsonObjectWriter.toString()); - } - } - - static Status decodeGrpcStatusDetailsBin(String grpcStatusDetailsBin) - throws InvalidProtocolBufferException { - final byte[] result = Base64.getDecoder().decode(grpcStatusDetailsBin); - return Status.parseFrom(result); - } -} diff --git a/grpc/src/test/java/com/linecorp/armeria/server/grpc/GrpcStatusDetailsBinHeaderTest.java b/grpc/src/test/java/com/linecorp/armeria/server/grpc/GrpcStatusDetailsBinHeaderTest.java index d5b11cc4754..c25c4e07b0f 100644 --- a/grpc/src/test/java/com/linecorp/armeria/server/grpc/GrpcStatusDetailsBinHeaderTest.java +++ b/grpc/src/test/java/com/linecorp/armeria/server/grpc/GrpcStatusDetailsBinHeaderTest.java @@ -16,7 +16,6 @@ package com.linecorp.armeria.server.grpc; -import static com.linecorp.armeria.server.grpc.UnframedGrpcErrorHandlerUtils.decodeGrpcStatusDetailsBin; import static org.assertj.core.api.Assertions.assertThat; import org.junit.jupiter.api.Test; @@ -99,7 +98,7 @@ void googleRpcErrorDetail() throws InvalidProtocolBufferException { .post(TestServiceGrpc.getEmptyCallMethod().getFullMethodName()) .content(MediaType.PROTOBUF, Empty.getDefaultInstance().toByteArray()) .execute(); - final Status status = decodeGrpcStatusDetailsBin( + final Status status = DefaultUnframedGrpcErrorHandler.decodeGrpcStatusDetailsBin( response.headers().get(GrpcHeaderNames.GRPC_STATUS_DETAILS_BIN)); assertThat(status).isEqualTo(googleRpcStatus); assertThat(response.trailers()).isEmpty(); diff --git a/grpc/src/test/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerUtilsTest.java b/grpc/src/test/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerUtilsTest.java index 2b5fab450a8..b6733def427 100644 --- a/grpc/src/test/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerUtilsTest.java +++ b/grpc/src/test/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerUtilsTest.java @@ -112,8 +112,8 @@ void convertErrorDetailToJsonNodeTest() throws IOException { .addDetails(Any.pack(localizedMessage)) .build(); - final JsonNode jsonNode = UnframedGrpcErrorHandlerUtils.convertErrorDetailToJsonNode( - status.getDetailsList(), UnframedGrpcErrorHandlerUtils.getErrorDetailsMarshaller()); + final JsonNode jsonNode = DefaultUnframedGrpcErrorHandler.convertErrorDetailToJsonNode( + status.getDetailsList()); final String expectedJsonString = "[{\"@type\":\"type.googleapis.com/google.rpc.ErrorInfo\"," + "\"reason\":\"Unknown Exception\",\"domain\":\"test\",\"metadata\":{\"key\":\"value\"}}," + @@ -138,8 +138,7 @@ void convertErrorDetailToJsonNodeTest() throws IOException { void shouldThrowIOException() { final Empty empty = Empty.getDefaultInstance(); final Status status = Status.newBuilder().addDetails(Any.pack(empty)).build(); - assertThatThrownBy(() -> UnframedGrpcErrorHandlerUtils.convertErrorDetailToJsonNode( - status.getDetailsList(), UnframedGrpcErrorHandlerUtils.getErrorDetailsMarshaller())) - .isInstanceOf(IOException.class); + assertThatThrownBy(() -> DefaultUnframedGrpcErrorHandler.convertErrorDetailToJsonNode( + status.getDetailsList())).isInstanceOf(IOException.class); } } From 80cb34bd5c35ed6c2d93b8d485a1fd186fdd61b7 Mon Sep 17 00:00:00 2001 From: Hu Guang Date: Mon, 16 May 2022 21:36:17 +0800 Subject: [PATCH 09/36] change class name --- ... DefaultUnframedGrpcErrorHandlerTest.java} | 84 +++++++++++++++---- 1 file changed, 67 insertions(+), 17 deletions(-) rename grpc/src/test/java/com/linecorp/armeria/server/grpc/{UnframedGrpcErrorHandlerUtilsTest.java => DefaultUnframedGrpcErrorHandlerTest.java} (68%) diff --git a/grpc/src/test/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerUtilsTest.java b/grpc/src/test/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandlerTest.java similarity index 68% rename from grpc/src/test/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerUtilsTest.java rename to grpc/src/test/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandlerTest.java index b6733def427..2d3cc1bc941 100644 --- a/grpc/src/test/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerUtilsTest.java +++ b/grpc/src/test/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandlerTest.java @@ -43,7 +43,7 @@ import com.google.rpc.RetryInfo; import com.google.rpc.Status; -class UnframedGrpcErrorHandlerUtilsTest { +class DefaultUnframedGrpcErrorHandlerTest { @Test void convertErrorDetailToJsonNodeTest() throws IOException { @@ -115,22 +115,72 @@ void convertErrorDetailToJsonNodeTest() throws IOException { final JsonNode jsonNode = DefaultUnframedGrpcErrorHandler.convertErrorDetailToJsonNode( status.getDetailsList()); final String expectedJsonString = - "[{\"@type\":\"type.googleapis.com/google.rpc.ErrorInfo\"," + - "\"reason\":\"Unknown Exception\",\"domain\":\"test\",\"metadata\":{\"key\":\"value\"}}," + - "{\"@type\":\"type.googleapis.com/google.rpc.RetryInfo\",\"retryDelay\":\"100s\"}," + - "{\"@type\":\"type.googleapis.com/google.rpc.DebugInfo\"," + - "\"stackEntries\":[\"stack1\",\"stack2\"],\"detail\":\"debug info\"}," + - "{\"@type\":\"type.googleapis.com/google.rpc.QuotaFailure\"," + - "\"violations\":[{\"description\":\"quota\"}]}," + - "{\"@type\":\"type.googleapis.com/google.rpc.PreconditionFailure\"," + - "\"violations\":[{\"description\":\"violation\"}]}," + - "{\"@type\":\"type.googleapis.com/google.rpc.BadRequest\"," + - "\"fieldViolations\":[{\"description\":\"field violation\"}]}," + - "{\"@type\":\"type.googleapis.com/google.rpc.RequestInfo\",\"requestId\":\"requestid\"}," + - "{\"@type\":\"type.googleapis.com/google.rpc.ResourceInfo\",\"description\":\"description\"}," + - "{\"@type\":\"type.googleapis.com/google.rpc.Help\"," + - "\"links\":[{\"description\":\"descrption\"}]}," + - "{\"@type\":\"type.googleapis.com/google.rpc.LocalizedMessage\",\"message\":\"message\"}]"; + "[\n" + + " {\n" + + " \"@type\":\"type.googleapis.com/google.rpc.ErrorInfo\",\n" + + " \"reason\":\"Unknown Exception\",\n" + + " \"domain\":\"test\",\n" + + " \"metadata\":{\n" + + " \"key\":\"value\"\n" + + " }\n" + + " },\n" + + " {\n" + + " \"@type\":\"type.googleapis.com/google.rpc.RetryInfo\",\n" + + " \"retryDelay\":\"100s\"\n" + + " },\n" + + " {\n" + + " \"@type\":\"type.googleapis.com/google.rpc.DebugInfo\",\n" + + " \"stackEntries\":[\n" + + " \"stack1\",\n" + + " \"stack2\"\n" + + " ],\n" + + " \"detail\":\"debug info\"\n" + + " },\n" + + " {\n" + + " \"@type\":\"type.googleapis.com/google.rpc.QuotaFailure\",\n" + + " \"violations\":[\n" + + " {\n" + + " \"description\":\"quota\"\n" + + " }\n" + + " ]\n" + + " },\n" + + " {\n" + + " \"@type\":\"type.googleapis.com/google.rpc.PreconditionFailure\",\n" + + " \"violations\":[\n" + + " {\n" + + " \"description\":\"violation\"\n" + + " }\n" + + " ]\n" + + " },\n" + + " {\n" + + " \"@type\":\"type.googleapis.com/google.rpc.BadRequest\",\n" + + " \"fieldViolations\":[\n" + + " {\n" + + " \"description\":\"field violation\"\n" + + " }\n" + + " ]\n" + + " },\n" + + " {\n" + + " \"@type\":\"type.googleapis.com/google.rpc.RequestInfo\",\n" + + " \"requestId\":\"requestid\"\n" + + " },\n" + + " {\n" + + " \"@type\":\"type.googleapis.com/google.rpc.ResourceInfo\",\n" + + " \"description\":\"description\"\n" + + " },\n" + + " {\n" + + " \"@type\":\"type.googleapis.com/google.rpc.Help\",\n" + + " \"links\":[\n" + + " {\n" + + " \"description\":\"descrption\"\n" + + " }\n" + + " ]\n" + + " },\n" + + " {\n" + + " \"@type\":\"type.googleapis.com/google.rpc.LocalizedMessage\",\n" + + " \"message\":\"message\"\n" + + " }\n" + + "]"; assertThatJson(jsonNode).isEqualTo(expectedJsonString); } From 05af491ba6ae04d6dea47dd02b2bab8f8633e0e6 Mon Sep 17 00:00:00 2001 From: Hu Guang Date: Mon, 16 May 2022 21:47:27 +0800 Subject: [PATCH 10/36] add not null annotation --- .../armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java index b216354ac8d..50c4c2a433f 100644 --- a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2021 LINE Corporation + * Copyright 2022 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 @@ -156,6 +156,7 @@ static UnframedGrpcErrorHandler ofJson(UnframedGrpcStatusMappingFunction statusM = ofStatusMappingFunction(statusMappingFunction); return (ctx, status, response) -> { final Code grpcCode = status.getCode(); + @Nullable final String grpcMessage = status.getDescription(); @Nullable final Throwable cause = getThrowableFromContext(ctx); @@ -188,6 +189,7 @@ static UnframedGrpcErrorHandler ofPlainText(UnframedGrpcStatusMappingFunction st = ofStatusMappingFunction(statusMappingFunction); return (ctx, status, response) -> { final Code grpcCode = status.getCode(); + @Nullable final String grpcMessage = status.getDescription(); @Nullable final Throwable cause = getThrowableFromContext(ctx); @@ -225,6 +227,7 @@ static UnframedGrpcErrorHandler ofRichJson(UnframedGrpcStatusMappingFunction sta .orElse(UnframedGrpcStatusMappingFunction.of()); return (ctx, status, response) -> { final Code grpcCode = status.getCode(); + @Nullable final String grpcMessage = status.getDescription(); @Nullable final Throwable cause = getThrowableFromContext(ctx); From ce42076d3cd68fbc2a49f67ce579674072884810 Mon Sep 17 00:00:00 2001 From: Hu Guang Date: Tue, 14 Jun 2022 00:06:11 +0800 Subject: [PATCH 11/36] Update grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java Co-authored-by: Trustin Lee --- .../armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java index 50c4c2a433f..9b9e8676702 100644 --- a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java @@ -127,7 +127,7 @@ private static Throwable getThrowableFromContext(ServiceRequestContext ctx) { } /** - * Returns a plain text or json response based on the content type. + * Returns a plaintext or JSON response based on the content type. * * @param statusMappingFunction The function which maps the {@link Throwable} or gRPC {@link Status} code * to an {@link HttpStatus} code. From 4649b2bb5f7aef16897f8eddc0a94b80a3fd8ad7 Mon Sep 17 00:00:00 2001 From: Hu Guang Date: Tue, 14 Jun 2022 00:06:30 +0800 Subject: [PATCH 12/36] Update grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java Co-authored-by: Trustin Lee --- .../armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java index 9b9e8676702..060629554a2 100644 --- a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java @@ -146,7 +146,7 @@ static UnframedGrpcErrorHandler of(UnframedGrpcStatusMappingFunction statusMappi } /** - * Returns a json response. + * Returns a JSON response. * * @param statusMappingFunction The function which maps the {@link Throwable} or gRPC {@link Status} code * to an {@link HttpStatus} code. From a72032f70da6c55fe4e54f324f4ee31f5a1905c3 Mon Sep 17 00:00:00 2001 From: Hu Guang Date: Tue, 14 Jun 2022 00:06:42 +0800 Subject: [PATCH 13/36] Update grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java Co-authored-by: Trustin Lee --- .../armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java index 060629554a2..20706f0e921 100644 --- a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java @@ -179,7 +179,7 @@ static UnframedGrpcErrorHandler ofJson(UnframedGrpcStatusMappingFunction statusM } /** - * Returns a plain text response. + * Returns a plaintext response. * * @param statusMappingFunction The function which maps the {@link Throwable} or gRPC {@link Status} code * to an {@link HttpStatus} code. From 48b28a77bb16ee92ce007de6c4cdca6d26f03954 Mon Sep 17 00:00:00 2001 From: Hu Guang Date: Tue, 14 Jun 2022 20:29:24 +0800 Subject: [PATCH 14/36] Update grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java Co-authored-by: Trustin Lee --- .../armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java index 20706f0e921..2f8887bc5e6 100644 --- a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java @@ -255,7 +255,7 @@ static UnframedGrpcErrorHandler ofRichJson(UnframedGrpcStatusMappingFunction sta try { rpcStatus = decodeGrpcStatusDetailsBin(grpcStatusDetailsBin); } catch (InvalidProtocolBufferException e) { - logger.warn("invalid protobuf exception happens when decode grpc-status-details-bin {}", + logger.warn("Unexpected exception while decoding grpc-status-details-bin: {}", grpcStatusDetailsBin); } if (rpcStatus != null) { From 953e05788e90168c65452cf28a62cd82bbeaf6fb Mon Sep 17 00:00:00 2001 From: Hu Guang Date: Tue, 14 Jun 2022 20:30:15 +0800 Subject: [PATCH 15/36] Update grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java Co-authored-by: Trustin Lee --- .../linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java index eea28c1663a..bc2d008033f 100644 --- a/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java @@ -49,7 +49,7 @@ static UnframedGrpcErrorHandler of(UnframedGrpcStatusMappingFunction statusMappi } /** - * Returns a json response. + * Returns a JSON response. */ static UnframedGrpcErrorHandler ofJson() { return DefaultUnframedGrpcErrorHandler.ofJson(UnframedGrpcStatusMappingFunction.of()); From 117d2439baf3b3519fa02d87226c7f345221e7f5 Mon Sep 17 00:00:00 2001 From: huguang Date: Tue, 14 Jun 2022 20:30:40 +0800 Subject: [PATCH 16/36] some change --- .../server/grpc/DefaultUnframedGrpcErrorHandler.java | 6 +++--- .../armeria/server/grpc/UnframedGrpcErrorHandler.java | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java index 20706f0e921..70e762c6fcb 100644 --- a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java @@ -140,7 +140,7 @@ static UnframedGrpcErrorHandler of(UnframedGrpcStatusMappingFunction statusMappi if (grpcMediaType != null && grpcMediaType.isJson()) { return ofJson(mappingFunction).handle(ctx, status, response); } else { - return ofPlainText(mappingFunction).handle(ctx, status, response); + return ofPlaintext(mappingFunction).handle(ctx, status, response); } }; } @@ -184,7 +184,7 @@ static UnframedGrpcErrorHandler ofJson(UnframedGrpcStatusMappingFunction statusM * @param statusMappingFunction The function which maps the {@link Throwable} or gRPC {@link Status} code * to an {@link HttpStatus} code. */ - static UnframedGrpcErrorHandler ofPlainText(UnframedGrpcStatusMappingFunction statusMappingFunction) { + static UnframedGrpcErrorHandler ofPlaintext(UnframedGrpcStatusMappingFunction statusMappingFunction) { final UnframedGrpcStatusMappingFunction mappingFunction = ofStatusMappingFunction(statusMappingFunction); return (ctx, status, response) -> { @@ -262,7 +262,7 @@ static UnframedGrpcErrorHandler ofRichJson(UnframedGrpcStatusMappingFunction sta try { messageBuilder.put("details", convertErrorDetailToJsonNode(rpcStatus.getDetailsList())); } catch (IOException e) { - logger.warn("error happens when convert error converting rpc status {} to strings", + logger.warn("Unexpected exception while converting RPC status {} to strings", rpcStatus); } } diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java index eea28c1663a..ac82892dec1 100644 --- a/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java @@ -69,7 +69,7 @@ static UnframedGrpcErrorHandler ofJson(UnframedGrpcStatusMappingFunction statusM * Returns a plain text response. */ static UnframedGrpcErrorHandler ofPlainText() { - return DefaultUnframedGrpcErrorHandler.ofPlainText(UnframedGrpcStatusMappingFunction.of()); + return DefaultUnframedGrpcErrorHandler.ofPlaintext(UnframedGrpcStatusMappingFunction.of()); } /** @@ -79,7 +79,7 @@ static UnframedGrpcErrorHandler ofPlainText() { * to an {@link HttpStatus} code. */ static UnframedGrpcErrorHandler ofPlainText(UnframedGrpcStatusMappingFunction statusMappingFunction) { - return DefaultUnframedGrpcErrorHandler.ofPlainText(statusMappingFunction); + return DefaultUnframedGrpcErrorHandler.ofPlaintext(statusMappingFunction); } /** From 6ffe02e7249c291cef0f1fa82ca8cbb377713dd8 Mon Sep 17 00:00:00 2001 From: huguang Date: Tue, 14 Jun 2022 21:03:10 +0800 Subject: [PATCH 17/36] minor change --- .../armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java index 00f3bd06f8e..8a977a28534 100644 --- a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java @@ -248,7 +248,6 @@ static UnframedGrpcErrorHandler ofRichJson(UnframedGrpcStatusMappingFunction sta grpcCode.value()) .build(); final ImmutableMap.Builder messageBuilder = ImmutableMap.builder(); - messageBuilder.put("code", httpStatus.code()); messageBuilder.put("status", grpcCode.name()); if (grpcMessage != null) { messageBuilder.put("message", grpcMessage); @@ -266,6 +265,7 @@ static UnframedGrpcErrorHandler ofRichJson(UnframedGrpcStatusMappingFunction sta } if (rpcStatus != null) { try { + messageBuilder.put("code", rpcStatus.getCode()); messageBuilder.put("details", convertErrorDetailToJsonNode(rpcStatus.getDetailsList())); } catch (IOException e) { logger.warn("Unexpected exception while converting RPC status {} to strings", From 576dc5e7eca7f753f9e2f5564bcf298c954d9f06 Mon Sep 17 00:00:00 2001 From: huguang Date: Tue, 14 Jun 2022 21:19:50 +0800 Subject: [PATCH 18/36] minor change --- .../armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java index 8a977a28534..b22b2c2bcf0 100644 --- a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java @@ -64,6 +64,7 @@ import io.grpc.Status; import io.grpc.Status.Code; +import io.netty.buffer.ByteBuf; final class DefaultUnframedGrpcErrorHandler { @@ -232,6 +233,7 @@ static UnframedGrpcErrorHandler ofRichJson(UnframedGrpcStatusMappingFunction sta requireNonNull(statusMappingFunction, "statusMappingFunction") .orElse(UnframedGrpcStatusMappingFunction.of()); return (ctx, status, response) -> { + final ByteBuf buffer = ctx.alloc().buffer(); final Code grpcCode = status.getCode(); @Nullable final String grpcMessage = status.getDescription(); @@ -248,6 +250,7 @@ static UnframedGrpcErrorHandler ofRichJson(UnframedGrpcStatusMappingFunction sta grpcCode.value()) .build(); final ImmutableMap.Builder messageBuilder = ImmutableMap.builder(); + messageBuilder.put("code", httpStatus.code()); messageBuilder.put("status", grpcCode.name()); if (grpcMessage != null) { messageBuilder.put("message", grpcMessage); @@ -265,7 +268,6 @@ static UnframedGrpcErrorHandler ofRichJson(UnframedGrpcStatusMappingFunction sta } if (rpcStatus != null) { try { - messageBuilder.put("code", rpcStatus.getCode()); messageBuilder.put("details", convertErrorDetailToJsonNode(rpcStatus.getDetailsList())); } catch (IOException e) { logger.warn("Unexpected exception while converting RPC status {} to strings", From cccd0e1c247bde27c1241fd916ee13702e0e7778 Mon Sep 17 00:00:00 2001 From: huguang Date: Thu, 16 Jun 2022 18:23:13 +0800 Subject: [PATCH 19/36] resolve some comment --- .../grpc/DefaultUnframedGrpcErrorHandler.java | 85 ++++++++++--------- .../DefaultUnframedGrpcErrorHandlerTest.java | 25 ++++-- 2 files changed, 62 insertions(+), 48 deletions(-) diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java index b22b2c2bcf0..9dcce977e8a 100644 --- a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java @@ -19,17 +19,15 @@ import static java.util.Objects.requireNonNull; import java.io.IOException; -import java.io.StringWriter; +import java.io.OutputStream; import java.util.Base64; import java.util.List; -import java.util.Map; import org.curioswitch.common.protobuf.json.MessageMarshaller; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import com.fasterxml.jackson.core.JsonGenerator; -import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Strings; @@ -65,6 +63,7 @@ import io.grpc.Status; import io.grpc.Status.Code; import io.netty.buffer.ByteBuf; +import io.netty.buffer.ByteBufOutputStream; final class DefaultUnframedGrpcErrorHandler { @@ -100,18 +99,16 @@ private static UnframedGrpcStatusMappingFunction ofStatusMappingFunction( } @VisibleForTesting - static JsonNode convertErrorDetailToJsonNode(List details) - throws IOException { - final StringWriter jsonObjectWriter = new StringWriter(); - try (JsonGenerator jsonGenerator = mapper.createGenerator(jsonObjectWriter)) { - jsonGenerator.writeStartArray(); + static void writeErrorDetails(List details, JsonGenerator jsonGenerator) throws IOException { + jsonGenerator.writeStartArray(); + try { for (Any detail : details) { ERROR_DETAILS_MARSHALLER.writeValue(detail, jsonGenerator); } - jsonGenerator.writeEndArray(); - jsonGenerator.flush(); - return mapper.readTree(jsonObjectWriter.toString()); + } catch (IOException e) { + logger.warn("Unexpected exception while writing error details {} to json", details); } + jsonGenerator.writeEndArray(); } @VisibleForTesting @@ -163,9 +160,7 @@ static UnframedGrpcErrorHandler ofJson(UnframedGrpcStatusMappingFunction statusM = ofStatusMappingFunction(statusMappingFunction); return (ctx, status, response) -> { final Code grpcCode = status.getCode(); - @Nullable final String grpcMessage = status.getDescription(); - @Nullable final Throwable cause = getThrowableFromContext(ctx); final HttpStatus httpStatus = mappingFunction.apply(ctx, status, cause); final ResponseHeaders responseHeaders = ResponseHeaders.builder(httpStatus) @@ -196,9 +191,7 @@ static UnframedGrpcErrorHandler ofPlaintext(UnframedGrpcStatusMappingFunction st = ofStatusMappingFunction(statusMappingFunction); return (ctx, status, response) -> { final Code grpcCode = status.getCode(); - @Nullable final String grpcMessage = status.getDescription(); - @Nullable final Throwable cause = getThrowableFromContext(ctx); final HttpStatus httpStatus = mappingFunction.apply(ctx, status, cause); final ResponseHeaders responseHeaders = ResponseHeaders.builder(httpStatus) @@ -234,49 +227,59 @@ static UnframedGrpcErrorHandler ofRichJson(UnframedGrpcStatusMappingFunction sta .orElse(UnframedGrpcStatusMappingFunction.of()); return (ctx, status, response) -> { final ByteBuf buffer = ctx.alloc().buffer(); + final Code grpcCode = status.getCode(); - @Nullable final String grpcMessage = status.getDescription(); - @Nullable final Throwable cause = getThrowableFromContext(ctx); final HttpStatus httpStatus = mappingFunction.apply(ctx, status, cause); final HttpHeaders trailers = !response.trailers().isEmpty() ? response.trailers() : response.headers(); - @Nullable final String grpcStatusDetailsBin = trailers.get(GrpcHeaderNames.GRPC_STATUS_DETAILS_BIN); final ResponseHeaders responseHeaders = ResponseHeaders.builder(httpStatus) .contentType(MediaType.JSON_UTF_8) .addInt(GrpcHeaderNames.GRPC_STATUS, grpcCode.value()) .build(); - final ImmutableMap.Builder messageBuilder = ImmutableMap.builder(); - messageBuilder.put("code", httpStatus.code()); - messageBuilder.put("status", grpcCode.name()); - if (grpcMessage != null) { - messageBuilder.put("message", grpcMessage); - } - if (cause != null && ctx.config().verboseResponses()) { - messageBuilder.put("stack-trace", Exceptions.traceText(cause)); - } - if (!Strings.isNullOrEmpty(grpcStatusDetailsBin)) { - com.google.rpc.Status rpcStatus = null; - try { - rpcStatus = decodeGrpcStatusDetailsBin(grpcStatusDetailsBin); - } catch (InvalidProtocolBufferException e) { - logger.warn("Unexpected exception while decoding grpc-status-details-bin: {}", - grpcStatusDetailsBin); + boolean success = false; + try (OutputStream outputStream = new ByteBufOutputStream(buffer); + JsonGenerator jsonGenerator = mapper.createGenerator(outputStream)) { + jsonGenerator.writeStartObject(); + jsonGenerator.writeFieldName("error"); + jsonGenerator.writeStartObject(); + jsonGenerator.writeNumberField("code", httpStatus.code()); + jsonGenerator.writeStringField("status", grpcCode.name()); + if (grpcMessage != null) { + jsonGenerator.writeStringField("message", grpcMessage); + } + if (cause != null && ctx.config().verboseResponses()) { + jsonGenerator.writeStringField("stack-trace", Exceptions.traceText(cause)); } - if (rpcStatus != null) { + if (!Strings.isNullOrEmpty(grpcStatusDetailsBin)) { + com.google.rpc.Status rpcStatus = null; try { - messageBuilder.put("details", convertErrorDetailToJsonNode(rpcStatus.getDetailsList())); - } catch (IOException e) { - logger.warn("Unexpected exception while converting RPC status {} to strings", - rpcStatus); + rpcStatus = decodeGrpcStatusDetailsBin(grpcStatusDetailsBin); + } catch (InvalidProtocolBufferException e) { + logger.warn("Unexpected exception while decoding grpc-status-details-bin: {}", + grpcStatusDetailsBin); + } + if (rpcStatus != null) { + jsonGenerator.writeFieldName("details"); + jsonGenerator.writeStartArray(); + writeErrorDetails(rpcStatus.getDetailsList(), jsonGenerator); } } + jsonGenerator.writeEndObject(); + jsonGenerator.writeEndObject(); + jsonGenerator.flush(); + success = true; + } catch (IOException e) { + logger.warn("Unexpected exception while generating json response"); + } finally { + if (!success) { + buffer.release(); + } } - final Map errorObject = ImmutableMap.of("error", messageBuilder.build()); - return HttpResponse.ofJson(responseHeaders, errorObject); + return HttpResponse.of(responseHeaders, HttpData.wrap(buffer)); }; } diff --git a/grpc/src/test/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandlerTest.java b/grpc/src/test/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandlerTest.java index 2d3cc1bc941..923435b993a 100644 --- a/grpc/src/test/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandlerTest.java +++ b/grpc/src/test/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandlerTest.java @@ -20,10 +20,12 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import java.io.IOException; +import java.io.StringWriter; import org.junit.jupiter.api.Test; -import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.core.JsonGenerator; +import com.fasterxml.jackson.databind.ObjectMapper; import com.google.protobuf.Any; import com.google.protobuf.Duration; import com.google.protobuf.Empty; @@ -43,8 +45,12 @@ import com.google.rpc.RetryInfo; import com.google.rpc.Status; +import com.linecorp.armeria.internal.common.JacksonUtil; + class DefaultUnframedGrpcErrorHandlerTest { + private static final ObjectMapper mapper = JacksonUtil.newDefaultObjectMapper(); + @Test void convertErrorDetailToJsonNodeTest() throws IOException { final ErrorInfo errorInfo = ErrorInfo.newBuilder() @@ -112,8 +118,10 @@ void convertErrorDetailToJsonNodeTest() throws IOException { .addDetails(Any.pack(localizedMessage)) .build(); - final JsonNode jsonNode = DefaultUnframedGrpcErrorHandler.convertErrorDetailToJsonNode( - status.getDetailsList()); + final StringWriter jsonObjectWriter = new StringWriter(); + final JsonGenerator jsonGenerator = mapper.createGenerator(jsonObjectWriter); + DefaultUnframedGrpcErrorHandler.writeErrorDetails(status.getDetailsList(), jsonGenerator); + jsonGenerator.flush(); final String expectedJsonString = "[\n" + " {\n" + @@ -181,14 +189,17 @@ void convertErrorDetailToJsonNodeTest() throws IOException { " \"message\":\"message\"\n" + " }\n" + "]"; - assertThatJson(jsonNode).isEqualTo(expectedJsonString); + assertThatJson(mapper.readTree(jsonObjectWriter.toString())).isEqualTo(expectedJsonString); } @Test - void shouldThrowIOException() { + void shouldThrowIOException() throws IOException { final Empty empty = Empty.getDefaultInstance(); final Status status = Status.newBuilder().addDetails(Any.pack(empty)).build(); - assertThatThrownBy(() -> DefaultUnframedGrpcErrorHandler.convertErrorDetailToJsonNode( - status.getDetailsList())).isInstanceOf(IOException.class); + final StringWriter jsonObjectWriter = new StringWriter(); + final JsonGenerator jsonGenerator = mapper.createGenerator(jsonObjectWriter); + + assertThatThrownBy(() -> DefaultUnframedGrpcErrorHandler.writeErrorDetails( + status.getDetailsList(), jsonGenerator)).isInstanceOf(IOException.class); } } From ae09d9d88c7ea2cf99645779771a23bda01c5a1e Mon Sep 17 00:00:00 2001 From: huguang Date: Thu, 16 Jun 2022 18:44:06 +0800 Subject: [PATCH 20/36] resolve some comment --- .../armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java | 2 +- .../armeria/server/grpc/UnframedGrpcErrorHandlerTest.java | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java index 9dcce977e8a..b567168e53b 100644 --- a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java @@ -262,7 +262,7 @@ static UnframedGrpcErrorHandler ofRichJson(UnframedGrpcStatusMappingFunction sta logger.warn("Unexpected exception while decoding grpc-status-details-bin: {}", grpcStatusDetailsBin); } - if (rpcStatus != null) { + if (rpcStatus != null && rpcStatus.getDetailsCount() > 0) { jsonGenerator.writeFieldName("details"); jsonGenerator.writeStartArray(); writeErrorDetails(rpcStatus.getDetailsList(), jsonGenerator); diff --git a/grpc/src/test/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerTest.java b/grpc/src/test/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerTest.java index f1647424dd0..e41ffbaa25a 100644 --- a/grpc/src/test/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerTest.java +++ b/grpc/src/test/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerTest.java @@ -19,6 +19,8 @@ import static net.javacrumbs.jsonunit.fluent.JsonFluentAssert.assertThatJson; import static org.assertj.core.api.Assertions.assertThat; +import java.time.Duration; + import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; From a49e3d91e25ed0dbf48e251ad5f81b64e59b4fb6 Mon Sep 17 00:00:00 2001 From: huguang Date: Thu, 16 Jun 2022 18:48:52 +0800 Subject: [PATCH 21/36] resolve some comment --- .../armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java | 1 - 1 file changed, 1 deletion(-) diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java index b567168e53b..6c808349465 100644 --- a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java @@ -264,7 +264,6 @@ static UnframedGrpcErrorHandler ofRichJson(UnframedGrpcStatusMappingFunction sta } if (rpcStatus != null && rpcStatus.getDetailsCount() > 0) { jsonGenerator.writeFieldName("details"); - jsonGenerator.writeStartArray(); writeErrorDetails(rpcStatus.getDetailsList(), jsonGenerator); } } From 16697f67edccf01981af7466846b55e201da1592 Mon Sep 17 00:00:00 2001 From: huguang Date: Thu, 16 Jun 2022 20:02:39 +0800 Subject: [PATCH 22/36] resolve some comment --- .../armeria/server/grpc/UnframedGrpcErrorHandlerTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/grpc/src/test/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerTest.java b/grpc/src/test/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerTest.java index e41ffbaa25a..f1647424dd0 100644 --- a/grpc/src/test/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerTest.java +++ b/grpc/src/test/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerTest.java @@ -19,8 +19,6 @@ import static net.javacrumbs.jsonunit.fluent.JsonFluentAssert.assertThatJson; import static org.assertj.core.api.Assertions.assertThat; -import java.time.Duration; - import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; From 1a1298c1f91bb03491ea65117bca4d2ac440ab7c Mon Sep 17 00:00:00 2001 From: huguang Date: Fri, 17 Jun 2022 10:56:25 +0800 Subject: [PATCH 23/36] change exception location --- .../server/grpc/DefaultUnframedGrpcErrorHandler.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java index 6c808349465..0bb75e832d4 100644 --- a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java @@ -101,12 +101,12 @@ private static UnframedGrpcStatusMappingFunction ofStatusMappingFunction( @VisibleForTesting static void writeErrorDetails(List details, JsonGenerator jsonGenerator) throws IOException { jsonGenerator.writeStartArray(); - try { - for (Any detail : details) { + for (Any detail : details) { + try { ERROR_DETAILS_MARSHALLER.writeValue(detail, jsonGenerator); + } catch (IOException e) { + logger.warn("Unexpected exception while writing error detail {} to json", detail); } - } catch (IOException e) { - logger.warn("Unexpected exception while writing error details {} to json", details); } jsonGenerator.writeEndArray(); } From e82d1f3c05417d2afbaaf506f3b1b1961cd887f7 Mon Sep 17 00:00:00 2001 From: huguang Date: Fri, 17 Jun 2022 10:57:35 +0800 Subject: [PATCH 24/36] change exception location --- .../armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java | 1 - 1 file changed, 1 deletion(-) diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java index 0bb75e832d4..1ca45e7f731 100644 --- a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java @@ -227,7 +227,6 @@ static UnframedGrpcErrorHandler ofRichJson(UnframedGrpcStatusMappingFunction sta .orElse(UnframedGrpcStatusMappingFunction.of()); return (ctx, status, response) -> { final ByteBuf buffer = ctx.alloc().buffer(); - final Code grpcCode = status.getCode(); final String grpcMessage = status.getDescription(); final Throwable cause = getThrowableFromContext(ctx); From abb80a9546a82cfced5e413500b528287e6e578d Mon Sep 17 00:00:00 2001 From: huguang Date: Wed, 22 Jun 2022 19:42:03 +0800 Subject: [PATCH 25/36] change response code to grpc.code --- .../armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java | 3 +-- .../armeria/server/grpc/UnframedGrpcErrorHandlerTest.java | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java index 1ca45e7f731..056737cf581 100644 --- a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java @@ -245,8 +245,7 @@ static UnframedGrpcErrorHandler ofRichJson(UnframedGrpcStatusMappingFunction sta jsonGenerator.writeStartObject(); jsonGenerator.writeFieldName("error"); jsonGenerator.writeStartObject(); - jsonGenerator.writeNumberField("code", httpStatus.code()); - jsonGenerator.writeStringField("status", grpcCode.name()); + jsonGenerator.writeNumberField("code", grpcCode.value()); if (grpcMessage != null) { jsonGenerator.writeStringField("message", grpcMessage); } diff --git a/grpc/src/test/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerTest.java b/grpc/src/test/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerTest.java index f1647424dd0..033c03b79f9 100644 --- a/grpc/src/test/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerTest.java +++ b/grpc/src/test/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerTest.java @@ -176,7 +176,7 @@ void richJson() throws JsonProcessingException { .execute(); assertThat(response.status()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR); assertThatJson(mapper.readTree(response.contentUtf8())).isEqualTo( - "{\"error\":{\"code\":500,\"status\":\"UNKNOWN\"," + + "{\"error\":{\"code\":2," + "\"message\":\"Unknown Exceptions Test\"," + "\"details\":[{\"@type\":\"type.googleapis.com/google.rpc.ErrorInfo\"," + "\"reason\":\"Unknown Exception\",\"domain\":\"test\"}]}}"); From 4b0c93e0a573e10a1f52176687647493ac1dace4 Mon Sep 17 00:00:00 2001 From: huguang Date: Wed, 22 Jun 2022 19:50:11 +0800 Subject: [PATCH 26/36] to make the error message compatible with other platform --- .../armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java index 056737cf581..8c5739dd9bc 100644 --- a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java @@ -260,7 +260,7 @@ static UnframedGrpcErrorHandler ofRichJson(UnframedGrpcStatusMappingFunction sta logger.warn("Unexpected exception while decoding grpc-status-details-bin: {}", grpcStatusDetailsBin); } - if (rpcStatus != null && rpcStatus.getDetailsCount() > 0) { + if (rpcStatus != null) { jsonGenerator.writeFieldName("details"); writeErrorDetails(rpcStatus.getDetailsList(), jsonGenerator); } From b182829d3701f77dad00fae849c78dfb57a47038 Mon Sep 17 00:00:00 2001 From: huguang Date: Thu, 23 Jun 2022 13:50:03 +0800 Subject: [PATCH 27/36] move utility method to the bottom of the class --- .../grpc/DefaultUnframedGrpcErrorHandler.java | 89 ++++++++++--------- 1 file changed, 45 insertions(+), 44 deletions(-) diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java index 8c5739dd9bc..14ba004472b 100644 --- a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java @@ -86,50 +86,6 @@ final class DefaultUnframedGrpcErrorHandler { private static final ObjectMapper mapper = JacksonUtil.newDefaultObjectMapper(); - /** - * Ensure that unframedGrpcStatusMappingFunction never returns null by falling back to the default. - */ - private static UnframedGrpcStatusMappingFunction ofStatusMappingFunction( - UnframedGrpcStatusMappingFunction statusMappingFunction) { - requireNonNull(statusMappingFunction, "statusMappingFunction"); - if (statusMappingFunction == UnframedGrpcStatusMappingFunction.of()) { - return statusMappingFunction; - } - return statusMappingFunction.orElse(UnframedGrpcStatusMappingFunction.of()); - } - - @VisibleForTesting - static void writeErrorDetails(List details, JsonGenerator jsonGenerator) throws IOException { - jsonGenerator.writeStartArray(); - for (Any detail : details) { - try { - ERROR_DETAILS_MARSHALLER.writeValue(detail, jsonGenerator); - } catch (IOException e) { - logger.warn("Unexpected exception while writing error detail {} to json", detail); - } - } - jsonGenerator.writeEndArray(); - } - - @VisibleForTesting - static com.google.rpc.Status decodeGrpcStatusDetailsBin(String grpcStatusDetailsBin) - throws InvalidProtocolBufferException { - final byte[] result = Base64.getDecoder().decode(grpcStatusDetailsBin); - return com.google.rpc.Status.parseFrom(result); - } - - @Nullable - private static Throwable getThrowableFromContext(ServiceRequestContext ctx) { - final RequestLogAccess log = ctx.log(); - final Throwable cause; - if (log.isAvailable(RequestLogProperty.RESPONSE_CAUSE)) { - cause = log.partial().responseCause(); - } else { - cause = null; - } - return cause; - } - /** * Returns a plaintext or JSON response based on the content type. * @@ -281,4 +237,49 @@ static UnframedGrpcErrorHandler ofRichJson(UnframedGrpcStatusMappingFunction sta } private DefaultUnframedGrpcErrorHandler() {} + + /** + * Ensure that unframedGrpcStatusMappingFunction never returns null by falling back to the default. + */ + private static UnframedGrpcStatusMappingFunction ofStatusMappingFunction( + UnframedGrpcStatusMappingFunction statusMappingFunction) { + requireNonNull(statusMappingFunction, "statusMappingFunction"); + if (statusMappingFunction == UnframedGrpcStatusMappingFunction.of()) { + return statusMappingFunction; + } + return statusMappingFunction.orElse(UnframedGrpcStatusMappingFunction.of()); + } + + @VisibleForTesting + static void writeErrorDetails(List details, JsonGenerator jsonGenerator) throws IOException { + jsonGenerator.writeStartArray(); + for (Any detail : details) { + try { + ERROR_DETAILS_MARSHALLER.writeValue(detail, jsonGenerator); + } catch (IOException e) { + logger.warn("Unexpected exception while writing error detail {} to json", detail); + } + } + jsonGenerator.writeEndArray(); + } + + @VisibleForTesting + static com.google.rpc.Status decodeGrpcStatusDetailsBin(String grpcStatusDetailsBin) + throws InvalidProtocolBufferException { + final byte[] result = Base64.getDecoder().decode(grpcStatusDetailsBin); + return com.google.rpc.Status.parseFrom(result); + } + + @Nullable + private static Throwable getThrowableFromContext(ServiceRequestContext ctx) { + final RequestLogAccess log = ctx.log(); + final Throwable cause; + if (log.isAvailable(RequestLogProperty.RESPONSE_CAUSE)) { + cause = log.partial().responseCause(); + } else { + cause = null; + } + return cause; + } + } From bff944c091179a5407b62d14049502f780a2acde Mon Sep 17 00:00:00 2001 From: huguang Date: Thu, 23 Jun 2022 16:15:33 +0800 Subject: [PATCH 28/36] checkstyle --- .../armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java | 1 - 1 file changed, 1 deletion(-) diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java index 14ba004472b..6bc44857fc4 100644 --- a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java @@ -281,5 +281,4 @@ private static Throwable getThrowableFromContext(ServiceRequestContext ctx) { } return cause; } - } From a9fce6816e31523063ba27765738e04601ba016d Mon Sep 17 00:00:00 2001 From: huguang Date: Mon, 25 Jul 2022 19:29:10 +0800 Subject: [PATCH 29/36] address the comments --- .../server/grpc/DefaultUnframedGrpcErrorHandler.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java index 6bc44857fc4..6d96110f951 100644 --- a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java @@ -214,7 +214,7 @@ static UnframedGrpcErrorHandler ofRichJson(UnframedGrpcStatusMappingFunction sta rpcStatus = decodeGrpcStatusDetailsBin(grpcStatusDetailsBin); } catch (InvalidProtocolBufferException e) { logger.warn("Unexpected exception while decoding grpc-status-details-bin: {}", - grpcStatusDetailsBin); + grpcStatusDetailsBin, e); } if (rpcStatus != null) { jsonGenerator.writeFieldName("details"); @@ -226,13 +226,17 @@ static UnframedGrpcErrorHandler ofRichJson(UnframedGrpcStatusMappingFunction sta jsonGenerator.flush(); success = true; } catch (IOException e) { - logger.warn("Unexpected exception while generating json response"); + logger.warn("Unexpected exception while generating a JSON response", e); } finally { if (!success) { buffer.release(); } } - return HttpResponse.of(responseHeaders, HttpData.wrap(buffer)); + if (success) { + return HttpResponse.of(responseHeaders, HttpData.wrap(buffer)); + } else { + return HttpResponse.of(responseHeaders); + } }; } @@ -257,7 +261,7 @@ static void writeErrorDetails(List details, JsonGenerator jsonGenerator) th try { ERROR_DETAILS_MARSHALLER.writeValue(detail, jsonGenerator); } catch (IOException e) { - logger.warn("Unexpected exception while writing error detail {} to json", detail); + logger.warn("Unexpected exception while writing an error detail to JSON. detail: {}", detail, e); } } jsonGenerator.writeEndArray(); From 7e6f35be3ef1fd199da6cb7783cec5cb7fa561d1 Mon Sep 17 00:00:00 2001 From: huguang Date: Mon, 25 Jul 2022 19:38:11 +0800 Subject: [PATCH 30/36] resolve checkstyle error --- .../armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java index 6d96110f951..2c4214938de 100644 --- a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java @@ -261,7 +261,8 @@ static void writeErrorDetails(List details, JsonGenerator jsonGenerator) th try { ERROR_DETAILS_MARSHALLER.writeValue(detail, jsonGenerator); } catch (IOException e) { - logger.warn("Unexpected exception while writing an error detail to JSON. detail: {}", detail, e); + logger.warn("Unexpected exception while writing an error detail to JSON. detail: {}", + detail, e); } } jsonGenerator.writeEndArray(); From fbc4a01331bcef3b349a3c489dd8499f42f19e41 Mon Sep 17 00:00:00 2001 From: Ikhun Um Date: Wed, 27 Jul 2022 16:49:58 +0900 Subject: [PATCH 31/36] Update grpc/src/test/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandlerTest.java --- .../server/grpc/DefaultUnframedGrpcErrorHandlerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/grpc/src/test/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandlerTest.java b/grpc/src/test/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandlerTest.java index 923435b993a..29db494f189 100644 --- a/grpc/src/test/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandlerTest.java +++ b/grpc/src/test/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandlerTest.java @@ -188,7 +188,7 @@ void convertErrorDetailToJsonNodeTest() throws IOException { " \"@type\":\"type.googleapis.com/google.rpc.LocalizedMessage\",\n" + " \"message\":\"message\"\n" + " }\n" + - "]"; + ']'; assertThatJson(mapper.readTree(jsonObjectWriter.toString())).isEqualTo(expectedJsonString); } From 542ca4166b6b843bf31cf2a094fccd3180f9b13b Mon Sep 17 00:00:00 2001 From: Ikhun Um Date: Wed, 27 Jul 2022 16:50:05 +0900 Subject: [PATCH 32/36] Update grpc/src/test/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandlerTest.java --- .../server/grpc/DefaultUnframedGrpcErrorHandlerTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/grpc/src/test/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandlerTest.java b/grpc/src/test/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandlerTest.java index 29db494f189..7b30d9f27b2 100644 --- a/grpc/src/test/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandlerTest.java +++ b/grpc/src/test/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandlerTest.java @@ -103,8 +103,7 @@ void convertErrorDetailToJsonNodeTest() throws IOException { .build(); final Status status = Status.newBuilder() - .setCode( - Code.UNKNOWN.getNumber()) + .setCode(Code.UNKNOWN.getNumber()) .setMessage("Unknown Exceptions Test") .addDetails(Any.pack(errorInfo)) .addDetails(Any.pack(retryInfo)) From 6a4e32c4439f3c1926ea58796deec4c065ffda42 Mon Sep 17 00:00:00 2001 From: Hu Guang Date: Fri, 29 Jul 2022 10:07:36 +0800 Subject: [PATCH 33/36] Update grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java Co-authored-by: jrhee17 --- .../armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java | 1 - 1 file changed, 1 deletion(-) diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java index 2c4214938de..379fe303e61 100644 --- a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java @@ -268,7 +268,6 @@ static void writeErrorDetails(List details, JsonGenerator jsonGenerator) th jsonGenerator.writeEndArray(); } - @VisibleForTesting static com.google.rpc.Status decodeGrpcStatusDetailsBin(String grpcStatusDetailsBin) throws InvalidProtocolBufferException { final byte[] result = Base64.getDecoder().decode(grpcStatusDetailsBin); From cf5f98a151675f2cedb6b7c20eb67d5ac96c9d3f Mon Sep 17 00:00:00 2001 From: Hu Guang Date: Sat, 30 Jul 2022 20:47:37 +0800 Subject: [PATCH 34/36] Update grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java Co-authored-by: minux --- .../linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java index 10373a91f36..ba5b0b13678 100644 --- a/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java @@ -84,7 +84,8 @@ static UnframedGrpcErrorHandler ofPlainText(UnframedGrpcStatusMappingFunction st /** * Returns a rich error JSON response based on Google APIs. - * Please refer Google error model + * See Google error model + * for more information. */ static UnframedGrpcErrorHandler ofRichJson() { return DefaultUnframedGrpcErrorHandler.ofRichJson(UnframedGrpcStatusMappingFunction.of()); From 8b3bc315a7ca881af68b31181c66a786f28e73dd Mon Sep 17 00:00:00 2001 From: Ikhun Um Date: Thu, 4 Aug 2022 13:16:57 +0900 Subject: [PATCH 35/36] Address comments --- .../server/grpc/UnframedGrpcErrorHandler.java | 14 +++---- ...er.java => UnframedGrpcErrorHandlers.java} | 38 ++++++++----------- ...t.java => ErrorDetailsMarshallerTest.java} | 6 +-- .../grpc/GrpcStatusDetailsBinHeaderTest.java | 2 +- .../grpc/UnframedGrpcErrorHandlerTest.java | 18 ++++++--- 5 files changed, 40 insertions(+), 38 deletions(-) rename grpc/src/main/java/com/linecorp/armeria/server/grpc/{DefaultUnframedGrpcErrorHandler.java => UnframedGrpcErrorHandlers.java} (91%) rename grpc/src/test/java/com/linecorp/armeria/server/grpc/{DefaultUnframedGrpcErrorHandlerTest.java => ErrorDetailsMarshallerTest.java} (97%) diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java index ba5b0b13678..637ec3cae8d 100644 --- a/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java @@ -35,7 +35,7 @@ public interface UnframedGrpcErrorHandler { * Returns a plain text or json response based on the content type. */ static UnframedGrpcErrorHandler of() { - return DefaultUnframedGrpcErrorHandler.of(UnframedGrpcStatusMappingFunction.of()); + return UnframedGrpcErrorHandlers.of(UnframedGrpcStatusMappingFunction.of()); } /** @@ -45,14 +45,14 @@ static UnframedGrpcErrorHandler of() { * to an {@link HttpStatus} code. */ static UnframedGrpcErrorHandler of(UnframedGrpcStatusMappingFunction statusMappingFunction) { - return DefaultUnframedGrpcErrorHandler.of(statusMappingFunction); + return UnframedGrpcErrorHandlers.of(statusMappingFunction); } /** * Returns a JSON response. */ static UnframedGrpcErrorHandler ofJson() { - return DefaultUnframedGrpcErrorHandler.ofJson(UnframedGrpcStatusMappingFunction.of()); + return UnframedGrpcErrorHandlers.ofJson(UnframedGrpcStatusMappingFunction.of()); } /** @@ -62,14 +62,14 @@ static UnframedGrpcErrorHandler ofJson() { * to an {@link HttpStatus} code. */ static UnframedGrpcErrorHandler ofJson(UnframedGrpcStatusMappingFunction statusMappingFunction) { - return DefaultUnframedGrpcErrorHandler.ofJson(statusMappingFunction); + return UnframedGrpcErrorHandlers.ofJson(statusMappingFunction); } /** * Returns a plain text response. */ static UnframedGrpcErrorHandler ofPlainText() { - return DefaultUnframedGrpcErrorHandler.ofPlaintext(UnframedGrpcStatusMappingFunction.of()); + return UnframedGrpcErrorHandlers.ofPlaintext(UnframedGrpcStatusMappingFunction.of()); } /** @@ -79,7 +79,7 @@ static UnframedGrpcErrorHandler ofPlainText() { * to an {@link HttpStatus} code. */ static UnframedGrpcErrorHandler ofPlainText(UnframedGrpcStatusMappingFunction statusMappingFunction) { - return DefaultUnframedGrpcErrorHandler.ofPlaintext(statusMappingFunction); + return UnframedGrpcErrorHandlers.ofPlaintext(statusMappingFunction); } /** @@ -88,7 +88,7 @@ static UnframedGrpcErrorHandler ofPlainText(UnframedGrpcStatusMappingFunction st * for more information. */ static UnframedGrpcErrorHandler ofRichJson() { - return DefaultUnframedGrpcErrorHandler.ofRichJson(UnframedGrpcStatusMappingFunction.of()); + return UnframedGrpcErrorHandlers.ofRichJson(UnframedGrpcStatusMappingFunction.of()); } /** diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlers.java similarity index 91% rename from grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java rename to grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlers.java index 379fe303e61..83bcf9c48ce 100644 --- a/grpc/src/main/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandler.java +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlers.java @@ -65,10 +65,11 @@ import io.netty.buffer.ByteBuf; import io.netty.buffer.ByteBufOutputStream; -final class DefaultUnframedGrpcErrorHandler { +final class UnframedGrpcErrorHandlers { - private static final Logger logger = LoggerFactory.getLogger(DefaultUnframedGrpcErrorHandler.class); + private static final Logger logger = LoggerFactory.getLogger(UnframedGrpcErrorHandlers.class); + // XXX(ikhoon): Support custom JSON marshaller? private static final MessageMarshaller ERROR_DETAILS_MARSHALLER = MessageMarshaller.builder() .omittingInsignificantWhitespace(true) @@ -93,8 +94,7 @@ final class DefaultUnframedGrpcErrorHandler { * to an {@link HttpStatus} code. */ static UnframedGrpcErrorHandler of(UnframedGrpcStatusMappingFunction statusMappingFunction) { - final UnframedGrpcStatusMappingFunction mappingFunction - = ofStatusMappingFunction(statusMappingFunction); + final UnframedGrpcStatusMappingFunction mappingFunction = withDefault(statusMappingFunction); return (ctx, status, response) -> { final MediaType grpcMediaType = response.contentType(); if (grpcMediaType != null && grpcMediaType.isJson()) { @@ -112,12 +112,11 @@ static UnframedGrpcErrorHandler of(UnframedGrpcStatusMappingFunction statusMappi * to an {@link HttpStatus} code. */ static UnframedGrpcErrorHandler ofJson(UnframedGrpcStatusMappingFunction statusMappingFunction) { - final UnframedGrpcStatusMappingFunction mappingFunction - = ofStatusMappingFunction(statusMappingFunction); + final UnframedGrpcStatusMappingFunction mappingFunction = withDefault(statusMappingFunction); return (ctx, status, response) -> { final Code grpcCode = status.getCode(); final String grpcMessage = status.getDescription(); - final Throwable cause = getThrowableFromContext(ctx); + final Throwable cause = responseCause(ctx); final HttpStatus httpStatus = mappingFunction.apply(ctx, status, cause); final ResponseHeaders responseHeaders = ResponseHeaders.builder(httpStatus) .contentType(MediaType.JSON_UTF_8) @@ -143,12 +142,11 @@ static UnframedGrpcErrorHandler ofJson(UnframedGrpcStatusMappingFunction statusM * to an {@link HttpStatus} code. */ static UnframedGrpcErrorHandler ofPlaintext(UnframedGrpcStatusMappingFunction statusMappingFunction) { - final UnframedGrpcStatusMappingFunction mappingFunction - = ofStatusMappingFunction(statusMappingFunction); + final UnframedGrpcStatusMappingFunction mappingFunction = withDefault(statusMappingFunction); return (ctx, status, response) -> { final Code grpcCode = status.getCode(); final String grpcMessage = status.getDescription(); - final Throwable cause = getThrowableFromContext(ctx); + final Throwable cause = responseCause(ctx); final HttpStatus httpStatus = mappingFunction.apply(ctx, status, cause); final ResponseHeaders responseHeaders = ResponseHeaders.builder(httpStatus) .contentType(MediaType.PLAIN_TEXT_UTF_8) @@ -185,7 +183,7 @@ static UnframedGrpcErrorHandler ofRichJson(UnframedGrpcStatusMappingFunction sta final ByteBuf buffer = ctx.alloc().buffer(); final Code grpcCode = status.getCode(); final String grpcMessage = status.getDescription(); - final Throwable cause = getThrowableFromContext(ctx); + final Throwable cause = responseCause(ctx); final HttpStatus httpStatus = mappingFunction.apply(ctx, status, cause); final HttpHeaders trailers = !response.trailers().isEmpty() ? response.trailers() : response.headers(); @@ -199,8 +197,6 @@ static UnframedGrpcErrorHandler ofRichJson(UnframedGrpcStatusMappingFunction sta try (OutputStream outputStream = new ByteBufOutputStream(buffer); JsonGenerator jsonGenerator = mapper.createGenerator(outputStream)) { jsonGenerator.writeStartObject(); - jsonGenerator.writeFieldName("error"); - jsonGenerator.writeStartObject(); jsonGenerator.writeNumberField("code", grpcCode.value()); if (grpcMessage != null) { jsonGenerator.writeStringField("message", grpcMessage); @@ -222,7 +218,6 @@ static UnframedGrpcErrorHandler ofRichJson(UnframedGrpcStatusMappingFunction sta } } jsonGenerator.writeEndObject(); - jsonGenerator.writeEndObject(); jsonGenerator.flush(); success = true; } catch (IOException e) { @@ -240,13 +235,12 @@ static UnframedGrpcErrorHandler ofRichJson(UnframedGrpcStatusMappingFunction sta }; } - private DefaultUnframedGrpcErrorHandler() {} - /** * Ensure that unframedGrpcStatusMappingFunction never returns null by falling back to the default. */ - private static UnframedGrpcStatusMappingFunction ofStatusMappingFunction( + private static UnframedGrpcStatusMappingFunction withDefault( UnframedGrpcStatusMappingFunction statusMappingFunction) { + requireNonNull(statusMappingFunction, "statusMappingFunction"); if (statusMappingFunction == UnframedGrpcStatusMappingFunction.of()) { return statusMappingFunction; @@ -275,14 +269,14 @@ static com.google.rpc.Status decodeGrpcStatusDetailsBin(String grpcStatusDetails } @Nullable - private static Throwable getThrowableFromContext(ServiceRequestContext ctx) { + private static Throwable responseCause(ServiceRequestContext ctx) { final RequestLogAccess log = ctx.log(); - final Throwable cause; if (log.isAvailable(RequestLogProperty.RESPONSE_CAUSE)) { - cause = log.partial().responseCause(); + return log.partial().responseCause(); } else { - cause = null; + return null; } - return cause; } + + private UnframedGrpcErrorHandlers() {} } diff --git a/grpc/src/test/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandlerTest.java b/grpc/src/test/java/com/linecorp/armeria/server/grpc/ErrorDetailsMarshallerTest.java similarity index 97% rename from grpc/src/test/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandlerTest.java rename to grpc/src/test/java/com/linecorp/armeria/server/grpc/ErrorDetailsMarshallerTest.java index 7b30d9f27b2..10617c71f54 100644 --- a/grpc/src/test/java/com/linecorp/armeria/server/grpc/DefaultUnframedGrpcErrorHandlerTest.java +++ b/grpc/src/test/java/com/linecorp/armeria/server/grpc/ErrorDetailsMarshallerTest.java @@ -47,7 +47,7 @@ import com.linecorp.armeria.internal.common.JacksonUtil; -class DefaultUnframedGrpcErrorHandlerTest { +class ErrorDetailsMarshallerTest { private static final ObjectMapper mapper = JacksonUtil.newDefaultObjectMapper(); @@ -119,7 +119,7 @@ void convertErrorDetailToJsonNodeTest() throws IOException { final StringWriter jsonObjectWriter = new StringWriter(); final JsonGenerator jsonGenerator = mapper.createGenerator(jsonObjectWriter); - DefaultUnframedGrpcErrorHandler.writeErrorDetails(status.getDetailsList(), jsonGenerator); + UnframedGrpcErrorHandlers.writeErrorDetails(status.getDetailsList(), jsonGenerator); jsonGenerator.flush(); final String expectedJsonString = "[\n" + @@ -198,7 +198,7 @@ void shouldThrowIOException() throws IOException { final StringWriter jsonObjectWriter = new StringWriter(); final JsonGenerator jsonGenerator = mapper.createGenerator(jsonObjectWriter); - assertThatThrownBy(() -> DefaultUnframedGrpcErrorHandler.writeErrorDetails( + assertThatThrownBy(() -> UnframedGrpcErrorHandlers.writeErrorDetails( status.getDetailsList(), jsonGenerator)).isInstanceOf(IOException.class); } } diff --git a/grpc/src/test/java/com/linecorp/armeria/server/grpc/GrpcStatusDetailsBinHeaderTest.java b/grpc/src/test/java/com/linecorp/armeria/server/grpc/GrpcStatusDetailsBinHeaderTest.java index c25c4e07b0f..b1e74ea7ad1 100644 --- a/grpc/src/test/java/com/linecorp/armeria/server/grpc/GrpcStatusDetailsBinHeaderTest.java +++ b/grpc/src/test/java/com/linecorp/armeria/server/grpc/GrpcStatusDetailsBinHeaderTest.java @@ -98,7 +98,7 @@ void googleRpcErrorDetail() throws InvalidProtocolBufferException { .post(TestServiceGrpc.getEmptyCallMethod().getFullMethodName()) .content(MediaType.PROTOBUF, Empty.getDefaultInstance().toByteArray()) .execute(); - final Status status = DefaultUnframedGrpcErrorHandler.decodeGrpcStatusDetailsBin( + final Status status = UnframedGrpcErrorHandlers.decodeGrpcStatusDetailsBin( response.headers().get(GrpcHeaderNames.GRPC_STATUS_DETAILS_BIN)); assertThat(status).isEqualTo(googleRpcStatus); assertThat(response.trailers()).isEmpty(); diff --git a/grpc/src/test/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerTest.java b/grpc/src/test/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerTest.java index 033c03b79f9..617f748b274 100644 --- a/grpc/src/test/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerTest.java +++ b/grpc/src/test/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandlerTest.java @@ -175,11 +175,19 @@ void richJson() throws JsonProcessingException { .content(MediaType.PROTOBUF, Empty.getDefaultInstance().toByteArray()) .execute(); assertThat(response.status()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR); - assertThatJson(mapper.readTree(response.contentUtf8())).isEqualTo( - "{\"error\":{\"code\":2," + - "\"message\":\"Unknown Exceptions Test\"," + - "\"details\":[{\"@type\":\"type.googleapis.com/google.rpc.ErrorInfo\"," + - "\"reason\":\"Unknown Exception\",\"domain\":\"test\"}]}}"); + assertThatJson(mapper.readTree(response.contentUtf8())) + .isEqualTo( + '{' + + " \"code\": 2," + + " \"message\": \"Unknown Exceptions Test\"," + + " \"details\": [" + + " {" + + " \"@type\": \"type.googleapis.com/google.rpc.ErrorInfo\"," + + " \"reason\": \"Unknown Exception\"," + + " \"domain\": \"test\"" + + " }" + + " ]" + + '}'); assertThat(response.trailers()).isEmpty(); } } From 21ce155a0f6def5880f2e2d3c31257f7ea990861 Mon Sep 17 00:00:00 2001 From: Ikhun Um Date: Thu, 4 Aug 2022 15:41:29 +0900 Subject: [PATCH 36/36] Address comments by @minwoox --- .../server/grpc/UnframedGrpcErrorHandler.java | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java b/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java index 637ec3cae8d..1c3c9b808b9 100644 --- a/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java +++ b/grpc/src/main/java/com/linecorp/armeria/server/grpc/UnframedGrpcErrorHandler.java @@ -88,7 +88,19 @@ static UnframedGrpcErrorHandler ofPlainText(UnframedGrpcStatusMappingFunction st * for more information. */ static UnframedGrpcErrorHandler ofRichJson() { - return UnframedGrpcErrorHandlers.ofRichJson(UnframedGrpcStatusMappingFunction.of()); + return ofRichJson(UnframedGrpcStatusMappingFunction.of()); + } + + /** + * Returns a rich error JSON response based on Google APIs. + * See Google error model + * for more information. + * + * @param statusMappingFunction The function which maps the {@link Throwable} or gRPC {@link Status} code + * to an {@link HttpStatus} code. + */ + static UnframedGrpcErrorHandler ofRichJson(UnframedGrpcStatusMappingFunction statusMappingFunction) { + return UnframedGrpcErrorHandlers.ofRichJson(statusMappingFunction); } /**