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

Retry access token refresh errors #15

Merged
merged 3 commits into from
Dec 8, 2017
Merged

Retry access token refresh errors #15

merged 3 commits into from
Dec 8, 2017

Conversation

krames
Copy link

@krames krames commented Dec 7, 2017

We are still seeing HTTP 401 exceptions containing the body Access token expired. Use your refresh token to obtain a new access token.

This PR detects these requests and then attempts to retry them 3 times using an incrmental back of of (count * 0.1 seconds). After the 3rd attempt is reached we just throw a BitBucket::Error::RefreshToken exception.

@krames krames requested a review from markphelps December 7, 2017 22:44
@@ -62,7 +62,7 @@ def get(user_name_or_project_key, repo_name, hook_uuid)
_validate_user_repo_params(user, repo) unless user? && repo?

get_request(
"/2.0/repositories/#{user_name}/#{repo_name}/hooks/#{hook_uuid}"
"/2.0/repositories/#{user_name_or_project_key}/#{repo_name}/hooks/#{hook_uuid}"
Copy link
Author

Choose a reason for hiding this comment

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

Fixes existing failing test.

yield
rescue BitBucket::Error::RefreshToken
count += 1
if count < 3

Choose a reason for hiding this comment

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

I think you want <= 3 here if you want it retry 3 times after the first failed attempt.

@@ -11,6 +13,9 @@ def on_complete(env)
when 400
raise BitBucket::Error::BadRequest.new(env)
when 401
if env[:body] =~ REFRESH_TOKEN

Choose a reason for hiding this comment

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

I guess there's no way to determine if its a 'retriable' 401 without using regex to parse the message?

Copy link
Author

Choose a reason for hiding this comment

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

Not that I can think of and unfortunately, I have not been able to reproduce this locally so I am sort of flying blind.

@krames krames merged commit c18d45b into master Dec 8, 2017
@joesiewert joesiewert deleted the access-token2 branch May 21, 2024 00:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants