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

Add retry on get user when rate limited #17

Merged
merged 3 commits into from
Oct 3, 2019

Conversation

michal-franc
Copy link
Contributor

@michal-franc michal-franc commented Sep 30, 2019

Problem - /users/{id} rate limited by auth0 on get auth0 user

could not find auth0 user: bad status code (429):
{
"statusCode":429,"error":"Too Many Requests",
"message":"Global limit has been reached",
"errorCode":"too_many_requests"
}

This is coming from
func (authClient *AuthClient) GetUserById(id string) (*User, error) {
https://github.com/form3tech-oss/terraform-provider-auth0/blob/master/auth0/auth0_client.go#L116

Auth0 rate limits for - /users/{id}
https://auth0.com/docs/policies/rate-limits#management-api-v2

plan limit /s burst /s
free 2 10
paid 15 50

Solution - retry with timed back off

Provider uses - https://github.com/parnurzeal/gorequest
This package supports - retries - https://github.com/parnurzeal/gorequest#retry


Changes

Retry on Get* calls when rate limited by Auth0

This is done by leveraging https://github.com/parnurzeal/gorequest retry functionality. We have decided to keep it simple and not add exponential back-off for now.

New optional provider parameters

auth0_request_max_retry_count - max retry on requests to Auth0 - default: 2
auth0_time_between_retries - time to wait between retried requests to Auth0 (in milliseconds) - default: 1000

Added acceptance test to verify throttling on GetUserById

This test is not perfect and fully reliable as it depends on burst generated by machine running it. To get the correct test we need to hit 50 requests per seconds on paid plan. It was still useful during local development to verify if retry helps with throttling. We can disable this test to run from the main test suite.

Refactored Auth0 client creation

Moved token generation to within the Auth0 client as it is used now in the new acceptance test.

@michal-franc michal-franc force-pushed the michalfranc-retry-on-429-auth0-get-user branch from b017460 to 690ec1e Compare September 30, 2019 14:27
@michal-franc michal-franc force-pushed the michalfranc-retry-on-429-auth0-get-user branch from 690ec1e to 0475962 Compare October 2, 2019 14:24
@michal-franc michal-franc changed the title [WIP] Add retry on get user Add retry on get user when rate limited Oct 2, 2019
Copy link
Contributor

@devenney devenney left a comment

Choose a reason for hiding this comment

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

Looks like a really good change. Few concerns around usability for users who cannot hack the code so easily to investigate/fix subtle issues.

auth0/auth0_client.go Outdated Show resolved Hide resolved
auth0/auth0_client.go Show resolved Hide resolved
This will enable us to see more descrtiptive errors from Auth0
Copy link
Contributor

@devenney devenney left a comment

Choose a reason for hiding this comment

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

LGTM

@michal-franc michal-franc merged commit 36b4ee0 into master Oct 3, 2019
@michal-franc michal-franc deleted the michalfranc-retry-on-429-auth0-get-user branch October 3, 2019 10:07
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