[UrlConnectionHttpClient] Wrap erroneous NPE from HttpURLConnection as IOException (#2572)

* [UrlConnectionHttpClient] Wrap erroneous NPE from HttpURLConnection as IOException

## Motivation

`HttpURLConnection#getInputStream0` has been observed to intermittently
throw `NullPointerException`s for reasons that still require further
investigation, but are assumed to be due to a bug in the JDK.
Propagating such NPEs is confusing for users and are not subject to
being retried on by the default retry policy configuration, so instead
we should bias towards propagating these as `IOException`s.

## Modifications

* When interacting specifically with HttpURLConnection#getResponseCode,
wrap any unexpected NullPointerExceptions as IOException
* Add corresponding unit test

## Result

* Transient errors will be retried and be less likely to surface to a
user's application layer

Author: Bennett-Lynch

.changes/next-release/bugfix-AWSSDKforJavav2-a3c92c2.json

@@ -0,0 +1,6 @@
+{
+    "category": "AWS SDK for Java v2", 
+    "contributor": "Bennett-Lynch", 
+    "type": "bugfix", 
+    "description": "[UrlConnectionHttpClient] Wrap erroneous NPE from HttpURLConnection as IOException"
+}

http-clients/url-connection-client/src/main/java/software/amazon/awssdk/http/urlconnection/UrlConnectionHttpClient.java

@@ -207,7 +207,7 @@ public HttpExecuteResponse call() throws IOException {
             request.contentStreamProvider().ifPresent(provider ->
                     invokeSafely(() -> IoUtils.copy(provider.newStream(), connection.getOutputStream())));
 
-            int responseCode = connection.getResponseCode();
+            int responseCode = getResponseCodeSafely(connection);
             boolean isErrorResponse = HttpStatusFamily.of(responseCode).isOneOf(CLIENT_ERROR, SERVER_ERROR);
             InputStream content = !isErrorResponse ? connection.getInputStream() : connection.getErrorStream();
             AbortableInputStream responseBody = content != null ?
@@ -224,6 +224,24 @@ public HttpExecuteResponse call() throws IOException {
                                       .build();
         }
 
+        /**
+         * {@link sun.net.www.protocol.http.HttpURLConnection#getInputStream0()} has been observed to intermittently throw
+         * {@link NullPointerException}s for reasons that still require further investigation, but are assumed to be due to a
+         * bug in the JDK. Propagating such NPEs is confusing for users and are not subject to being retried on by the default 
+         * retry policy configuration, so instead we bias towards propagating these as {@link IOException}s.
+         * <p>
+         * TODO: Determine precise root cause of intermittent NPEs, submit JDK bug report if applicable, and consider applying
+         * this behavior only on unpatched JVM runtime versions.
+         */
+        private static int getResponseCodeSafely(HttpURLConnection connection) throws IOException {
+            Validate.paramNotNull(connection, "connection");
+            try {
+                return connection.getResponseCode();
+            } catch (NullPointerException e) {
+                throw new IOException("Unexpected NullPointerException when trying to read response from HttpURLConnection", e);
+            }
+        }
+
         private Map<String, List<String>> extractHeaders(HttpURLConnection response) {
             return response.getHeaderFields().entrySet().stream()
                            .filter(e -> e.getKey() != null)

http-clients/url-connection-client/src/test/java/software/amazon/awssdk/http/urlconnection/UrlConnectionHttpClientWithCustomCreateWireMockTest.java

@@ -14,17 +14,30 @@
  */
 package software.amazon.awssdk.http.urlconnection;
 
+import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.spy;
 import static software.amazon.awssdk.utils.FunctionalUtils.invokeSafely;
+import static software.amazon.awssdk.utils.FunctionalUtils.safeFunction;
 
+import java.io.IOException;
 import java.net.HttpURLConnection;
+import java.util.function.Function;
 import org.junit.Ignore;
+import org.junit.Test;
 import software.amazon.awssdk.http.SdkHttpClient;
 import software.amazon.awssdk.http.SdkHttpClientTestSuite;
 
 public final class UrlConnectionHttpClientWithCustomCreateWireMockTest extends SdkHttpClientTestSuite {
+
+    private Function<HttpURLConnection, HttpURLConnection> connectionInterceptor = Function.identity();
+
     @Override
     protected SdkHttpClient createSdkHttpClient(SdkHttpClientOptions options) {
-        return UrlConnectionHttpClient.create((uri) -> invokeSafely(() -> (HttpURLConnection) uri.toURL().openConnection()));
+        return UrlConnectionHttpClient.create(uri -> invokeSafely(() -> {
+            HttpURLConnection connection = (HttpURLConnection) uri.toURL().openConnection();
+            return connectionInterceptor.apply(connection);
+        }));
     }
 
     @Ignore // Not supported when using custom factory
@@ -41,4 +54,18 @@ public void testTrustAllWorks() {
     @Override
     public void testCustomTlsTrustManagerAndTrustAllFails() {
     }
+
+    @Test
+    public void testGetResponseCodeNpeIsWrappedAsIo() throws Exception {
+        connectionInterceptor = safeFunction(connection -> {
+            connection = spy(connection);
+            doThrow(new NullPointerException()).when(connection).getResponseCode();
+            return connection;
+        });
+
+        assertThatThrownBy(() -> testForResponseCode(HttpURLConnection.HTTP_OK))
+            .isInstanceOf(IOException.class)
+            .hasMessage("Unexpected NullPointerException when trying to read response from HttpURLConnection")
+            .hasCauseInstanceOf(NullPointerException.class);
+    }
 }

test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientTestSuite.java

@@ -152,7 +152,7 @@ public void testCustomTlsTrustManagerAndTrustAllFails() throws Exception {
         assertThatThrownBy(() -> createSdkHttpClient(httpClientOptions)).isInstanceOf(IllegalArgumentException.class);
     }
 
-    private void testForResponseCode(int returnCode) throws Exception {
+    protected void testForResponseCode(int returnCode) throws Exception {
         testForResponseCode(returnCode, SdkHttpMethod.POST);
     }