-
Notifications
You must be signed in to change notification settings - Fork 885
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create EC2MetadataClientException for IMDS 4XX errors #5947
Create EC2MetadataClientException for IMDS 4XX errors #5947
Conversation
core/imds/src/main/java/software/amazon/awssdk/imds/internal/AsyncHttpRequestHelper.java
Outdated
Show resolved
Hide resolved
...sdk-core/src/main/java/software/amazon/awssdk/core/exception/Ec2MetadataClientException.java
Outdated
Show resolved
Hide resolved
core/imds/src/main/java/software/amazon/awssdk/imds/internal/DefaultEc2MetadataClient.java
Outdated
Show resolved
Hide resolved
core/imds/src/main/java/software/amazon/awssdk/imds/internal/DefaultEc2MetadataClient.java
Outdated
Show resolved
Hide resolved
core/imds/src/test/java/software/amazon/awssdk/imds/internal/Ec2MetadataAsyncClientTest.java
Outdated
Show resolved
Hide resolved
Can we add some documentation here to mention what exceptions it could throw? https://github.com/aws/aws-sdk-java-v2/blob/master/core/imds/src/main/java/software/amazon/awssdk/imds/Ec2MetadataClient.java#L70 |
…iption and some code refactoring
core/imds/src/main/java/software/amazon/awssdk/imds/internal/AsyncHttpRequestHelper.java
Show resolved
Hide resolved
core/imds/src/main/java/software/amazon/awssdk/imds/internal/DefaultEc2MetadataClient.java
Outdated
Show resolved
Hide resolved
core/imds/src/main/java/software/amazon/awssdk/imds/internal/DefaultEc2MetadataClient.java
Outdated
Show resolved
Hide resolved
core/imds/src/main/java/software/amazon/awssdk/imds/internal/AsyncHttpRequestHelper.java
Outdated
Show resolved
Hide resolved
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change LGTM. Good job!
Let's have a mini APi surface area/support review with the team before merging
3e0fd77
into
feature/master/imdsException
Motivation and Context
The IMDS client currently throws SdkClientException for all 4XX errors, contradicting AWS SDK guidelines which state that client exceptions should only be used when the client fails to make a service call. When IMDS returns a response (even 4XX), it means the call was successful but rejected by the service. This improper error classification prevents specific error handling. Users can't implement specific handling for such cases because the client discards the HTTP status code information, forcing all errors to be treated the same way.
Fixing #5786
Modifications
Testing
Added unit tests to verify the new Ec2MetadataClientException. Tests include mocked IMDS responses with different status codes (4XX), verifying status code preservation, error message formatting, and proper exception handling in both sync and async clients.
Screenshots (if appropriate)
Types of changes
Checklist
mvn install
succeedsscripts/new-change
script and following the instructions. Commit the new file created by the script in.changes/next-release
with your changes.License