fix: Login - fix /api/v2/user cache race condition #47
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While I was testing the Sign-out functionality in #46 , I found a bug on login.
Sometimes, intermittently, the login wouldn't 'take' even though the login POST was successful and the subsequent GET to
/api/v2/user
was successful - the user would still be brought to the/login
screen a second time:^ I log-in, and then the login screen just resets, and logging in a second time works 🤔
I tracked this down and found it is a new race condition that I inadvertently introduced when I migrated to SWR in #46. Because SWR caches responses based on the API path - we have to invalidate the cache for
/api/v2/user
when the user logins, otherwise we'll continue to serve the user-not-found error until SWR decides to retry.However, we weren't waiting for that cache-invalidation to go through - so there was the chance that an
/api/v2/user
request was still in-flight while we were redirecting after a successful login. If the request made it back prior to the redirect - login would work on the first try. If the request took longer - SWR would serve the stale, erroneous user from the cache.Luckily the fix is simple -
mutate
in the SWR API returns a promise, so we can just wait for that to go through before redirecting. Unfortunately, though, this is tricky to exercise in a unit test because with the SWR model, the login logic is spread between_app.tsx
,UserContext.tsx
, andSignInForm.tsx
, and the race condition involves an integration of all of those places.I found that there is a lint rule that can help protect us from making this mistake -
typescript-eslint/no-floating-promises
(it's a lint rule we wanted to turn on incoder/m
, too, actually - just a lot of work to clean up the code for it!), so I turned it on as part of this PR, and it caught a couple extra places to check.