Skip to content
This repository was archived by the owner on Jan 23, 2026. It is now read-only.

Fix retry token refresh upon network failure and retry refresh upon 5xx errors - #379

Merged
alaister merged 1 commit into
supabase:nextfrom
pixtron:fix/issue-333
Aug 15, 2022
Merged

Fix retry token refresh upon network failure and retry refresh upon 5xx errors#379
alaister merged 1 commit into
supabase:nextfrom
pixtron:fix/issue-333

Conversation

@pixtron

@pixtron pixtron commented Aug 13, 2022

Copy link
Copy Markdown
Contributor

What kind of change does this PR introduce?

Bug fix

What is the current behavior?

  1. Since feature): [gotrue-js] Refactor return and error types #301 the error NETWORK_FAILURE.ERROR_MESSAGE is not thrown anymore, without this message failed refresh request are not retryed anymore:
    https://github.com/supabase/gotrue-js/blob/67a6575102be9c50763ff5d06fd2ee44b18a69f3/src/GoTrueClient.ts#L870-L877
  2. When the server/cloudflare proxy sends a body with no or invalid json, the error handling throws an exception (see Error messages not being parsed correctly #289) and does not reject the promise.
  3. 5xx server responses could be retryed as well (eg. cloudflares 524: A timeout occurred)

Resolves: #333
Related to: #289

What is the new behavior?

This PR introduces a new error AuthRetryableFetchError Exponential backoff will now retry if this error is returned.
handleError will reject with AuthRetryableFetchError when

Additionaly when response.json() throws an error (eg. invalid json) the error is caught and the promise is rejected with an AuthUnknownError.

In order to test the error handling this PR adds a new dev dependency jest-mock-server, which let's us setup a mock server that responds with errors or aborts a request without sending any response.

@kangmingtay kangmingtay left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

good catch @pixtron, this is very nice!

@pixtron

pixtron commented Aug 13, 2022

Copy link
Copy Markdown
Contributor Author

just recognized i mentioned the wrong PR in the description. It's #301 that introduced the regression. Edited the description as well.

@alaister alaister left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you @pixtron!

@alaister
alaister merged commit 18dcc36 into supabase:next Aug 15, 2022
@github-actions

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 1.23.0-next.17 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 2.0.0-rc.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants