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

DXCDT-520: Use a custom client with retries for auth0 api client config #788

Merged
merged 7 commits into from
Aug 25, 2023

Conversation

sergiught
Copy link
Contributor

@sergiught sergiught commented Aug 24, 2023

🔧 Changes

go-auth0@v1 introduced a new way of expressing retry logic when instantiating the management client. This change is a deviation from v0 which implemented unlimited retries by default. Even with three retries, users are experiencing issues #273 (comment) due to the requests running in parallel and multiple failing at the same time, quickly hitting the 3 retry count.

In this PR we skip using the retry logic from the Go SDK and introduce a custom client that will:

  • Retry an unlimited amount of times when hitting a rate limit error, after waiting the amount of time specified in the X-RateLimit-Reset header.
  • Retry up to 3 times transient errors with exponential backoff and jitter.
  • Retry up to 3 times on 500s with exponential backoff and jitter.

📚 References

🔬 Testing

📝 Checklist

  • All new/changed/fixed functionality is covered by tests (or N/A)
  • I have added documentation for all new/changed functionality (or N/A)

@sergiught sergiught self-assigned this Aug 24, 2023
@codecov-commenter
Copy link

codecov-commenter commented Aug 24, 2023

Codecov Report

Merging #788 (724f4e2) into patch/fix-cert-in-test (c0c49ef) will increase coverage by 0.02%.
The diff coverage is 95.23%.

Additional details and impacted files

Impacted file tree graph

@@                    Coverage Diff                     @@
##           patch/fix-cert-in-test     #788      +/-   ##
==========================================================
+ Coverage                   90.41%   90.43%   +0.02%     
==========================================================
  Files                          99       99              
  Lines                       13122    13184      +62     
==========================================================
+ Hits                        11864    11923      +59     
- Misses                        891      893       +2     
- Partials                      367      368       +1     
Files Changed Coverage Δ
internal/config/config.go 97.50% <95.23%> (-2.50%) ⬇️

@sergiught sergiught changed the base branch from v1 to bump-go-1.21 August 24, 2023 07:29
@sergiught sergiught force-pushed the fix/retry-logic branch 2 times, most recently from ec507a1 to 9b18a1d Compare August 24, 2023 14:35
@sergiught sergiught marked this pull request as ready for review August 24, 2023 15:07
@sergiught sergiught requested a review from a team as a code owner August 24, 2023 15:07
Base automatically changed from bump-go-1.21 to v1 August 25, 2023 06:50
@sergiught sergiught changed the base branch from v1 to patch/fix-cert-in-test August 25, 2023 09:07
Base automatically changed from patch/fix-cert-in-test to v1 August 25, 2023 09:20
// Cloudflare-specific server error that is generated
// because Cloudflare did not receive an HTTP response
// from the origin server after an HTTP Connection was made.
524,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sergiught sergiught requested a review from willvedd August 25, 2023 12:46
Copy link
Contributor

@willvedd willvedd left a comment

Choose a reason for hiding this comment

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

Great 👍

@sergiught sergiught merged commit cf36419 into v1 Aug 25, 2023
@sergiught sergiught deleted the fix/retry-logic branch August 25, 2023 13:14
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.

3 participants