Skip to content

Commit 8518d87

Browse files
authored
Fix NPE in AsyncRetryableStage (#6936)
When retrying and checking for the 'x-amz-retry-after' or 'retry-after' header on the response, ensure that the response is not null first.
1 parent a966125 commit 8518d87

3 files changed

Lines changed: 75 additions & 0 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"type": "bugfix",
3+
"category": "AWS SDK for Java v2",
4+
"contributor": "",
5+
"description": "Fix an issue in the async SDK clients where a retry can lead to a `NullPointerException` if the exception that the SDK encountered did not originate from the service, such as a connection exception."
6+
}

core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/pipeline/stages/AsyncRetryableStage.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,10 @@ private Duration suggestedDelay() {
192192
* Returns the suggested backoff delay based on the 'x-amz-retry-after' header value in the response.
193193
*/
194194
private Optional<Duration> xAmzRetryAfter(SdkHttpResponse response) {
195+
if (response == null) {
196+
return Optional.empty();
197+
}
198+
195199
Optional<String> optionalXAmzRetryAfter = response.firstMatchingHeader(X_AMZ_RETRY_AFTER_HEADER);
196200
return optionalXAmzRetryAfter.map(xAmzRetryAfter -> {
197201
try {

core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/http/pipeline/stages/AsyncRetryableStageTest.java

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import static org.mockito.Mockito.verify;
2323
import static org.mockito.Mockito.when;
2424

25+
import java.io.IOException;
2526
import java.net.URI;
2627
import java.time.Duration;
2728
import java.util.concurrent.CompletableFuture;
@@ -33,6 +34,7 @@
3334
import org.junit.jupiter.api.BeforeAll;
3435
import org.junit.jupiter.api.BeforeEach;
3536
import org.junit.jupiter.params.ParameterizedTest;
37+
import org.junit.jupiter.params.provider.CsvSource;
3638
import org.junit.jupiter.params.provider.MethodSource;
3739
import org.mockito.ArgumentCaptor;
3840
import software.amazon.awssdk.core.Response;
@@ -233,4 +235,67 @@ void execute_retryableException_treatsRetryAfterCorrectly(RetryAfterTestCase tes
233235

234236
assertThat(refreshRequest.suggestedDelay().get()).isEqualTo(testCase.expectedDelay());
235237
}
238+
239+
@ParameterizedTest(name = "New Retries = {0}")
240+
@CsvSource({"true", "false"})
241+
void execute_delegateThrows_noHttpResponse_uses0SuggestedDelay(boolean newRetries2026) throws Exception {
242+
SdkClientConfiguration clientConfig = SdkClientConfiguration.builder()
243+
.option(SdkClientOption.RETRY_STRATEGY, mockRetryStrategy)
244+
.option(SdkClientOption.SCHEDULED_EXECUTOR_SERVICE,
245+
executorService)
246+
.build();
247+
248+
HttpClientDependencies deps = HttpClientDependencies.builder()
249+
.clientConfiguration(clientConfig)
250+
.build();
251+
252+
AsyncRetryableStage<SdkResponse> retryableStage = new AsyncRetryableStage<>(mock(TransformingAsyncResponseHandler.class),
253+
deps, mockDelegatePipeline);
254+
255+
SdkHttpFullRequest httpRequest = SdkHttpFullRequest.builder()
256+
.method(SdkHttpMethod.GET)
257+
.uri(URI.create("https://my-service.amazonaws.com"))
258+
.build();
259+
260+
ExecutionAttributes execAttrs = ExecutionAttributes.builder()
261+
.put(SdkInternalExecutionAttribute.NEW_RETRIES_2026_ENABLED,
262+
newRetries2026)
263+
.build();
264+
265+
ExecutionContext execCtx = ExecutionContext.builder()
266+
.metricCollector(NoOpMetricCollector.create())
267+
.executionAttributes(execAttrs)
268+
.build();
269+
270+
RequestExecutionContext ctx = RequestExecutionContext.builder()
271+
.originalRequest(mock(SdkRequest.class))
272+
.executionContext(execCtx)
273+
.build();
274+
275+
SdkHttpFullResponse.Builder httpResponse = SdkHttpFullResponse.builder()
276+
.statusCode(502);
277+
278+
Response<SdkResponse> response = Response.<SdkResponse>builder()
279+
.httpResponse(httpResponse.build())
280+
.isSuccess(false)
281+
.exception(SdkException.builder().build())
282+
.build();
283+
284+
285+
CompletableFuture<Response<SdkResponse>> future = new CompletableFuture<>();
286+
future.completeExceptionally(new IOException("connection"));
287+
when(mockDelegatePipeline.execute(any(), any())).thenReturn(future);
288+
289+
CompletableFuture<Response<SdkResponse>> execute = retryableStage.execute(httpRequest, ctx);
290+
// exception thrown doesn't matter, just results in exception because we mock just enough...
291+
assertThatThrownBy(execute::join);
292+
293+
ArgumentCaptor<RefreshRetryTokenRequest> refreshRequestCaptor = ArgumentCaptor.forClass(RefreshRetryTokenRequest.class);
294+
295+
verify(mockRetryStrategy).refreshRetryToken(refreshRequestCaptor.capture());
296+
297+
RefreshRetryTokenRequest refreshRequest = refreshRequestCaptor.getValue();
298+
299+
assertThat(refreshRequest.suggestedDelay().get()).isEqualTo(Duration.ZERO);
300+
}
236301
}

0 commit comments

Comments
 (0)