Improve exceptions thrown when an error happens during proxy CONNECT

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

@@ -16,6 +16,8 @@
 package io.servicetalk.grpc.api;
 
 import io.servicetalk.http.api.Http2Exception;
+import io.servicetalk.http.api.HttpResponseMetaData;
+import io.servicetalk.http.api.ProxyConnectResponseException;
 import io.servicetalk.serializer.api.SerializationException;
 
 import java.util.concurrent.CancellationException;
@@ -28,6 +30,7 @@
 import static io.servicetalk.grpc.api.GrpcStatusCode.UNIMPLEMENTED;
 import static io.servicetalk.grpc.api.GrpcStatusCode.UNKNOWN;
 import static io.servicetalk.grpc.api.GrpcStatusCode.fromHttp2ErrorCode;
+import static io.servicetalk.grpc.api.GrpcUtils.fromHttpStatus;
 import static java.util.Objects.requireNonNull;
 
 /**
@@ -140,6 +143,9 @@ static GrpcStatus toGrpcStatus(Throwable cause) {
             status = new GrpcStatus(CANCELLED, cause);
         } else if (cause instanceof TimeoutException) {
             status = new GrpcStatus(DEADLINE_EXCEEDED, cause);
+        } else if (cause instanceof ProxyConnectResponseException) {
+            final HttpResponseMetaData response = ((ProxyConnectResponseException) cause).response();
+            status = new GrpcStatus(fromHttpStatus(response.status()), cause);
         } else {
             // Initialize detail because cause is often lost
             status = new GrpcStatus(UNKNOWN, cause, cause.toString());

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

@@ -104,6 +104,7 @@
 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.PROXY_AUTHENTICATION_REQUIRED;
 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;
@@ -275,15 +276,20 @@ static void setStatus(final HttpHeaders trailers, final Throwable cause, final B
     }
 
     private static void validateStatusCode(HttpResponseStatus status) {
-        final int statusCode = status.code();
-        if (statusCode == OK.code()) {
+        if (status.code() == OK.code()) {
             return;
         }
+        throw GrpcStatusException.of(Status.newBuilder().setCode(fromHttpStatus(status).value())
+                .setMessage("HTTP status code: " + status).build());
+    }
+
+    static GrpcStatusCode fromHttpStatus(final HttpResponseStatus status) {
+        final int statusCode = status.code();
         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()) {
+        } else if (statusCode == UNAUTHORIZED.code() || statusCode == PROXY_AUTHENTICATION_REQUIRED.code()) {
             grpcStatusCode = UNAUTHENTICATED;
         } else if (statusCode == FORBIDDEN.code()) {
             grpcStatusCode = PERMISSION_DENIED;
@@ -298,8 +304,7 @@ private static void validateStatusCode(HttpResponseStatus status) {
         } else {
             grpcStatusCode = UNKNOWN;
         }
-        throw GrpcStatusException.of(Status.newBuilder().setCode(grpcStatusCode.value())
-                .setMessage("HTTP status code: " + status).build());
+        return grpcStatusCode;
     }
 
     static <Resp> Publisher<Resp> validateResponseAndGetPayload(final StreamingHttpResponse response,

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

@@ -19,11 +19,13 @@
 import io.servicetalk.context.api.ContextMap.Key;
 import io.servicetalk.grpc.api.DefaultGrpcClientMetadata;
 import io.servicetalk.grpc.api.GrpcClientMetadata;
+import io.servicetalk.grpc.api.GrpcStatusCode;
 import io.servicetalk.grpc.api.GrpcStatusException;
+import io.servicetalk.http.api.HttpResponseStatus;
+import io.servicetalk.http.api.ProxyConnectResponseException;
 import io.servicetalk.http.api.StreamingHttpConnectionFilter;
 import io.servicetalk.http.api.StreamingHttpRequest;
 import io.servicetalk.http.api.StreamingHttpResponse;
-import io.servicetalk.http.netty.ProxyResponseException;
 import io.servicetalk.http.netty.ProxyTunnel;
 import io.servicetalk.test.resources.DefaultTestCerts;
 import io.servicetalk.transport.api.ClientSslConfigBuilder;
@@ -41,16 +43,23 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import java.io.OutputStream;
 import java.net.InetSocketAddress;
 
 import static io.servicetalk.context.api.ContextMap.Key.newKey;
+import static io.servicetalk.grpc.api.GrpcStatusCode.INVALID_ARGUMENT;
+import static io.servicetalk.grpc.api.GrpcStatusCode.UNAUTHENTICATED;
+import static io.servicetalk.grpc.api.GrpcStatusCode.UNAVAILABLE;
 import static io.servicetalk.grpc.api.GrpcStatusCode.UNKNOWN;
 import static io.servicetalk.http.api.HttpProtocolVersion.HTTP_2_0;
+import static io.servicetalk.http.api.HttpResponseStatus.BAD_REQUEST;
 import static io.servicetalk.http.api.HttpResponseStatus.INTERNAL_SERVER_ERROR;
 import static io.servicetalk.http.api.HttpResponseStatus.PROXY_AUTHENTICATION_REQUIRED;
+import static io.servicetalk.http.api.HttpResponseStatus.SERVICE_UNAVAILABLE;
 import static io.servicetalk.test.resources.DefaultTestCerts.serverPemHostname;
 import static io.servicetalk.transport.netty.internal.AddressUtils.localAddress;
 import static io.servicetalk.transport.netty.internal.AddressUtils.serverHostAndPort;
+import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
@@ -125,24 +134,44 @@ void testRequest() throws Exception {
     }
 
     @Test
-    void testProxyAuthRequired() throws Exception {
+    void testProxyAuthRequired() {
         proxyTunnel.basicAuthToken(AUTH_TOKEN);
-        GrpcStatusException e = assertThrows(GrpcStatusException.class,
-                () -> client.sayHello(HelloRequest.newBuilder().setName("foo").build()));
-        assertThat(e.status().code(), is(UNKNOWN));
-        Throwable cause = e.getCause();
-        assertThat(cause, is(instanceOf(ProxyResponseException.class)));
-        assertThat(((ProxyResponseException) cause).status(), is(PROXY_AUTHENTICATION_REQUIRED));
+        assertProxyConnectResponseException(UNAUTHENTICATED, PROXY_AUTHENTICATION_REQUIRED);
     }
 
     @Test
-    void testBadProxyResponse() throws Exception {
+    void testInternalServerError() {
         proxyTunnel.badResponseProxy();
+        assertProxyConnectResponseException(UNKNOWN, INTERNAL_SERVER_ERROR);
+    }
+
+    @Test
+    void testServiceUnavailable() {
+        proxyTunnel.proxyRequestHandler((socket, host, port, protocol) -> {
+            final OutputStream os = socket.getOutputStream();
+            os.write((protocol + ' ' + SERVICE_UNAVAILABLE + "\r\n\r\n").getBytes(UTF_8));
+            os.flush();
+        });
+        assertProxyConnectResponseException(UNAVAILABLE, SERVICE_UNAVAILABLE);
+    }
+
+    @Test
+    void testClientError() {
+        proxyTunnel.proxyRequestHandler((socket, host, port, protocol) -> {
+            final OutputStream os = socket.getOutputStream();
+            os.write((protocol + ' ' + BAD_REQUEST + "\r\n\r\n").getBytes(UTF_8));
+            os.flush();
+        });
+        assertProxyConnectResponseException(INVALID_ARGUMENT, BAD_REQUEST);
+    }
+
+    private void assertProxyConnectResponseException(GrpcStatusCode grpcStatusCode,
+                                                     HttpResponseStatus httpResponseStatus) {
         GrpcStatusException e = assertThrows(GrpcStatusException.class,
                 () -> client.sayHello(HelloRequest.newBuilder().setName("foo").build()));
-        assertThat(e.status().code(), is(UNKNOWN));
+        assertThat(e.status().code(), is(grpcStatusCode));
         Throwable cause = e.getCause();
-        assertThat(cause, is(instanceOf(ProxyResponseException.class)));
-        assertThat(((ProxyResponseException) cause).status(), is(INTERNAL_SERVER_ERROR));
+        assertThat(cause, is(instanceOf(ProxyConnectResponseException.class)));
+        assertThat(((ProxyConnectResponseException) cause).response().status(), is(httpResponseStatus));
     }
 }

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

@@ -127,12 +127,14 @@
 import static io.servicetalk.grpc.api.GrpcStatusCode.FAILED_PRECONDITION;
 import static io.servicetalk.grpc.api.GrpcStatusCode.INTERNAL;
 import static io.servicetalk.grpc.api.GrpcStatusCode.INVALID_ARGUMENT;
+import static io.servicetalk.grpc.api.GrpcStatusCode.UNAUTHENTICATED;
 import static io.servicetalk.grpc.api.GrpcStatusCode.UNKNOWN;
 import static io.servicetalk.grpc.internal.DeadlineUtils.GRPC_TIMEOUT_HEADER_KEY;
 import static io.servicetalk.http.api.HttpExecutionStrategies.offloadNone;
 import static io.servicetalk.http.api.HttpResponseStatus.BAD_REQUEST;
 import static io.servicetalk.http.api.HttpResponseStatus.EXPECTATION_FAILED;
 import static io.servicetalk.http.api.HttpResponseStatus.PRECONDITION_FAILED;
+import static io.servicetalk.http.api.HttpResponseStatus.PROXY_AUTHENTICATION_REQUIRED;
 import static io.servicetalk.http.api.HttpResponseStatus.REQUEST_HEADER_FIELDS_TOO_LARGE;
 import static io.servicetalk.http.api.HttpResponseStatus.REQUEST_TIMEOUT;
 import static io.servicetalk.http.api.HttpResponseStatus.StatusClass.CLIENT_ERROR_4XX;
@@ -541,6 +543,9 @@ void clientH2ReturnStatus() throws Exception {
                     // this exception differently internally.
                     assertThat("h2 response code: " + httpCode, stCode, equalTo(UNKNOWN.value()));
                     assertThat("h2 response code: " + httpCode, grpcJavaCode, equalTo(INTERNAL.value()));
+                } else if (httpCode == PROXY_AUTHENTICATION_REQUIRED.code()) {
+                    assertThat("h2 response code: " + httpCode, stCode, equalTo(UNAUTHENTICATED.value()));
+                    assertThat("h2 response code: " + httpCode, grpcJavaCode, equalTo(UNKNOWN.value()));
                 } else if (httpCode == REQUEST_TIMEOUT.code()) {
                     assertThat("h2 response code: " + httpCode, stCode, equalTo(DEADLINE_EXCEEDED.value()));
                     assertThat("h2 response code: " + httpCode, grpcJavaCode, equalTo(UNKNOWN.value()));

servicetalk-http-api/gradle/spotbugs/main-exclusions.xml

@@ -314,4 +314,13 @@
     <Method name="catchPayloadFailure"/>
     <Bug pattern="THROWS_METHOD_THROWS_CLAUSE_THROWABLE"/>
   </Match>
+
+  <!-- Parameters/state is intentional -->
+  <Match>
+    <Class name="io.servicetalk.http.api.ProxyConnectResponseException"/>
+    <Or>
+      <Bug pattern="EI_EXPOSE_REP"/>
+      <Bug pattern="EI_EXPOSE_REP2"/>
+    </Or>
+  </Match>
 </FindBugsFilter>

servicetalk-http-netty/src/main/java/io/servicetalk/http/netty/ProxyConnectException.java → servicetalk-http-api/src/main/java/io/servicetalk/http/api/ProxyConnectException.java

@@ -13,39 +13,36 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package io.servicetalk.http.netty;
-
-import io.servicetalk.transport.api.RetryableException;
+package io.servicetalk.http.api;
 
 import java.io.IOException;
 
 /**
  * An exception while processing
  * <a href="https://datatracker.ietf.org/doc/html/rfc9110#section-9.3.6">HTTP/1.1 CONNECT</a> request.
+ *
+ * @see SingleAddressHttpClientBuilder#proxyAddress(Object)
  */
 public class ProxyConnectException extends IOException {
 
     private static final long serialVersionUID = 4453075928788773272L;
 
-    ProxyConnectException(final String message) {
+    /**
+     * Creates a new instance.
+     *
+     * @param message the detail message
+     */
+    public ProxyConnectException(final String message) {
         super(message);
     }
 
-    ProxyConnectException(final String message, final Throwable cause) {
+    /**
+     * Creates a new instance.
+     *
+     * @param message the detail message
+     * @param cause the original cause
+     */
+    public ProxyConnectException(final String message, final Throwable cause) {
         super(message, cause);
     }
-
-    static final class RetryableProxyConnectException extends ProxyConnectException
-            implements RetryableException {
-
-        private static final long serialVersionUID = 5118637083568536242L;
-
-        RetryableProxyConnectException(final String message) {
-            super(message);
-        }
-
-        RetryableProxyConnectException(final String message, final Throwable cause) {
-            super(message, cause);
-        }
-    }
 }

servicetalk-http-api/src/main/java/io/servicetalk/http/api/ProxyConnectResponseException.java

@@ -0,0 +1,50 @@
+/*
+ * Copyright © 2023 Apple Inc. and the ServiceTalk project authors
+ *
+ * Licensed 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
+ *
+ *   http://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 io.servicetalk.http.api;
+
+/**
+ * A subclass of {@link ProxyConnectException} that indicates an unexpected response status from a proxy received for
+ * <a href="https://datatracker.ietf.org/doc/html/rfc9110#section-9.3.6">HTTP/1.1 CONNECT</a> request.
+ *
+ * @see SingleAddressHttpClientBuilder#proxyAddress(Object)
+ */
+public class ProxyConnectResponseException extends ProxyConnectException {
+
+    private static final long serialVersionUID = 5117046591069113007L;
+
+    private final HttpResponseMetaData response;
+
+    /**
+     * Creates a new instance.
+     *
+     * @param message the detail message
+     * @param response {@link HttpResponseMetaData} of the received response
+     */
+    public ProxyConnectResponseException(final String message, final HttpResponseMetaData response) {
+        super(message);
+        this.response = response;
+    }
+
+    /**
+     * Returns {@link HttpResponseMetaData} that was received in response to
+     * <a href="https://datatracker.ietf.org/doc/html/rfc9110#section-9.3.6">HTTP/1.1 CONNECT</a> request.
+     *
+     * @return {@link HttpResponseMetaData} that was received
+     */
+    public HttpResponseMetaData response() {
+        return response;
+    }
+}

servicetalk-http-api/src/main/java/io/servicetalk/http/api/SingleAddressHttpClientBuilder.java

@@ -58,7 +58,9 @@ public interface SingleAddressHttpClientBuilder<U, R> extends HttpClientBuilder<
      * always initialized using
      * <a href="https://datatracker.ietf.org/doc/html/rfc9110#section-9.3.6">HTTP/1.1 CONNECT</a> request. The actual
      * protocol will be negotiated via <a href="https://tools.ietf.org/html/rfc7301">ALPN extension</a> of TLS protocol,
-     * taking into account HTTP protocols configured via {@link #protocols(HttpProtocolConfig...)} method.
+     * taking into account HTTP protocols configured via {@link #protocols(HttpProtocolConfig...)} method. In case of
+     * any error during {@code CONNECT} process, {@link ProxyConnectException} or {@link ProxyConnectResponseException}
+     * will be thrown when a request attempt is made through the constructed client instance.
      *
      * @param proxyAddress Unresolved address of the proxy. When used with a builder created for a resolved address,
      * {@code proxyAddress} should also be already resolved – otherwise runtime exceptions may occur.