From 8f9add1169ba270363e65f4cf1361fe061ad028b Mon Sep 17 00:00:00 2001 From: Maximilian Pfeffer Date: Sun, 26 Jul 2026 17:11:00 +0200 Subject: [PATCH] Fix #935 retry on IOExceptions too. Refactoring of Helper classes into a Chain of Responsibility pattern. --- .idea/.gitignore | 10 + .../msal4j/AbstractClientApplicationBase.java | 2 +- .../aad/msal4j/CorrelationIdRequestChain.java | 57 ++++ .../aad/msal4j/DefaultRetryPolicy.java | 2 +- .../DefaultRetryableExceptionPolicy.java | 35 +++ .../com/microsoft/aad/msal4j/HttpHelper.java | 291 ------------------ .../aad/msal4j/IMDSManagedIdentitySource.java | 5 +- .../{IHttpHelper.java => IRequestChain.java} | 16 +- .../aad/msal4j/IRetryableExceptionPolicy.java | 38 +++ .../msal4j/ManagedIdentityApplication.java | 2 +- .../aad/msal4j/OidcDiscoveryProvider.java | 2 +- .../aad/msal4j/RetryRequestChain.java | 70 +++++ .../aad/msal4j/SendRequestChain.java | 21 ++ .../microsoft/aad/msal4j/ServiceBundle.java | 6 +- .../ServiceFabricManagedIdentitySource.java | 9 +- .../aad/msal4j/TelemetryRequestChain.java | 98 ++++++ .../aad/msal4j/ThrottlingRequestChain.java | 162 ++++++++++ .../aad/msal4j/ManagedIdentityTests.java | 3 +- .../aad/msal4j/RetryRequestChainTest.java | 207 +++++++++++++ .../aad/msal4j/TokenRequestExecutorTest.java | 2 +- 20 files changed, 721 insertions(+), 317 deletions(-) create mode 100644 .idea/.gitignore create mode 100644 msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/CorrelationIdRequestChain.java create mode 100644 msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/DefaultRetryableExceptionPolicy.java delete mode 100644 msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/HttpHelper.java rename msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/{IHttpHelper.java => IRequestChain.java} (56%) create mode 100644 msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/IRetryableExceptionPolicy.java create mode 100644 msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/RetryRequestChain.java create mode 100644 msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/SendRequestChain.java create mode 100644 msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TelemetryRequestChain.java create mode 100644 msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ThrottlingRequestChain.java create mode 100644 msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/RetryRequestChainTest.java diff --git a/.idea/.gitignore b/.idea/.gitignore new file mode 100644 index 000000000..30cf57ed7 --- /dev/null +++ b/.idea/.gitignore @@ -0,0 +1,10 @@ +# Default ignored files +/shelf/ +/workspace.xml +# Editor-based HTTP Client requests +/httpRequests/ +# Ignored default folder with query files +/queries/ +# Datasource local storage ignored files +/dataSources/ +/dataSources.local.xml diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AbstractClientApplicationBase.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AbstractClientApplicationBase.java index f381d8084..849406f5e 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AbstractClientApplicationBase.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AbstractClientApplicationBase.java @@ -562,7 +562,7 @@ public T correlationId(String val) { super.serviceBundle = new ServiceBundle( builder.executorService, new TelemetryManager(telemetryConsumer, builder.onlySendFailureTelemetry), - new HttpHelper(this, new DefaultRetryPolicy()) + new ThrottlingRequestChain(this, new DefaultRetryPolicy()) ); if (aadAadInstanceDiscoveryResponse != null) { diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/CorrelationIdRequestChain.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/CorrelationIdRequestChain.java new file mode 100644 index 000000000..0be92fc75 --- /dev/null +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/CorrelationIdRequestChain.java @@ -0,0 +1,57 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.microsoft.aad.msal4j; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Correlation-id link of the HTTP request chain: verifies that the correlation id sent with the + * request matches the one returned in the response, once retries have been exhausted. + */ +class CorrelationIdRequestChain implements IRequestChain { + private static final Logger LOG = LoggerFactory.getLogger(CorrelationIdRequestChain.class); + + private final IRequestChain next; + + CorrelationIdRequestChain(IRequestChain next) { + this.next = next; + } + + @Override + public IHttpResponse executeHttpRequest(HttpRequest httpRequest, RequestContext requestContext, ServiceBundle serviceBundle) + throws Exception { + IHttpResponse httpResponse = next.executeHttpRequest(httpRequest, requestContext, serviceBundle); + + if (httpResponse.headers() != null) { + verifyReturnedCorrelationId(httpRequest, httpResponse); + } + + return httpResponse; + } + + private static void verifyReturnedCorrelationId(final HttpRequest httpRequest, + IHttpResponse httpResponse) { + + String sentCorrelationId = httpRequest.headerValue( + HttpHeaders.CORRELATION_ID_HEADER_NAME); + + String returnedCorrelationId = HttpUtils.headerValue( + httpResponse.headers(), + HttpHeaders.CORRELATION_ID_HEADER_NAME); + + if (StringHelper.isBlank(returnedCorrelationId) || + !returnedCorrelationId.equals(sentCorrelationId)) { + + String msg = LogHelper.createMessage( + String.format( + "Sent (%s) Correlation Id is not same as received (%s).", + sentCorrelationId, + returnedCorrelationId), + sentCorrelationId); + + LOG.info(msg); + } + } +} diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/DefaultRetryPolicy.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/DefaultRetryPolicy.java index 6465ffe76..c770883c9 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/DefaultRetryPolicy.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/DefaultRetryPolicy.java @@ -13,7 +13,7 @@ class DefaultRetryPolicy implements IRetryPolicy { @Override public boolean isRetryable(IHttpResponse httpResponse) { return HttpStatus.isServerError(httpResponse.statusCode()) && - HttpHelper.getRetryAfterHeader(httpResponse) == null; + ThrottlingRequestChain.getRetryAfterHeader(httpResponse) == null; } @Override diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/DefaultRetryableExceptionPolicy.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/DefaultRetryableExceptionPolicy.java new file mode 100644 index 000000000..321872331 --- /dev/null +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/DefaultRetryableExceptionPolicy.java @@ -0,0 +1,35 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.microsoft.aad.msal4j; + +import java.net.SocketException; +import java.net.SocketTimeoutException; +import java.net.UnknownHostException; +import javax.net.ssl.SSLException; + +/** + * Default policy for retrying HTTP requests that failed due to a network-level exception. + */ +class DefaultRetryableExceptionPolicy implements IRetryableExceptionPolicy { + private static final int RETRY_NUM = 1; + private static final int RETRY_DELAY_MS = 1000; + + @Override + public boolean isRetryable(Exception exception) { + if (exception instanceof SSLException || exception instanceof UnknownHostException) { + return false; + } + return exception instanceof SocketException || exception instanceof SocketTimeoutException; + } + + @Override + public int getMaxRetryCount(Exception exception) { + return RETRY_NUM; + } + + @Override + public int getRetryDelayMs(Exception exception) { + return RETRY_DELAY_MS; + } +} diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/HttpHelper.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/HttpHelper.java deleted file mode 100644 index 2f6da6ae5..000000000 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/HttpHelper.java +++ /dev/null @@ -1,291 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -package com.microsoft.aad.msal4j; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.TreeMap; -import java.util.TreeSet; - -import static com.microsoft.aad.msal4j.Constants.POINT_DELIMITER; - -class HttpHelper implements IHttpHelper { - - private static final Logger LOG = LoggerFactory.getLogger(HttpHelper.class); - public static final String RETRY_AFTER_HEADER = "Retry-After"; - - private IHttpClient httpClient; - private IRetryPolicy retryPolicy; - private boolean retryDisabled; - - HttpHelper(IHttpClient httpClient, IRetryPolicy retryPolicy) { - this.httpClient = httpClient; - this.retryPolicy = retryPolicy != null ? retryPolicy : new DefaultRetryPolicy(); - } - - HttpHelper(AbstractApplicationBase application, IRetryPolicy retryPolicy) { - this.httpClient = application.httpClient(); - this.retryDisabled = application.isRetryDisabled(); - this.retryPolicy = retryPolicy != null ? retryPolicy : new DefaultRetryPolicy(); - } - - public IHttpResponse executeHttpRequest(HttpRequest httpRequest, - RequestContext requestContext, - ServiceBundle serviceBundle) { - checkForThrottling(requestContext); - - HttpEvent httpEvent = new HttpEvent(); // for tracking http telemetry - IHttpResponse httpResponse; - - try (TelemetryHelper telemetryHelper = serviceBundle.getTelemetryManager().createTelemetryHelper( - requestContext.telemetryRequestId(), - requestContext.clientId(), - httpEvent, - false)) { - - addRequestInfoToTelemetry(httpRequest, httpEvent); - - try { - httpResponse = executeHttpRequestWithRetries(httpRequest, httpClient); - - } catch (Exception e) { - httpEvent.setOauthErrorCode(AuthenticationErrorCode.UNKNOWN); - throw new MsalClientException(e); - } - - addResponseInfoToTelemetry(httpResponse, httpEvent); - - if (httpResponse.headers() != null) { - HttpHelper.verifyReturnedCorrelationId(httpRequest, httpResponse); - } - } - processThrottlingInstructions(httpResponse, requestContext); - - return httpResponse; - } - - //Overloaded version of the more commonly used HTTP executor. It does not use ServiceBundle, allowing an HTTP call to be - // made only with more bespoke request-level parameters rather than those from the app-level ServiceBundle - IHttpResponse executeHttpRequest(HttpRequest httpRequest, - RequestContext requestContext, - TelemetryManager telemetryManager, - IHttpClient httpClient) { - checkForThrottling(requestContext); - - HttpEvent httpEvent = new HttpEvent(); // for tracking http telemetry - IHttpResponse httpResponse; - - try (TelemetryHelper telemetryHelper = telemetryManager.createTelemetryHelper( - requestContext.telemetryRequestId(), - requestContext.clientId(), - httpEvent, - false)) { - - addRequestInfoToTelemetry(httpRequest, httpEvent); - - try { - httpResponse = executeHttpRequestWithRetries(httpRequest, httpClient); - - } catch (Exception e) { - httpEvent.setOauthErrorCode(AuthenticationErrorCode.UNKNOWN); - throw new MsalClientException(e); - } - - addResponseInfoToTelemetry(httpResponse, httpEvent); - - if (httpResponse.headers() != null) { - HttpHelper.verifyReturnedCorrelationId(httpRequest, httpResponse); - } - } - processThrottlingInstructions(httpResponse, requestContext); - - return httpResponse; - } - - IHttpResponse executeHttpRequest(HttpRequest httpRequest) { - IHttpResponse httpResponse; - - try { - httpResponse = executeHttpRequestWithRetries(httpRequest, httpClient); - } catch (Exception e) { - throw new MsalClientException(e); - } - - if (httpResponse.headers() != null) { - HttpHelper.verifyReturnedCorrelationId(httpRequest, httpResponse); - } - - return httpResponse; - } - - private String getRequestThumbprint(RequestContext requestContext) { - StringBuilder sb = new StringBuilder(); - sb.append(requestContext.clientId() + POINT_DELIMITER); - sb.append(requestContext.authority() + POINT_DELIMITER); - - IAcquireTokenParameters apiParameters = requestContext.apiParameters(); - - if (apiParameters instanceof SilentParameters) { - IAccount account = ((SilentParameters) apiParameters).account(); - if (account != null) { - sb.append(account.homeAccountId() + POINT_DELIMITER); - } - } - - Set sortedScopes = new TreeSet<>(apiParameters.scopes()); - sb.append(String.join(" ", sortedScopes)); - - return StringHelper.createSha256Hash(sb.toString()); - } - - IHttpResponse executeHttpRequestWithRetries(HttpRequest httpRequest, IHttpClient httpClient) - throws Exception { - IHttpResponse httpResponse = httpClient.send(httpRequest); - - if (retryDisabled) { - return httpResponse; - } - - int retryCount = 0; - int maxRetries = retryPolicy.getMaxRetryCount(httpResponse); - - while (retryPolicy.isRetryable(httpResponse) && retryCount < maxRetries) { - Thread.sleep(retryPolicy.getRetryDelayMs(httpResponse)); - - retryCount++; - - httpResponse = httpClient.send(httpRequest); - } - - return httpResponse; - } - - private void checkForThrottling(RequestContext requestContext) { - if (requestContext.clientApplication() instanceof PublicClientApplication && - requestContext.apiParameters() != null) { - String requestThumbprint = getRequestThumbprint(requestContext); - - long retryInMs = ThrottlingCache.retryInMs(requestThumbprint); - - if (retryInMs > 0) { - throw new MsalThrottlingException(retryInMs); - } - } - } - - private void processThrottlingInstructions(IHttpResponse httpResponse, RequestContext requestContext) { - if (requestContext.clientApplication() instanceof PublicClientApplication) { - Long expirationTimestamp = null; - - Integer retryAfterHeaderVal = getRetryAfterHeader(httpResponse); - if (retryAfterHeaderVal != null) { - expirationTimestamp = System.currentTimeMillis() + retryAfterHeaderVal * 1000; - } else if (httpResponse.statusCode() == HttpStatus.HTTP_TOO_MANY_REQUESTS || - (httpResponse.statusCode() >= HttpStatus.HTTP_INTERNAL_ERROR)) { - - expirationTimestamp = System.currentTimeMillis() + ThrottlingCache.DEFAULT_THROTTLING_TIME_SEC * 1000; - } - if (expirationTimestamp != null) { - ThrottlingCache.set(getRequestThumbprint(requestContext), expirationTimestamp); - } - } - } - - static Integer getRetryAfterHeader(IHttpResponse httpResponse) { - - if (httpResponse.headers() != null) { - TreeMap> headers = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); - headers.putAll(httpResponse.headers()); - - if (headers.containsKey(RETRY_AFTER_HEADER) && headers.get(RETRY_AFTER_HEADER).size() == 1) { - try { - int headerValue = Integer.parseInt(headers.get(RETRY_AFTER_HEADER).get(0)); - - if (headerValue > 0 && headerValue <= ThrottlingCache.MAX_THROTTLING_TIME_SEC) { - return headerValue; - } - } catch (NumberFormatException ex) { - LOG.warn("Failed to parse value of Retry-After header - NumberFormatException"); - } - } - } - return null; - } - - private void addRequestInfoToTelemetry(final HttpRequest httpRequest, HttpEvent httpEvent) { - try { - httpEvent.setHttpPath(httpRequest.url().toURI()); - httpEvent.setHttpMethod(httpRequest.httpMethod().toString()); - if (!StringHelper.isBlank(httpRequest.url().getQuery())) { - httpEvent.setQueryParameters(httpRequest.url().getQuery()); - } - } catch (Exception ex) { - String correlationId = httpRequest.headerValue( - HttpHeaders.CORRELATION_ID_HEADER_NAME); - - LOG.warn(LogHelper.createMessage("Setting URL telemetry fields failed: " + - LogHelper.getPiiScrubbedDetails(ex), - correlationId != null ? correlationId : "")); - } - } - - private void addResponseInfoToTelemetry(IHttpResponse httpResponse, HttpEvent httpEvent) { - - httpEvent.setHttpResponseStatus(httpResponse.statusCode()); - - Map> headers = httpResponse.headers(); - - String userAgent = HttpUtils.headerValue(headers, "User-Agent"); - if (!StringHelper.isBlank(userAgent)) { - httpEvent.setUserAgent(userAgent); - } - - String xMsRequestId = HttpUtils.headerValue(headers, "x-ms-request-id"); - if (!StringHelper.isBlank(xMsRequestId)) { - httpEvent.setRequestIdHeader(xMsRequestId); - } - - String xMsClientTelemetry = HttpUtils.headerValue(headers, "x-ms-clitelem"); - if (xMsClientTelemetry != null) { - XmsClientTelemetryInfo xmsClientTelemetryInfo = - XmsClientTelemetryInfo.parseXmsTelemetryInfo(xMsClientTelemetry); - - if (xmsClientTelemetryInfo != null) { - httpEvent.setXmsClientTelemetryInfo(xmsClientTelemetryInfo); - } - } - } - - private static void verifyReturnedCorrelationId(final HttpRequest httpRequest, - IHttpResponse httpResponse) { - - String sentCorrelationId = httpRequest.headerValue( - HttpHeaders.CORRELATION_ID_HEADER_NAME); - - String returnedCorrelationId = HttpUtils.headerValue( - httpResponse.headers(), - HttpHeaders.CORRELATION_ID_HEADER_NAME); - - if (StringHelper.isBlank(returnedCorrelationId) || - !returnedCorrelationId.equals(sentCorrelationId)) { - - String msg = LogHelper.createMessage( - String.format( - "Sent (%s) Correlation Id is not same as received (%s).", - sentCorrelationId, - returnedCorrelationId), - sentCorrelationId); - - LOG.info(msg); - } - } - - void setRetryPolicy(IRetryPolicy retryPolicy) { - this.retryPolicy = retryPolicy; - } -} diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/IMDSManagedIdentitySource.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/IMDSManagedIdentitySource.java index 10bbbf818..4586d2fd6 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/IMDSManagedIdentitySource.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/IMDSManagedIdentitySource.java @@ -36,10 +36,7 @@ public IMDSManagedIdentitySource(MsalRequest msalRequest, IEnvironmentVariables environmentVariables = getEnvironmentVariables(); //IMDS uses a different retry policy than the default used in other MI flows - IHttpHelper httpHelper = serviceBundle.getHttpHelper(); - if (httpHelper instanceof HttpHelper) { - ((HttpHelper) httpHelper).setRetryPolicy(new IMDSRetryPolicy()); - } + serviceBundle.getHttpHelper().setRetryPolicy(new IMDSRetryPolicy()); if (!StringHelper.isNullOrBlank(environmentVariables.getEnvironmentVariable(Constants.AZURE_POD_IDENTITY_AUTHORITY_HOST))){ LOG.info("[Managed Identity] Environment variable AZURE_POD_IDENTITY_AUTHORITY_HOST for IMDS returned endpoint: {}", environmentVariables.getEnvironmentVariable(Constants.AZURE_POD_IDENTITY_AUTHORITY_HOST)); diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/IHttpHelper.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/IRequestChain.java similarity index 56% rename from msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/IHttpHelper.java rename to msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/IRequestChain.java index 099e40b9c..1abc8492f 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/IHttpHelper.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/IRequestChain.java @@ -4,25 +4,25 @@ package com.microsoft.aad.msal4j; /** - * Interface representing the HTTP helper component that handles network communications. + * Interface representing a link in the HTTP request chain (throttling, telemetry, correlation-id + * verification, retry, sending). *

- * This interface abstracts the HTTP communication layer used for sending requests. - * It's used internally by the library to execute HTTP requests during various operations. + * Each link in the chain implements this interface and delegates to its successor, referenced only + * through this interface, so that the request/response flows through all of them in sequence. */ -interface IHttpHelper { +interface IRequestChain { /** * Executes an HTTP request. - *

- * This method handles all aspects of sending the HTTP request and processing the response, - * such as applying retry policies and handling errors. * * @param httpRequest The HTTP request to be executed * @param requestContext Context information about the current request, including correlation IDs for telemetry * @param serviceBundle Bundle of services that may be needed during request execution, such as retry policies * @return An {@link IHttpResponse} object containing the response + * @throws Exception Implementations further down the chain (e.g. the actual send) may throw; implementations + * that handle/wrap exceptions themselves (e.g. telemetry) do not declare this. */ IHttpResponse executeHttpRequest(HttpRequest httpRequest, RequestContext requestContext, - ServiceBundle serviceBundle); + ServiceBundle serviceBundle) throws Exception; } diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/IRetryableExceptionPolicy.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/IRetryableExceptionPolicy.java new file mode 100644 index 000000000..b389c8c34 --- /dev/null +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/IRetryableExceptionPolicy.java @@ -0,0 +1,38 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.microsoft.aad.msal4j; + +/** + * Interface for policies deciding how HTTP request failures caused by exceptions + * (e.g. network-level {@link java.io.IOException}s) should be retried. + *

+ * This complements {@link IRetryPolicy}, which handles retries based on HTTP responses. + * Implementations decide whether a given exception represents a transient failure worth + * retrying, as opposed to a permanent error (e.g. TLS/certificate misconfiguration). + */ +interface IRetryableExceptionPolicy { + /** + * Determines whether a request should be retried based on the exception thrown. + * + * @param exception The exception thrown while attempting the request + * @return true if retry should be attempted, false otherwise + */ + boolean isRetryable(Exception exception); + + /** + * Gets the maximum number of retries to attempt based on the exception thrown. + * + * @param exception The exception thrown while attempting the request + * @return maximum retry count for this specific exception + */ + int getMaxRetryCount(Exception exception); + + /** + * Gets the delay in milliseconds to wait before the next retry attempt. + * + * @param exception The exception thrown while attempting the request + * @return delay in milliseconds before attempting the next retry + */ + int getRetryDelayMs(Exception exception); +} diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ManagedIdentityApplication.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ManagedIdentityApplication.java index 5385e9e0d..71319c92d 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ManagedIdentityApplication.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ManagedIdentityApplication.java @@ -37,7 +37,7 @@ private ManagedIdentityApplication(Builder builder) { super.serviceBundle = new ServiceBundle( builder.executorService, new TelemetryManager(telemetryConsumer, builder.onlySendFailureTelemetry), - new HttpHelper(this, new ManagedIdentityRetryPolicy()) + new ThrottlingRequestChain(this, new ManagedIdentityRetryPolicy()) ); log = LoggerFactory.getLogger(ManagedIdentityApplication.class); diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/OidcDiscoveryProvider.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/OidcDiscoveryProvider.java index 9ba37ec6a..fcc5da5c9 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/OidcDiscoveryProvider.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/OidcDiscoveryProvider.java @@ -10,7 +10,7 @@ static OidcDiscoveryResponse performOidcDiscovery(OidcAuthority authority, Abstr HttpMethod.GET, authority.canonicalAuthorityUrl.toString()); - IHttpResponse httpResponse = ((HttpHelper)clientApplication.serviceBundle.getHttpHelper()).executeHttpRequest(httpRequest); + IHttpResponse httpResponse = clientApplication.serviceBundle.getHttpHelper().executeHttpRequest(httpRequest); OidcDiscoveryResponse response = JsonHelper.convertJsonStringToJsonSerializableObject(httpResponse.body(), OidcDiscoveryResponse::fromJson); diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/RetryRequestChain.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/RetryRequestChain.java new file mode 100644 index 000000000..6b9ec1f3f --- /dev/null +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/RetryRequestChain.java @@ -0,0 +1,70 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.microsoft.aad.msal4j; + +/** + * Retry link of the HTTP request chain: retries the request, based on either the HTTP response + * or an exception thrown while sending, according to the configured retry policies. + */ +class RetryRequestChain implements IRequestChain { + private final IRequestChain next; + private IRetryPolicy retryPolicy; + private IRetryableExceptionPolicy retryableExceptionPolicy = new DefaultRetryableExceptionPolicy(); + private boolean retryDisabled; + + RetryRequestChain(IRequestChain next, IRetryPolicy retryPolicy, boolean retryDisabled) { + this.next = next; + this.retryPolicy = retryPolicy != null ? retryPolicy : new DefaultRetryPolicy(); + this.retryDisabled = retryDisabled; + } + + @Override + public IHttpResponse executeHttpRequest(HttpRequest httpRequest, RequestContext requestContext, ServiceBundle serviceBundle) + throws Exception { + IHttpResponse httpResponse = null; + Exception caughtException = null; + int retryCount = 0; + boolean retry; + + do { + try { + httpResponse = next.executeHttpRequest(httpRequest, requestContext, serviceBundle); + caughtException = null; + } catch (Exception e) { + httpResponse = null; + caughtException = e; + } + + retry = false; + + if (!retryDisabled) { + if (caughtException != null) { + if (retryCount < retryableExceptionPolicy.getMaxRetryCount(caughtException) + && retryableExceptionPolicy.isRetryable(caughtException)) { + Thread.sleep(retryableExceptionPolicy.getRetryDelayMs(caughtException)); + retryCount++; + retry = true; + } + } else { + if (retryCount < retryPolicy.getMaxRetryCount(httpResponse) + && retryPolicy.isRetryable(httpResponse)) { + Thread.sleep(retryPolicy.getRetryDelayMs(httpResponse)); + retryCount++; + retry = true; + } + } + } + } while (retry); + + if (caughtException != null) { + throw caughtException; + } + + return httpResponse; + } + + void setRetryPolicy(IRetryPolicy retryPolicy) { + this.retryPolicy = retryPolicy; + } +} diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/SendRequestChain.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/SendRequestChain.java new file mode 100644 index 000000000..2e9892d68 --- /dev/null +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/SendRequestChain.java @@ -0,0 +1,21 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.microsoft.aad.msal4j; + +/** + * Innermost link of the HTTP request chain: sends the request via the configured {@link IHttpClient}. + */ +class SendRequestChain implements IRequestChain { + private final IHttpClient httpClient; + + SendRequestChain(IHttpClient httpClient) { + this.httpClient = httpClient; + } + + @Override + public IHttpResponse executeHttpRequest(HttpRequest httpRequest, RequestContext requestContext, ServiceBundle serviceBundle) + throws Exception { + return httpClient.send(httpRequest); + } +} diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ServiceBundle.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ServiceBundle.java index 5559da7b9..3614b9b38 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ServiceBundle.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ServiceBundle.java @@ -9,10 +9,10 @@ class ServiceBundle { private ExecutorService executorService; private TelemetryManager telemetryManager; - private IHttpHelper httpHelper; + private ThrottlingRequestChain httpHelper; private ServerSideTelemetry serverSideTelemetry; - ServiceBundle(ExecutorService executorService, TelemetryManager telemetryManager, IHttpHelper httpHelper) { + ServiceBundle(ExecutorService executorService, TelemetryManager telemetryManager, ThrottlingRequestChain httpHelper) { this.executorService = executorService; this.telemetryManager = telemetryManager; this.httpHelper = httpHelper; @@ -28,7 +28,7 @@ TelemetryManager getTelemetryManager() { return telemetryManager; } - IHttpHelper getHttpHelper() { + ThrottlingRequestChain getHttpHelper() { return httpHelper; } diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ServiceFabricManagedIdentitySource.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ServiceFabricManagedIdentitySource.java index 9159c15ad..e38a4359f 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ServiceFabricManagedIdentitySource.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ServiceFabricManagedIdentitySource.java @@ -24,7 +24,7 @@ class ServiceFabricManagedIdentitySource extends AbstractManagedIdentitySource { //No other flow need this and an app developer may not be aware of it, so it was decided that for the Service Fabric flow we will simply override // any HttpClient that may have been set by the app developer with our own client which performs the validation logic. private static IHttpClient httpClient = new DefaultHttpClientManagedIdentity(null, null, null, null); - private static HttpHelper httpHelper = new HttpHelper(httpClient, new ManagedIdentityRetryPolicy()); + private static ThrottlingRequestChain httpHelper = new ThrottlingRequestChain(httpClient, new ManagedIdentityRetryPolicy()); @Override public void createManagedIdentityRequest(String resource) { @@ -70,8 +70,7 @@ public ManagedIdentityResponse getManagedIdentityResponse( managedIdentityRequest.headers, managedIdentityRequest.getBodyAsString()); - response = httpHelper.executeHttpRequest(httpRequest, managedIdentityRequest.requestContext(), serviceBundle.getTelemetryManager(), - httpClient); + response = httpHelper.executeHttpRequest(httpRequest, managedIdentityRequest.requestContext(), serviceBundle.getTelemetryManager()); } catch (URISyntaxException e) { throw new RuntimeException(e); } catch (MsalClientException e) { @@ -121,11 +120,11 @@ private static URI validateAndGetUri(String msiEndpoint) //However, unit tests often need to mock HttpClient and need a way to inject the mocked object into this class. static void setHttpClient(IHttpClient client) { httpClient = client; - httpHelper = new HttpHelper(httpClient, new ManagedIdentityRetryPolicy()); + httpHelper = new ThrottlingRequestChain(httpClient, new ManagedIdentityRetryPolicy()); } static void resetHttpClient() { httpClient = new DefaultHttpClientManagedIdentity(null, null, null, null); - httpHelper = new HttpHelper(httpClient, new ManagedIdentityRetryPolicy()); + httpHelper = new ThrottlingRequestChain(httpClient, new ManagedIdentityRetryPolicy()); } } \ No newline at end of file diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TelemetryRequestChain.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TelemetryRequestChain.java new file mode 100644 index 000000000..f5fff246c --- /dev/null +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TelemetryRequestChain.java @@ -0,0 +1,98 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.microsoft.aad.msal4j; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.List; +import java.util.Map; + +/** + * Telemetry link of the HTTP request chain: records request/response telemetry around the rest + * of the chain and converts any exception raised further down into an {@link MsalClientException}. + */ +class TelemetryRequestChain implements IRequestChain { + private static final Logger LOG = LoggerFactory.getLogger(TelemetryRequestChain.class); + + private final IRequestChain next; + + TelemetryRequestChain(IRequestChain next) { + this.next = next; + } + + @Override + public IHttpResponse executeHttpRequest(HttpRequest httpRequest, RequestContext requestContext, ServiceBundle serviceBundle) { + return executeHttpRequest(httpRequest, requestContext, serviceBundle.getTelemetryManager()); + } + + IHttpResponse executeHttpRequest(HttpRequest httpRequest, RequestContext requestContext, TelemetryManager telemetryManager) { + HttpEvent httpEvent = new HttpEvent(); // for tracking http telemetry + IHttpResponse httpResponse; + + try (TelemetryHelper telemetryHelper = telemetryManager.createTelemetryHelper( + requestContext.telemetryRequestId(), + requestContext.clientId(), + httpEvent, + false)) { + + addRequestInfoToTelemetry(httpRequest, httpEvent); + + try { + httpResponse = next.executeHttpRequest(httpRequest, requestContext, null); + } catch (Exception e) { + httpEvent.setOauthErrorCode(AuthenticationErrorCode.UNKNOWN); + throw new MsalClientException(e); + } + + addResponseInfoToTelemetry(httpResponse, httpEvent); + } + + return httpResponse; + } + + private void addRequestInfoToTelemetry(final HttpRequest httpRequest, HttpEvent httpEvent) { + try { + httpEvent.setHttpPath(httpRequest.url().toURI()); + httpEvent.setHttpMethod(httpRequest.httpMethod().toString()); + if (!StringHelper.isBlank(httpRequest.url().getQuery())) { + httpEvent.setQueryParameters(httpRequest.url().getQuery()); + } + } catch (Exception ex) { + String correlationId = httpRequest.headerValue( + HttpHeaders.CORRELATION_ID_HEADER_NAME); + + LOG.warn(LogHelper.createMessage("Setting URL telemetry fields failed: " + + LogHelper.getPiiScrubbedDetails(ex), + correlationId != null ? correlationId : "")); + } + } + + private void addResponseInfoToTelemetry(IHttpResponse httpResponse, HttpEvent httpEvent) { + + httpEvent.setHttpResponseStatus(httpResponse.statusCode()); + + Map> headers = httpResponse.headers(); + + String userAgent = HttpUtils.headerValue(headers, "User-Agent"); + if (!StringHelper.isBlank(userAgent)) { + httpEvent.setUserAgent(userAgent); + } + + String xMsRequestId = HttpUtils.headerValue(headers, "x-ms-request-id"); + if (!StringHelper.isBlank(xMsRequestId)) { + httpEvent.setRequestIdHeader(xMsRequestId); + } + + String xMsClientTelemetry = HttpUtils.headerValue(headers, "x-ms-clitelem"); + if (xMsClientTelemetry != null) { + XmsClientTelemetryInfo xmsClientTelemetryInfo = + XmsClientTelemetryInfo.parseXmsTelemetryInfo(xMsClientTelemetry); + + if (xmsClientTelemetryInfo != null) { + httpEvent.setXmsClientTelemetryInfo(xmsClientTelemetryInfo); + } + } + } +} diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ThrottlingRequestChain.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ThrottlingRequestChain.java new file mode 100644 index 000000000..77d79d3ec --- /dev/null +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ThrottlingRequestChain.java @@ -0,0 +1,162 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.microsoft.aad.msal4j; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.List; +import java.util.Set; +import java.util.TreeMap; +import java.util.TreeSet; + +import static com.microsoft.aad.msal4j.Constants.POINT_DELIMITER; + +/** + * Outermost link of the HTTP request chain: applies request throttling before delegating to the + * rest of the chain (telemetry, correlation-id verification, retry, send), and records throttling + * instructions from the response afterwards. + *

+ * This is also the composition root that wires up the rest of the chain, so unlike the other links + * it keeps direct references to specific chain members it needs for its bespoke entry points + * (the {@link TelemetryManager}-based overload, the correlation-id-only overload, and retry policy + * swapping) rather than only the generic {@link IRequestChain} successor. + */ +class ThrottlingRequestChain implements IRequestChain { + private static final Logger LOG = LoggerFactory.getLogger(ThrottlingRequestChain.class); + public static final String RETRY_AFTER_HEADER = "Retry-After"; + + private final TelemetryRequestChain telemetryChain; + private final CorrelationIdRequestChain correlationIdChain; + private final RetryRequestChain retryChain; + + ThrottlingRequestChain(IHttpClient httpClient, IRetryPolicy retryPolicy) { + this(new SendRequestChain(httpClient), retryPolicy, false); + } + + ThrottlingRequestChain(AbstractApplicationBase application, IRetryPolicy retryPolicy) { + this(new SendRequestChain(application.httpClient()), retryPolicy, application.isRetryDisabled()); + } + + private ThrottlingRequestChain(SendRequestChain sendChain, IRetryPolicy retryPolicy, boolean retryDisabled) { + this.retryChain = new RetryRequestChain(sendChain, retryPolicy, retryDisabled); + this.correlationIdChain = new CorrelationIdRequestChain(retryChain); + this.telemetryChain = new TelemetryRequestChain(correlationIdChain); + } + + @Override + public IHttpResponse executeHttpRequest(HttpRequest httpRequest, + RequestContext requestContext, + ServiceBundle serviceBundle) { + checkForThrottling(requestContext); + + IHttpResponse httpResponse = telemetryChain.executeHttpRequest(httpRequest, requestContext, serviceBundle); + + processThrottlingInstructions(httpResponse, requestContext); + + return httpResponse; + } + + //Overloaded version of the more commonly used HTTP executor. It does not use ServiceBundle, allowing an HTTP call to be + // made only with more bespoke request-level parameters rather than those from the app-level ServiceBundle + IHttpResponse executeHttpRequest(HttpRequest httpRequest, + RequestContext requestContext, + TelemetryManager telemetryManager) { + checkForThrottling(requestContext); + + IHttpResponse httpResponse = telemetryChain.executeHttpRequest(httpRequest, requestContext, telemetryManager); + + processThrottlingInstructions(httpResponse, requestContext); + + return httpResponse; + } + + IHttpResponse executeHttpRequest(HttpRequest httpRequest) { + IHttpResponse httpResponse; + + try { + httpResponse = correlationIdChain.executeHttpRequest(httpRequest, null, null); + } catch (Exception e) { + throw new MsalClientException(e); + } + + return httpResponse; + } + + void setRetryPolicy(IRetryPolicy retryPolicy) { + retryChain.setRetryPolicy(retryPolicy); + } + + private String getRequestThumbprint(RequestContext requestContext) { + StringBuilder sb = new StringBuilder(); + sb.append(requestContext.clientId() + POINT_DELIMITER); + sb.append(requestContext.authority() + POINT_DELIMITER); + + IAcquireTokenParameters apiParameters = requestContext.apiParameters(); + + if (apiParameters instanceof SilentParameters) { + IAccount account = ((SilentParameters) apiParameters).account(); + if (account != null) { + sb.append(account.homeAccountId() + POINT_DELIMITER); + } + } + + Set sortedScopes = new TreeSet<>(apiParameters.scopes()); + sb.append(String.join(" ", sortedScopes)); + + return StringHelper.createSha256Hash(sb.toString()); + } + + private void checkForThrottling(RequestContext requestContext) { + if (requestContext.clientApplication() instanceof PublicClientApplication && + requestContext.apiParameters() != null) { + String requestThumbprint = getRequestThumbprint(requestContext); + + long retryInMs = ThrottlingCache.retryInMs(requestThumbprint); + + if (retryInMs > 0) { + throw new MsalThrottlingException(retryInMs); + } + } + } + + private void processThrottlingInstructions(IHttpResponse httpResponse, RequestContext requestContext) { + if (requestContext.clientApplication() instanceof PublicClientApplication) { + Long expirationTimestamp = null; + + Integer retryAfterHeaderVal = getRetryAfterHeader(httpResponse); + if (retryAfterHeaderVal != null) { + expirationTimestamp = System.currentTimeMillis() + retryAfterHeaderVal * 1000; + } else if (httpResponse.statusCode() == HttpStatus.HTTP_TOO_MANY_REQUESTS || + (httpResponse.statusCode() >= HttpStatus.HTTP_INTERNAL_ERROR)) { + + expirationTimestamp = System.currentTimeMillis() + ThrottlingCache.DEFAULT_THROTTLING_TIME_SEC * 1000; + } + if (expirationTimestamp != null) { + ThrottlingCache.set(getRequestThumbprint(requestContext), expirationTimestamp); + } + } + } + + static Integer getRetryAfterHeader(IHttpResponse httpResponse) { + + if (httpResponse.headers() != null) { + TreeMap> headers = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); + headers.putAll(httpResponse.headers()); + + if (headers.containsKey(RETRY_AFTER_HEADER) && headers.get(RETRY_AFTER_HEADER).size() == 1) { + try { + int headerValue = Integer.parseInt(headers.get(RETRY_AFTER_HEADER).get(0)); + + if (headerValue > 0 && headerValue <= ThrottlingCache.MAX_THROTTLING_TIME_SEC) { + return headerValue; + } + } catch (NumberFormatException ex) { + LOG.warn("Failed to parse value of Retry-After header - NumberFormatException"); + } + } + } + return null; + } +} diff --git a/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ManagedIdentityTests.java b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ManagedIdentityTests.java index 04db8c693..88163a634 100644 --- a/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ManagedIdentityTests.java +++ b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ManagedIdentityTests.java @@ -815,7 +815,8 @@ void managedIdentity_RequestFailed_UnreachableNetwork(ManagedIdentitySourceType assertMsalServiceException(acquireTokenCommon(ManagedIdentityTestConstants.RESOURCE), source, MsalError.MANAGED_IDENTITY_UNREACHABLE_NETWORK); - verify(httpClientMock, times(1)).send(any()); + // SocketException is now retried once (DefaultRetryableExceptionPolicy) before failing + verify(httpClientMock, times(2)).send(any()); } @ParameterizedTest diff --git a/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/RetryRequestChainTest.java b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/RetryRequestChainTest.java new file mode 100644 index 000000000..478ac3456 --- /dev/null +++ b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/RetryRequestChainTest.java @@ -0,0 +1,207 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.microsoft.aad.msal4j; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.net.SocketException; + +import javax.net.ssl.SSLException; + +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class RetryRequestChainTest { + + private final HttpRequest httpRequest = mock(HttpRequest.class); + private final IRequestChain next = mock(IRequestChain.class); + private final IRetryPolicy retryPolicy = mock(IRetryPolicy.class); + + private static HttpResponse response(int statusCode) { + return new HttpResponse().statusCode(statusCode); + } + + @Test + void responseRetry_SucceedsOnSecondAttempt() throws Exception { + HttpResponse retryableResponse = response(HttpStatus.HTTP_INTERNAL_ERROR); + HttpResponse finalResponse = response(HttpStatus.HTTP_OK); + + when(next.executeHttpRequest(any(), any(), any())) + .thenReturn(retryableResponse) + .thenReturn(finalResponse); + when(retryPolicy.isRetryable(retryableResponse)).thenReturn(true); + when(retryPolicy.isRetryable(finalResponse)).thenReturn(false); + when(retryPolicy.getMaxRetryCount(any())).thenReturn(2); + when(retryPolicy.getRetryDelayMs(any())).thenReturn(1); + + RetryRequestChain chain = new RetryRequestChain(next, retryPolicy, false); + + IHttpResponse result = chain.executeHttpRequest(httpRequest, null, null); + + assertSame(finalResponse, result); + verify(next, times(2)).executeHttpRequest(any(), any(), any()); + } + + @Test + void responseRetry_ExhaustsRetries_ReturnsLastResponseWithoutThrowing() throws Exception { + HttpResponse retryableResponse = response(HttpStatus.HTTP_INTERNAL_ERROR); + + when(next.executeHttpRequest(any(), any(), any())).thenReturn(retryableResponse); + when(retryPolicy.isRetryable(retryableResponse)).thenReturn(true); + when(retryPolicy.getMaxRetryCount(any())).thenReturn(2); + when(retryPolicy.getRetryDelayMs(any())).thenReturn(1); + + RetryRequestChain chain = new RetryRequestChain(next, retryPolicy, false); + + IHttpResponse result = chain.executeHttpRequest(httpRequest, null, null); + + assertSame(retryableResponse, result); + // 1 initial attempt + 2 retries + verify(next, times(3)).executeHttpRequest(any(), any(), any()); + } + + @Test + void exceptionRetry_SucceedsOnSecondAttempt() throws Exception { + HttpResponse finalResponse = response(HttpStatus.HTTP_OK); + + when(next.executeHttpRequest(any(), any(), any())) + .thenThrow(new SocketException("connection reset")) + .thenReturn(finalResponse); + when(retryPolicy.isRetryable(finalResponse)).thenReturn(false); + + RetryRequestChain chain = new RetryRequestChain(next, retryPolicy, false); + + IHttpResponse result = chain.executeHttpRequest(httpRequest, null, null); + + assertSame(finalResponse, result); + verify(next, times(2)).executeHttpRequest(any(), any(), any()); + } + + @Test + void exceptionRetry_ExhaustsRetries_RethrowsSameException() throws Exception { + SocketException socketException = new SocketException("connection reset"); + + when(next.executeHttpRequest(any(), any(), any())).thenThrow(socketException); + + RetryRequestChain chain = new RetryRequestChain(next, retryPolicy, false); + + Exception thrown = assertThrows(SocketException.class, + () -> chain.executeHttpRequest(httpRequest, null, null)); + + assertSame(socketException, thrown); + // DefaultRetryableExceptionPolicy allows exactly 1 retry: 1 initial attempt + 1 retry + verify(next, times(2)).executeHttpRequest(any(), any(), any()); + } + + @Test + void nonRetryableException_RethrownImmediately() throws Exception { + SSLException sslException = new SSLException("handshake failed"); + + when(next.executeHttpRequest(any(), any(), any())).thenThrow(sslException); + + RetryRequestChain chain = new RetryRequestChain(next, retryPolicy, false); + + Exception thrown = assertThrows(SSLException.class, + () -> chain.executeHttpRequest(httpRequest, null, null)); + + assertSame(sslException, thrown); + verify(next, times(1)).executeHttpRequest(any(), any(), any()); + } + + @Test + void retryDisabled_RetryableResponseIsNotRetried() throws Exception { + HttpResponse retryableResponse = response(HttpStatus.HTTP_INTERNAL_ERROR); + + when(next.executeHttpRequest(any(), any(), any())).thenReturn(retryableResponse); + + RetryRequestChain chain = new RetryRequestChain(next, retryPolicy, true); + + IHttpResponse result = chain.executeHttpRequest(httpRequest, null, null); + + assertSame(retryableResponse, result); + verify(next, times(1)).executeHttpRequest(any(), any(), any()); + verify(retryPolicy, never()).isRetryable(any()); + } + + @Test + void retryDisabled_RetryableExceptionIsRethrownImmediately() throws Exception { + SocketException socketException = new SocketException("connection reset"); + + when(next.executeHttpRequest(any(), any(), any())).thenThrow(socketException); + + RetryRequestChain chain = new RetryRequestChain(next, retryPolicy, true); + + Exception thrown = assertThrows(SocketException.class, + () -> chain.executeHttpRequest(httpRequest, null, null)); + + assertSame(socketException, thrown); + verify(next, times(1)).executeHttpRequest(any(), any(), any()); + } + + @Test + void transitionExceptionThenResponse_ReturnsFinalResponse() throws Exception { + HttpResponse finalResponse = response(HttpStatus.HTTP_OK); + + when(next.executeHttpRequest(any(), any(), any())) + .thenThrow(new SocketException("connection reset")) + .thenReturn(finalResponse); + when(retryPolicy.isRetryable(finalResponse)).thenReturn(false); + + RetryRequestChain chain = new RetryRequestChain(next, retryPolicy, false); + + IHttpResponse result = chain.executeHttpRequest(httpRequest, null, null); + + assertSame(finalResponse, result); + verify(next, times(2)).executeHttpRequest(any(), any(), any()); + } + + @Test + void transitionResponseThenException_RethrowsImmediately() throws Exception { + HttpResponse retryableResponse = response(HttpStatus.HTTP_INTERNAL_ERROR); + SSLException sslException = new SSLException("handshake failed"); + + when(next.executeHttpRequest(any(), any(), any())) + .thenReturn(retryableResponse) + .thenThrow(sslException); + when(retryPolicy.isRetryable(retryableResponse)).thenReturn(true); + when(retryPolicy.getMaxRetryCount(any())).thenReturn(2); + when(retryPolicy.getRetryDelayMs(any())).thenReturn(1); + + RetryRequestChain chain = new RetryRequestChain(next, retryPolicy, false); + + Exception thrown = assertThrows(SSLException.class, + () -> chain.executeHttpRequest(httpRequest, null, null)); + + assertSame(sslException, thrown); + verify(next, times(2)).executeHttpRequest(any(), any(), any()); + } + + @Test + void exceptionRetry_NeverCallsResponseBasedRetryPolicyWithNullResponse() throws Exception { + HttpResponse finalResponse = response(HttpStatus.HTTP_OK); + + when(next.executeHttpRequest(any(), any(), any())) + .thenThrow(new SocketException("connection reset")) + .thenReturn(finalResponse); + when(retryPolicy.isRetryable(finalResponse)).thenReturn(false); + + RetryRequestChain chain = new RetryRequestChain(next, retryPolicy, false); + + chain.executeHttpRequest(httpRequest, null, null); + + // retryPolicy is only ever allowed to see a response, never a null caused by the caught exception + verify(retryPolicy, never()).isRetryable(null); + verify(retryPolicy, never()).getMaxRetryCount(null); + verify(retryPolicy, never()).getRetryDelayMs(null); + } +} diff --git a/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/TokenRequestExecutorTest.java b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/TokenRequestExecutorTest.java index ac7437559..d58bc87ea 100644 --- a/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/TokenRequestExecutorTest.java +++ b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/TokenRequestExecutorTest.java @@ -121,7 +121,7 @@ private TokenRequestExecutor createMockedTokenRequest() throws MalformedURLExcep ServiceBundle serviceBundle = new ServiceBundle( null, new TelemetryManager(null, false), - new HttpHelper(new DefaultHttpClient(null, null, null, null), new DefaultRetryPolicy())); + new ThrottlingRequestChain(new DefaultHttpClient(null, null, null, null), new DefaultRetryPolicy())); return spy(new TokenRequestExecutor( new AADAuthority(new URL(TestConstants.ORGANIZATIONS_AUTHORITY)), refreshTokenRequest, serviceBundle));