-
Notifications
You must be signed in to change notification settings - Fork 846
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
feature(Waiters): Add the reason for failure in the exception thrown whenever a waiter transitions to a failure state. #5230
Conversation
…whenever a waiter transitions to a failure state
cb34e52
to
c5baa96
Compare
@@ -24,6 +25,10 @@ import software.amazon.awssdk.utils.ToString; | |||
@Generated("software.amazon.awssdk:codegen") | |||
@SdkInternalApi | |||
public final class WaitersRuntime { | |||
|
|||
public static final String FAILURE_MESSAGE_FORMAT_FOR_STATUS_MATCHER = "A waiter acceptor was matched to response status %d" |
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.
nit: private?
A waiter acceptor was matched on HTTP response status code (%s) ....
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.
Updated the message to similar as below suggestion An error waiter acceptor was matched...
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.
Looks like we are just missing parentheses around HTTP response status code (%d)
@@ -69,6 +70,11 @@ | |||
*/ | |||
public abstract class BaseWaiterClassSpec implements ClassSpec { | |||
|
|||
public static final String FAILURE_MESSAGE_FORMAT_FOR_PATH_MATCHER = "A waiter acceptor with the matcher %s matched the " |
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.
Suggesting the following:
An error waiter acceptor with the matcher (%s) was matched on parameter (%s=%s) and
transitioned the waiter to failure state.
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.
Was wondering why error waiter acceptor
and not just waiter acceptor
since the marcher is based on path and not a error matcher
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.
My thinking was that it's an error waiter acceptor because it leads the request to failure. I can see how it's confusing. NVM then!
public static final String FAILURE_MESSAGE_FORMAT_FOR_PATH_MATCHER = "A waiter acceptor with the matcher %s matched the " | ||
+ "expected value %s for the argument %s " | ||
+ "and transitioned the waiter to failure state"; | ||
public static final String FAILURE_MESSAGE_FORMAT_FOR_ERROR_MATCHER = "A waiter acceptor was matched to error condition " |
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.
Suggesting the following:
An error waiter acceptor was matched on error condition (%s) and ...
c5baa96
to
56d96da
Compare
Quality Gate failedFailed conditions |
…whenever a waiter transitions to a failure state. (aws#5230) * feature(Waiters): Add the reason for failure in the exception thrown whenever a waiter transitions to a failure state * Handle review comments
Motivation and Context
Currently the Waiter fails with below message
Issues like 2262 and also customer requests to add more info whenever a waiter fails. With multiple failure conditionals acceptors, its difficult for the user why exactly the Waiter failed.
This PR makes the exception message to have reason of failure in its message.
Modifications
When Waiter fails due to Response Field
Error Message
Failure message for failure due to Status code
Error Message
Failure message for failure due to Errors
Error Message
With Error code causing the failure
The exception is
Testing
Screenshots (if appropriate)
Types of changes
License