-
Notifications
You must be signed in to change notification settings - Fork 314
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
fix(clerk-js): Reduce retries for /tokens
on 5xx errors
#5106
Conversation
🦋 Changeset detectedLatest commit: fea2b19 The changes in this PR will be included in the next version bump. This PR includes changesets to release 19 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
/tokens
on 500 errors/tokens
on 5xx errors
@@ -85,7 +85,8 @@ export class Session extends BaseResource implements SessionResource { | |||
|
|||
getToken: GetToken = async (options?: GetTokenOptions): Promise<string | null> => { | |||
return runWithExponentialBackOff(() => this._getToken(options), { | |||
shouldRetry: (error: unknown, currentIteration: number) => !is4xxError(error) && currentIteration < 4, | |||
shouldRetry: (error: unknown, currentIteration: number) => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really not want to retry on 5xx at all? Traditionally retrying on 5xx is common practice in the case of intermittent errors, but I do see the risk with potentially making the underlying problem worse.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see the case of FAPI going down, and the first call to /tokens
fail with a 5xx, but the subsequential ones might succeed once the server normalizes
Maybe instead of not retrying at all, we can throttle with a larger delay?
Closing this in favor of another solution we are already working on! |
Description
When requesting a token from FAPI on
/tokens
, we retry 4 times if the response is NOT a 4xx error.We want to also stop retrying on 5xx errors.
Checklist
pnpm test
runs as expected.pnpm build
runs as expected.Type of change