Merge branch 'dev' into bugfix/1991-auth-fragment-is-not-opened-with-extras

VolodaUa · web-flow · commit 718b1417f182 · 2024-12-12T09:37:12.000+02:00
diff --git a/changelog.txt b/changelog.txt
@@ -1,6 +1,6 @@
 vNext
 ----------
-- [MINOR] Add suberror for network errors (#2537)
+- [MAJOR] Add suberror for network errors (#2537)
 - [PATCH] Translate MFA token error to UIRequiredException instead of ServiceException (#2538)
 - [MINOR] Add Child Spans for Interactive Span (#2516)
 - [MINOR] For MSAL CPP flows, match exact claims when deleting AT with intersecting scopes (#2548)
diff --git a/common4j/src/main/com/microsoft/identity/common/java/exception/ConnectionError.kt b/common4j/src/main/com/microsoft/identity/common/java/exception/ConnectionError.kt
@@ -22,27 +22,19 @@
 // THE SOFTWARE.
 package com.microsoft.identity.common.java.exception
 
+import java.io.EOFException
+import java.net.ConnectException
+import java.net.SocketException
+import java.net.SocketTimeoutException
+import java.net.UnknownHostException
+import javax.net.ssl.SSLException
+
 enum class ConnectionError(val value: String) {
-    FAILED_TO_OPEN_CONNECTION("ce_failed_to_open_connection"),
-    FAILED_TO_SET_REQUEST_METHOD("ce_failed_to_set_request_method"),
-    FAILED_TO_WRITE_TO_OUTPUT_STREAM("ce_failed_to_write_to_output_stream"),
-    FAILED_TO_READ_FROM_INPUT_STREAM("ce_failed_to_read_from_input_stream"),
-    FAILED_TO_GET_RESPONSE_CODE("ce_failed_to_get_response_code"),
+    NO_NETWORK("ce_no_network"),
+    NETWORK_TEMPORARILY_UNAVAILABLE("ce_network_temporarily_unavailable"),
+    UNEXPECTED_EXCEPTION("ce_unexpected_exception"),
     CONNECTION_TIMEOUT("ce_connection_timeout");
 
-    /**
-     * Converts this [ConnectionError] into a [ClientException]
-     **/
-    fun getClientException(cause: Throwable): ClientException {
-        val e = ClientException(
-            ClientException.IO_ERROR,
-            "An IO error occurred in the network layer: " + cause.message,
-            cause
-        )
-        e.subErrorCode = value
-        return e
-    }
-
     /**
      * Returns true if the given [Throwable] is a connection error.
      **/
@@ -53,4 +45,46 @@ enum class ConnectionError(val value: String) {
 
         return this.value == throwable.subErrorCode
     }
+
+    companion object {
+        /**
+         * Converts this [ConnectionError] into a [ClientException]
+         **/
+        @JvmStatic
+        fun getClientException(cause: Throwable): ClientException {
+            val e = ClientException(
+                ClientException.IO_ERROR,
+                "An IO error occurred in the network layer: " + cause.message,
+                cause
+            )
+
+            e.subErrorCode = getConnectionError(cause).value
+            return e
+        }
+
+        /**
+         * Converts a [Throwable] into a suberrorCode
+         **/
+        private fun getConnectionError(cause: Throwable): ConnectionError {
+            if (cause is SocketTimeoutException) {
+                // Slow or unreliable network
+                return CONNECTION_TIMEOUT
+            }
+
+            if (cause is EOFException // Unexpected disconnect
+                || cause is SSLException // SSL Handshake failed
+                || cause is ConnectException // Remote socket connection failed
+            ) {
+                return NETWORK_TEMPORARILY_UNAVAILABLE
+            }
+
+            if (cause is UnknownHostException // Unable to query DNS for hostname
+                || cause is SocketException // No conn to remote socket, or Airplane mode, or no internet permission will hit this
+            ) {
+                return NO_NETWORK
+            }
+
+            return UNEXPECTED_EXCEPTION
+        }
+    }
 }
diff --git a/common4j/src/main/com/microsoft/identity/common/java/net/UrlConnectionHttpClient.java b/common4j/src/main/com/microsoft/identity/common/java/net/UrlConnectionHttpClient.java
@@ -48,14 +48,19 @@
 
 import java.io.BufferedReader;
 import java.io.Closeable;
+import java.io.EOFException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.io.OutputStream;
+import java.net.ConnectException;
 import java.net.HttpURLConnection;
+import java.net.NoRouteToHostException;
 import java.net.ProtocolException;
+import java.net.SocketException;
 import java.net.SocketTimeoutException;
 import java.net.URL;
+import java.net.UnknownHostException;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
@@ -66,6 +71,7 @@
 import javax.annotation.Nullable;
 import javax.net.ssl.HttpsURLConnection;
 import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLException;
 import javax.net.ssl.SSLSocketFactory;
 
 import io.opentelemetry.api.trace.Span;
@@ -313,7 +319,7 @@ private static HttpRequest constructHttpRequest(@NonNull HttpClient.HttpMethod h
      * @return The converted string
      * @throws IOException Thrown when failing to access inputStream stream.
      */
-    private String convertStreamToString(final InputStream inputStream) throws ClientException {
+    private String convertStreamToString(final InputStream inputStream) throws IOException {
         try {
             final BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream,
                     AuthenticationConstants.CHARSET_UTF8));
@@ -326,8 +332,6 @@ private String convertStreamToString(final InputStream inputStream) throws Clien
             }
 
             return stringBuilder.toString();
-        } catch (IOException e) {
-            throw ConnectionError.FAILED_TO_READ_FROM_INPUT_STREAM.getClientException(e);
         } finally {
             safeCloseStream(inputStream);
         }
@@ -353,10 +357,17 @@ private static void safeCloseStream(final Closeable stream) {
     }
 
     private HttpResponse executeHttpSend(HttpRequest request, Consumer<HttpResponse> completionCallback) throws ClientException {
-        final HttpURLConnection urlConnection = setupConnection(request);
-
-        sendRequest(urlConnection, request.getRequestContent(), request.getRequestHeaders().get(HttpConstants.HeaderField.CONTENT_TYPE));
+        try {
+            final HttpURLConnection urlConnection = setupConnection(request);
+            sendRequest(urlConnection, request.getRequestContent(), request.getRequestHeaders().get(HttpConstants.HeaderField.CONTENT_TYPE));
+            return getHttpResponse(completionCallback, urlConnection);
+        }  catch (final IOException e) {
+            throw ConnectionError.getClientException(e);
+        }
+    }
 
+    private HttpResponse getHttpResponse(Consumer<HttpResponse> completionCallback,
+                                         HttpURLConnection urlConnection) throws IOException {
         InputStream responseStream = null;
         HttpResponse response = null;
         try {
@@ -366,18 +377,13 @@ private HttpResponse executeHttpSend(HttpRequest request, Consumer<HttpResponse>
                 // SocketTimeoutExcetion is thrown when connection timeout happens. For connection
                 // timeout, we want to retry once. Throw the exception to the upper layer, and the
                 // upper layer will handle the retry.
-                throw ConnectionError.CONNECTION_TIMEOUT.getClientException(e);
+                throw e;
             } catch (final IOException ioException) {
                 // 404, for example, will generate an exception.  We should catch it.
                 responseStream = urlConnection.getErrorStream();
             }
 
-            final int statusCode;
-            try {
-                statusCode = urlConnection.getResponseCode();
-            } catch (IOException e) {
-                throw ConnectionError.FAILED_TO_GET_RESPONSE_CODE.getClientException(e);
-            }
+            final int statusCode = urlConnection.getResponseCode();
 
             final Date date = new Date(urlConnection.getDate());
 
@@ -401,12 +407,12 @@ private HttpResponse executeHttpSend(HttpRequest request, Consumer<HttpResponse>
                 );
 
                 span.setAttribute(
-                        com.microsoft.identity.common.java.opentelemetry.AttributeName.ccs_request_id.name(),
+                        AttributeName.ccs_request_id.name(),
                         response.getHeaderValue(XMS_CCS_REQUEST_ID, 0)
                 );
 
                 span.setAttribute(
-                        com.microsoft.identity.common.java.opentelemetry.AttributeName.ccs_request_sequence.name(),
+                        AttributeName.ccs_request_sequence.name(),
                         response.getHeaderValue(XMS_CCS_REQUEST_SEQUENCE, 0)
                 );
             }
@@ -419,7 +425,6 @@ private HttpResponse executeHttpSend(HttpRequest request, Consumer<HttpResponse>
                     AttributeName.http_status_code.name(),
                     response.getStatusCode()
             );
-
         } finally {
             completionCallback.accept(response);
             safeCloseStream(responseStream);
@@ -428,14 +433,9 @@ private HttpResponse executeHttpSend(HttpRequest request, Consumer<HttpResponse>
         return response;
     }
 
-    private HttpURLConnection setupConnection(HttpRequest request) throws ClientException {
+    private HttpURLConnection setupConnection(HttpRequest request) throws IOException {
         final String methodName = ":setupConnection";
-        final HttpURLConnection urlConnection;
-        try {
-            urlConnection = HttpUrlConnectionFactory.createHttpURLConnection(request.getRequestUrl());
-        } catch (IOException e) {
-            throw ConnectionError.FAILED_TO_OPEN_CONNECTION.getClientException(e);
-        }
+        final HttpURLConnection urlConnection = HttpUrlConnectionFactory.createHttpURLConnection(request.getRequestUrl());
 
         // Apply request headers and update the headers with default attributes first
         final Set<Map.Entry<String, String>> headerEntries = request.getRequestHeaders().entrySet();
@@ -454,12 +454,7 @@ private HttpURLConnection setupConnection(HttpRequest request) throws ClientExce
             Logger.warn(TAG + methodName, "gets a request from an unexpected protocol: " + request.getRequestUrl().getProtocol());
         }
 
-        try {
-            urlConnection.setRequestMethod(request.getRequestMethod());
-        } catch (ProtocolException e) {
-            throw ConnectionError.FAILED_TO_SET_REQUEST_METHOD.getClientException(e);
-        }
-
+        urlConnection.setRequestMethod(request.getRequestMethod());
         urlConnection.setConnectTimeout(getConnectTimeoutMs());
         urlConnection.setReadTimeout(getReadTimeoutMs());
         urlConnection.setInstanceFollowRedirects(true);
@@ -479,7 +474,7 @@ private int getConnectTimeoutMs() {
 
     private static void sendRequest(@NonNull final HttpURLConnection connection,
                                     final byte[] contentRequest,
-                                    final String requestContentType) throws ClientException {
+                                    final String requestContentType) throws IOException {
         if (contentRequest == null) {
             return;
         }
@@ -497,8 +492,6 @@ private static void sendRequest(@NonNull final HttpURLConnection connection,
         try {
             out = connection.getOutputStream();
             out.write(contentRequest);
-        } catch (IOException e) {
-            throw ConnectionError.FAILED_TO_WRITE_TO_OUTPUT_STREAM.getClientException(e);
         } finally {
             safeCloseStream(out);
         }
diff --git a/common4j/src/test/com/microsoft/identity/common/java/net/UrlConnectionHttpClientTest.java b/common4j/src/test/com/microsoft/identity/common/java/net/UrlConnectionHttpClientTest.java
@@ -1218,7 +1218,7 @@ public void testConnectingToTLS13ServerWhileEnforcing12OnClientSide() throws Cli
             );
             Assert.fail();
         } catch (ClientException e){
-            Assert.assertTrue(ConnectionError.FAILED_TO_GET_RESPONSE_CODE.compare(e));
+            Assert.assertTrue(ConnectionError.NETWORK_TEMPORARILY_UNAVAILABLE.compare(e));
         }
     }
 

Original file line number	Diff line number	Diff line change
`@@ -1218,7 +1218,7 @@ public void testConnectingToTLS13ServerWhileEnforcing12OnClientSide() throws Cli`
`1218`	`1218`	`);`
`1219`	`1219`	`Assert.fail();`
`1220`	`1220`	`} catch (ClientException e){`
`1221`		`- Assert.assertTrue(ConnectionError.FAILED_TO_GET_RESPONSE_CODE.compare(e));`
	`1221`	`+ Assert.assertTrue(ConnectionError.NETWORK_TEMPORARILY_UNAVAILABLE.compare(e));`
`1222`	`1222`	`}`
`1223`	`1223`	`}`
`1224`	`1224`