Skip to content

fix(auth): prevent infinite fetch recursion and multiple wrapper layers#385

Open
paulirish wants to merge 2 commits intomainfrom
auth-infinite
Open

fix(auth): prevent infinite fetch recursion and multiple wrapper layers#385
paulirish wants to merge 2 commits intomainfrom
auth-infinite

Conversation

@paulirish
Copy link
Collaborator

This PR fixes the issue where the frontend would hammer the auth endpoints when the user was logged out or a session expired. It actually spammed my terminal so hard (probably several hours) that iTerm2 crashed. First time i've seen that!

Turns out there were two main bugs in the useAuth hook:

  • Infinite Recursion: The window.fetch wrapper was intercepting 401s to trigger a refresh, but the refresh request itself was being intercepted by the same wrapper. If the refresh token was expired (returning a 401), it would just loop infinitely.
  • Wrapper Stacking: Since useAuth is used in ~45 different components, we were potentially stacking dozens of nested wrappers on top of window.fetch.

obv this is hacks on top of hacks. Presumably there's another fix that cleans all this up a bit. 🤷

This fixes an issue where the frontend would spam the auth endpoints repeatedly when logged out or when a session expired.

1. Infinite Recursion on 401: The window.fetch wrapper would catch a 401, call tryRefresh(), which then called fetch() again, triggering the wrapper recursively if the refresh also failed. We now use the original fetch for refresh attempts and exclude auth endpoints from auto-refresh logic.
2. Multiple Wrapper Layers: Since useAuth is a hook used by many components, multiple instances were independently wrapping window.fetch. We now store the original fetch globally and ensure wrapping only happens once.
@paulirish
Copy link
Collaborator Author

obv this is hacks on top of hacks. Presumably there's another fix that cleans all this up a bit. 🤷

I ran a simplifier agent and got this:

Auth nor React is not my thing so I can't really judge how worth it this is. Sorry to dump it on you.

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.

1 participant