Treat UncheckedIOException as IOExceptions

aws / aws-sdk-java-v2

The official AWS SDK for Java - Version 2

Apache License 2.0

2.16k stars 840 forks source link

Treat UncheckedIOException as IOExceptions #5319

Closed anirudh9391 closed 2 months ago

anirudh9391 commented 3 months ago

Motivation and Context

Timeout triggered termination of the connection stream, causes SDK to not perform retries even if configured.

Modifications

Users of DynamoDB experienced and reported that API timeouts causing UncheckedIOException do not get retried, despite the client being configured to retry timeout exceptions.

Testing

Added unit test

Screenshots (if appropriate)

Types of changes

[ x ] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)

joviegas commented 3 months ago

Could you please mention some details like which service was facing this issue and how this issue surfaced ? In this particular case who exactly was throwing UncheckedIOException ? Also please add changelogs

joviegas commented 3 months ago

Something to also consider while fixing is instance like https://github.com/aws/aws-sdk-java-v2/blob/9fec27c2d2f04167ae7c8cf91a958f8f3aa4bde4/core/sdk-core/src/main/java/software/amazon/awssdk/core/BytesWrapper.java#L91 where UnCheckedIOException are exception which do not pass on retries .

anirudh9391 commented 3 months ago

aws-sdk-java-v2/core/sdk-core/src/main/java/software/amazon/awssdk/core/BytesWrapper.java

Yes, as discussed offline yesterday, we need to cut a broader task to identify and handle UncheckedIOExceptions throughout the SDK codebase. Now that we know it most certainly has to be retried. This SIM addresses the imminent task, which is timeout caused UncheckedIOExceptions.

sonarcloud[bot] commented 3 months ago

Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
92.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

joviegas commented 2 months ago

Can we please update the title and change log to reflect that these changes modify the handling of exceptions when closing the response stream during an SDKInterrupted exception, changing it to just log the cause of failure.
If possible, can we write a functional test or at least perform a one-off test to ensure this addresses the issue.
Can we please also update the fix for ApiCallAttemptTimeoutTrackingStage since the reported stack trace showed this class.

zoewangg commented 2 months ago

Closing in favor of https://github.com/aws/aws-sdk-java-v2/pull/5439