gRPC improve error status for non-200 h2 response status (#2221)

Motivation:
ServiceTalk currently assumes the response status is 200
when decoding into gRPC. This means we will fail during
checking the content-type or during deserialization if
an error is return from a middle proxy with a different
encoding type. We can provide more informative gRPC
error codes if we validate the h2 response status.

servicetalk-grpc-api/src/main/java/io/servicetalk/grpc/api/GrpcStatusCode.java

@@ -40,17 +40,34 @@ public enum GrpcStatusCode {
     NOT_FOUND(5),
     /** Some entity that we attempted to create already exists. */
     ALREADY_EXISTS(6),
-    /** Permission denied for a particular client. Different from {@link #UNAUTHENTICATED}. */
+    /**
+     * Permission denied for a particular client. Must not be used for the following cases:
+     * <ul>
+     *     <li>rejections caused by exhausting some resource (use {@link #RESOURCE_EXHAUSTED} instead)</li>
+     *     <li>the caller cannot be identified (use {@link #UNAUTHENTICATED} instead)</li>
+     * </ul>
+     */
     PERMISSION_DENIED(7),
     /** Resource exhausted. */
     RESOURCE_EXHAUSTED(8),
-    /** The action cannot be executed on the current system state. Client should not retry.. */
+    /** The action cannot be executed on the current system state. Client should not retry. */
     FAILED_PRECONDITION(9),
-    /** Aborted, typically due to a concurrency issue (think CAS). Client may retry the whole sequence.. */
+    /** Aborted, typically due to a concurrency issue (think CAS). Client may retry the whole sequence. */
     ABORTED(10),
-    /** Used for range errors. */
+    /**
+     * Used for range errors (e.g. seeking or reading past end of file.)
+     * <p>
+     * Unlike {@link #INVALID_ARGUMENT}, this error indicates a problem that may be fixed if the system state changes.
+     * For example, a 32-bit file system will generate {@link #INVALID_ARGUMENT} if asked to read at an offset that is
+     * not in the range [0,2^32-1], but it will generate OUT_OF_RANGE if asked to read from an offset past the current
+     * file size.
+     * <p>
+     * There is a fair bit of overlap with {@link #FAILED_PRECONDITION}. This error is more specific and recommended
+     * in scenarios when callers who are iterating through a space can easily look for an OUT_OF_RANGE error to detect
+     * when they are done.
+     */
     OUT_OF_RANGE(11),
-    /** Unimplemented action. */
+    /** The method/operation is not implemented/supported. */
     UNIMPLEMENTED(12),
     /** Internal invariant violated. */
     INTERNAL(13),

servicetalk-grpc-api/src/main/java/io/servicetalk/grpc/api/GrpcUtils.java

@@ -35,6 +35,7 @@
 import io.servicetalk.http.api.HttpResponse;
 import io.servicetalk.http.api.HttpResponseFactory;
 import io.servicetalk.http.api.HttpResponseMetaData;
+import io.servicetalk.http.api.HttpResponseStatus;
 import io.servicetalk.http.api.HttpSerializer;
 import io.servicetalk.http.api.StatelessTrailersTransformer;
 import io.servicetalk.http.api.StreamingHttpResponse;
@@ -77,7 +78,11 @@
 import static io.servicetalk.grpc.api.GrpcHeaderValues.SERVICETALK_USER_AGENT;
 import static io.servicetalk.grpc.api.GrpcStatusCode.CANCELLED;
 import static io.servicetalk.grpc.api.GrpcStatusCode.DEADLINE_EXCEEDED;
-import static io.servicetalk.grpc.api.GrpcStatusCode.INTERNAL;
+import static io.servicetalk.grpc.api.GrpcStatusCode.FAILED_PRECONDITION;
+import static io.servicetalk.grpc.api.GrpcStatusCode.INVALID_ARGUMENT;
+import static io.servicetalk.grpc.api.GrpcStatusCode.PERMISSION_DENIED;
+import static io.servicetalk.grpc.api.GrpcStatusCode.UNAUTHENTICATED;
+import static io.servicetalk.grpc.api.GrpcStatusCode.UNAVAILABLE;
 import static io.servicetalk.grpc.api.GrpcStatusCode.UNIMPLEMENTED;
 import static io.servicetalk.grpc.api.GrpcStatusCode.UNKNOWN;
 import static io.servicetalk.grpc.api.GrpcStatusCode.fromHttp2ErrorCode;
@@ -89,6 +94,19 @@
 import static io.servicetalk.http.api.HttpHeaderNames.USER_AGENT;
 import static io.servicetalk.http.api.HttpHeaderValues.TRAILERS;
 import static io.servicetalk.http.api.HttpRequestMethod.POST;
+import static io.servicetalk.http.api.HttpResponseStatus.BAD_GATEWAY;
+import static io.servicetalk.http.api.HttpResponseStatus.EXPECTATION_FAILED;
+import static io.servicetalk.http.api.HttpResponseStatus.FORBIDDEN;
+import static io.servicetalk.http.api.HttpResponseStatus.GATEWAY_TIMEOUT;
+import static io.servicetalk.http.api.HttpResponseStatus.NOT_FOUND;
+import static io.servicetalk.http.api.HttpResponseStatus.NOT_IMPLEMENTED;
+import static io.servicetalk.http.api.HttpResponseStatus.OK;
+import static io.servicetalk.http.api.HttpResponseStatus.PRECONDITION_FAILED;
+import static io.servicetalk.http.api.HttpResponseStatus.REQUEST_TIMEOUT;
+import static io.servicetalk.http.api.HttpResponseStatus.SERVICE_UNAVAILABLE;
+import static io.servicetalk.http.api.HttpResponseStatus.StatusClass.CLIENT_ERROR_4XX;
+import static io.servicetalk.http.api.HttpResponseStatus.TOO_MANY_REQUESTS;
+import static io.servicetalk.http.api.HttpResponseStatus.UNAUTHORIZED;
 import static java.lang.String.valueOf;
 import static java.util.Collections.emptyList;
 import static java.util.Objects.requireNonNull;
@@ -263,10 +281,39 @@ static GrpcStatusException toGrpcException(Throwable cause) {
                 : new GrpcStatusException(toGrpcStatus(cause), () -> null);
     }
 
+    private static void validateStatusCode(HttpResponseStatus status) {
+        final int statusCode = status.code();
+        if (statusCode == OK.code()) {
+            return;
+        }
+        final GrpcStatusCode grpcStatusCode;
+        if (statusCode == BAD_GATEWAY.code() || statusCode == SERVICE_UNAVAILABLE.code() ||
+                statusCode == GATEWAY_TIMEOUT.code() || statusCode == TOO_MANY_REQUESTS.code()) {
+            grpcStatusCode = UNAVAILABLE;
+        } else if (statusCode == UNAUTHORIZED.code()) {
+            grpcStatusCode = UNAUTHENTICATED;
+        } else if (statusCode == FORBIDDEN.code()) {
+            grpcStatusCode = PERMISSION_DENIED;
+        } else if (statusCode == NOT_FOUND.code() || statusCode == NOT_IMPLEMENTED.code()) {
+            grpcStatusCode = UNIMPLEMENTED;
+        } else if (statusCode == REQUEST_TIMEOUT.code()) {
+            grpcStatusCode = DEADLINE_EXCEEDED;
+        } else if (statusCode == PRECONDITION_FAILED.code() || statusCode == EXPECTATION_FAILED.code()) {
+            grpcStatusCode = FAILED_PRECONDITION;
+        } else if (CLIENT_ERROR_4XX.contains(statusCode)) {
+            grpcStatusCode = INVALID_ARGUMENT;
+        } else {
+            grpcStatusCode = UNKNOWN;
+        }
+        throw GrpcStatusException.of(Status.newBuilder().setCode(grpcStatusCode.value())
+                .setMessage("HTTP status code: " + status).build());
+    }
+
     static <Resp> Publisher<Resp> validateResponseAndGetPayload(final StreamingHttpResponse response,
                                                                 final CharSequence expectedContentType,
                                                                 final BufferAllocator allocator,
                                                                 final GrpcStreamingDeserializer<Resp> deserializer) {
+        validateStatusCode(response.status()); // gRPC protocol requires 200, don't look further if this check fails.
         // In case of an empty response, gRPC-server may return only one HEADER frame with endStream=true. Our
         // HTTP1-based implementation translates them into response headers so we need to look for a grpc-status in both
         // headers and trailers. Since this is streaming response and we have the headers now, we check for the
@@ -293,6 +340,7 @@ static <Resp> Resp validateResponseAndGetPayload(final HttpResponse response,
                                                      final CharSequence expectedContentType,
                                                      final BufferAllocator allocator,
                                                      final GrpcDeserializer<Resp> deserializer) {
+        validateStatusCode(response.status()); // gRPC protocol requires 200, don't look further if this check fails.
         // In case of an empty response, gRPC-server may return only one HEADER frame with endStream=true. Our
         // HTTP1-based implementation translates them into response headers so we need to look for a grpc-status in both
         // headers and trailers.
@@ -320,7 +368,7 @@ static void validateContentType(HttpHeaders headers, CharSequence expectedConten
         if (!contentEqualsIgnoreCase(requestContentType, expectedContentType) &&
                 (requestContentType == null ||
                     !regionMatches(requestContentType, true, 0, APPLICATION_GRPC, 0, APPLICATION_GRPC.length()))) {
-            throw GrpcStatusException.of(Status.newBuilder().setCode(INTERNAL.value())
+            throw GrpcStatusException.of(Status.newBuilder().setCode(UNKNOWN.value())
                     .setMessage("invalid content-type: " + requestContentType).build());
         }
     }

servicetalk-grpc-netty/src/test/java/io/servicetalk/grpc/netty/HttpResponseUponGrpcRequestTest.java

@@ -49,7 +49,7 @@ final class HttpResponseUponGrpcRequestTest {
         ServerContext serverContext = HttpServers.forAddress(localAddress(0))
                 .protocols(h2Default())
                 .listenAndAwait((ctx, request, responseFactory) ->
-                        succeeded(responseFactory.badRequest().payloadBody(responsePayload, textSerializerUtf8())));
+                        succeeded(responseFactory.ok().payloadBody(responsePayload, textSerializerUtf8())));
 
         client = GrpcClients.forAddress(serverHostAndPort(serverContext))
                 .buildBlocking(new TesterProto.Tester.ClientFactory());
@@ -111,7 +111,7 @@ private static void assertThrowsGrpcStatusException(Executable executable) {
     }
 
     private static void assertGrpcStatusException(GrpcStatusException grpcStatusException) {
-        assertThat(grpcStatusException.status().code(), is(GrpcStatusCode.INTERNAL));
+        assertThat(grpcStatusException.status().code(), is(GrpcStatusCode.UNKNOWN));
         String description = grpcStatusException.status().description();
         assertThat(description, notNullValue());
         assertThat(description, containsString("invalid content-type: text/plain;"));