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.

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

An initial implementation of client retries. #522

Merged
merged 2 commits into from
Apr 2, 2024
Merged

Conversation

ChuckCrawford
Copy link
Collaborator

@ChuckCrawford ChuckCrawford commented Mar 20, 2024

Overview

Adds the ability to configure the client to retry certain types of failures.

Changes

When configured for retries:

  • The client will retry any request that fails with a network error, 5xx, or 429 status code.
  • The client will back off before retrying a request:
    • Server-provided indicators of how long to wait will be used when available.
    • Otherwise we use a truncated binary exponential backoff with jitter.

Other Notes

Note that this functionality is not supported in the following API functions at this time:

  • pagerduty.CreateEvent
  • pagerduty.CreateEventWithHTTPClient
  • client.Do

This likely be optional functionality for the current 1.9 release. Please give us your feedback.

@ChuckCrawford ChuckCrawford changed the title [DEVI]An initial implementation of client retries. [DEVI] An initial implementation of client retries. Mar 20, 2024
@ChuckCrawford ChuckCrawford marked this pull request as ready for review March 20, 2024 18:17
@ChuckCrawford ChuckCrawford added this to the v1.9.0 milestone Mar 20, 2024
@ChuckCrawford ChuckCrawford changed the title [DEVI] An initial implementation of client retries. An initial implementation of client retries. Mar 20, 2024
client.go Show resolved Hide resolved
return nil, respErr
}

if resp.StatusCode < 200 || resp.StatusCode > 299 {
Copy link

@tomhart-msc tomhart-msc Mar 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to return 3xx as an error?

Edit: I see that's the existing behaviour. Question is not a blocker.

Copy link

@tomhart-msc tomhart-msc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, minor comments.

Copy link
Contributor

@imjaroiswebdev imjaroiswebdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍🏽

@ChuckCrawford ChuckCrawford merged commit de82e6b into main Apr 2, 2024
2 checks passed
@ChuckCrawford ChuckCrawford deleted the fb/retry-support branch April 2, 2024 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants