Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Provide caller with information on raised exception after retry #926

Closed
jantman opened this issue May 21, 2016 · 4 comments
Closed

Provide caller with information on raised exception after retry #926

jantman opened this issue May 21, 2016 · 4 comments
Labels
feature-request This issue requests a feature.

Comments

@jantman
Copy link
Contributor

jantman commented May 21, 2016

As far as I can tell, when retryhandler.MaxAttemptsDecorator reaches its maximum number of attempts, the retryable exception is returned to the caller. I just spent about 3 hours tracing through boto3 and botocore and writing test code to reproduce a RequestLimitExceeded error, only to find that botocore was properly retrying and the only thing I can think of is that the request limit was really exceeded (even after 5 exponential backoff retries).

It would be very useful in cases like this if the retry handler either (a) wrapped the actual exception in a (e.g.) MaxRetriesExceeded exception, to make it clear that the request was retried and perhaps include the number of retries and maybe the sleep duration, or (b) at least fire an event with this information.

While I do really appreciate that the retry logic is so transparent, this also makes it a bit opaque for debugging, as there's really no way I can find to confirm (especially programmatically in the caller) that the request was, in fact, retried and all retries failed.

Thanks,
Jason Antman

(this came up when investigating this issue.)
@JordonPhillips JordonPhillips added the feature-request This issue requests a feature. label May 26, 2016
@jamesls
Copy link
Member

jamesls commented May 26, 2016

Thanks for the feedback. The only thing we'll need to keep in mind is we'll need to do this in a way that's backwards compatible. Otherwise, I think being able to communicate this information somehow would be useful.

@JordonPhillips
Copy link
Contributor

@jamesls I was just thinking about this, could we just add it to the ResponseMetadata?

@jamesls
Copy link
Member

jamesls commented May 27, 2016

@JordonPhillips I think that would work.

@jantman jantman mentioned this issue Jun 23, 2016
Closed
jantman added a commit to jantman/botocore that referenced this issue Jun 23, 2016
@jantman
fixes boto#926 - expose retry information in exceptions
f639a54 
* in botocore.retryhandler.MaxAttemptsDecorator.__call__(), add a MaxAttemptsReached=True element to ResponseMetadata if the maximum number of attempts is reached
* in botocore.endpoint.Endpoint._send_request(), add a NumAttempts element to ResponseMetadata, containing the attempt number
@jantman jantman mentioned this issue Jun 23, 2016
Closed
@jantman
Copy link
Contributor Author

jantman commented Jun 23, 2016

@jamesls @JordonPhillips I've got a simple implementation of this as #965 - coverage stayed the same, though it appears that most of Endpoint isn't covered. I don't really have any experience with unittest, so I didn't attempt to rectify that.

The implementation may be a bit naive, so I'm certainly open to changing/reworking it as needed.

jantman added a commit to jantman/botocore that referenced this issue Jun 24, 2016
@jantman
issue boto#926 - check that response is not None instead of checking …
4ae8bfd 
…that it's a tuple, per PR comment
jantman added a commit to jantman/botocore that referenced this issue Jun 24, 2016
@jantman
issue boto#926 - instead of passing number of attempts back in Respon…
61be4cd 
…seMetadata, pass back max_attempts and a list of retry error reasons, per PR comment
jantman added a commit to jantman/botocore that referenced this issue Jun 24, 2016
@jantman
issue boto#926 - clean up code a bit
4d5828b
jantman added a commit to jantman/botocore that referenced this issue Aug 15, 2016
@jantman
fixes boto#926 - expose retry information in exceptions
b44d22b
@jamesls jamesls closed this as completed in ed311bd Sep 1, 2016
jamesls added a commit that referenced this issue Sep 1, 2016
@jamesls
Merge branch 'jantman-issue926_retry_info' into develop
c3babf3 
* jantman-issue926_retry_info:
  Update error message when max retries reached
  Add changelog entry for response metadata change
  Add retry info to ClientError text
  Replace list of retries with number of retries
  fixes #926 - expose retry information in exceptions
awstools pushed a commit that referenced this issue Sep 1, 2016
Merge branch 'release-1.4.51'
79f1f4e 
* release-1.4.51:
  Bumping version to 1.4.51
  Update to latest models
  Update error message when max retries reached
  Add changelog entry for last_response feature
  Add changelog entry for response metadata change
  Add retry info to ClientError text
  Replace list of retries with number of retries
  fixes #926 - expose retry information in exceptions
  Add last_response attr to WaiterError
  Waiter throw ClientError for unexpected error response
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature-request This issue requests a feature.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@jantman @jamesls @JordonPhillips