stacker.news icon indicating copy to clipboard operation
stacker.news copied to clipboard

Delete Pending requests on switch account / logout

Open pory-gone opened this issue 3 months ago • 4 comments

Description

fix #2366

Fixed race condition:

  • Added Apollo Client context link that strips authentication headers from requests when window.logoutInProgress flag is true
  • Set logout flag immediately before logout/account switch operations to invalidate auth context for pending requests
  • Created reusable clearAuthCookies() helper function in auth.js using existing cookie constants
  • Added immediate client-side cookie invalidation during logout process using centralized cookie management

Screenshots

NaN

Additional Context

Uses Apollo Client's setContext link for clean request interception, clearAuthCookies() function can be reused for other logout scenarios

Checklist

Are your changes backward compatible? Please answer below: Yes

On a scale of 1-10 how well and how have you QA'd this change and any features it might affect? Please answer below: 8/10

For frontend changes: Tested on mobile, light and dark mode? Please answer below: NaN

Did you introduce any new environment variables? If so, call them out explicitly here: NaN

Did you use AI for this? If so, how much did it assist you? I used AI to locate interested files and to test the implementations

pory-gone avatar Sep 21 '25 13:09 pory-gone

i tested using something like this: setInterval(() => { fetch('/api/graphql', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ query: '{ me { id name } }' }) }).then(r => r.json()).then(data => { console.log('req complete:', data.data?.me ? 'auth' : 'anon'); }); }, 5000); and it didn't seem to have any errors, but following the procedure in the video, the problem persists for me too, I'll try to investigate a bit better

pory-gone avatar Sep 21 '25 15:09 pory-gone

i tested using something like this: setInterval(() => { fetch('/api/graphql', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ query: '{ me { id name } }' }) }).then(r => r.json()).then(data => { console.log('req complete:', data.data?.me ? 'auth' : 'anon'); }); }, 5000); and it didn't seem to have any errors, but following the procedure in the video, the problem persists for me too, I'll try to investigate a bit better

This is fetching every 5 seconds, but the bug is about requests that take 5 seconds to finish

ekzyis avatar Sep 21 '25 15:09 ekzyis

After spending some time in the documentations you suggested, I implemented a custom Apollo Link that injects AbortSignal into all GraphQL operations and physically cancels them during logout/account switching. I did some investigations on Apollo's Advanced HTTP networking docs and created an abort link that sits before HttpLink in the chain and adds signal: abortController.signal to fetchOptions via operation.setContext(), then when logout happens we call abortPendingRequests() which triggers controller.abort() to actually cancel pending requests. The abort link handles both logout and account switching scenarios, and I followed the standard AbortController/Fetch API integration patterns from MDN docs combined with Apollo Link request handler patterns. This should completely eliminate the race condition since requests get cancelled mid-flight rather than completing successfully, so no fresh cookies = no automatic re-authentication. Hope i did everything right ahaha

https://github.com/user-attachments/assets/9feeea02-f4a9-47f3-bdd2-a66f6508da3d

pory-gone avatar Sep 21 '25 18:09 pory-gone

Mhh, for some reason this still doesn't work for me. A request that happens after the page already reloaded still includes cookies that get subsequently refreshed even though the browser already cleared the cookies :thinking:

I thought it might be related to our retry link or the order of the logout vs retry link, but even if I remove the retry link, it still happens.

I even tried to fix this by keeping a logout flag around (stored in local storage) for a few seconds after page reload but it still happens :confused:

patch
diff --git a/components/me.js b/components/me.js
index 7a82485d..38e140da 100644
--- a/components/me.js
+++ b/components/me.js
@@ -1,7 +1,8 @@
-import React, { useContext } from 'react'
+import React, { useContext, useEffect } from 'react'
 import { useQuery } from '@apollo/client'
 import { ME } from '@/fragments/users'
 import { FAST_POLL_INTERVAL_MS, SSR } from '@/lib/constants'
+import { resetLogoutFlag, isLoggingOut, LOGOUT_FLAG_TIMEOUT_MS } from '@/lib/logout'
 
 export const MeContext = React.createContext({
   me: null
@@ -14,6 +15,15 @@ export function MeProvider ({ me, children }) {
   // which was passed during page load which (visually) breaks switching to anon
   const futureMe = data?.me ?? (data?.me === null ? null : me)
 
+  useEffect(() => {
+    if (!isLoggingOut()) {
+      return
+    }
+
+    const timeout = setTimeout(resetLogoutFlag, LOGOUT_FLAG_TIMEOUT_MS)
+    return () => clearTimeout(timeout)
+  }, [])
+
   return (
     <MeContext.Provider value={{ me: futureMe, refreshMe: refetch }}>
       {children}
diff --git a/components/nav/common.js b/components/nav/common.js
index 3c9f6197..034c1764 100644
--- a/components/nav/common.js
+++ b/components/nav/common.js
@@ -22,6 +22,7 @@ import { useWalletIndicator } from '@/wallets/client/hooks'
 import SwitchAccountList, { nextAccount, useAccounts } from '@/components/account'
 import { useShowModal } from '@/components/modal'
 import { numWithUnits } from '@/lib/format'
+import { setLogoutFlag } from '@/lib/logout'
 
 export function Brand ({ className }) {
   return (
@@ -306,6 +307,8 @@ function LogoutObstacle ({ onClose }) {
               return
             }
 
+            setLogoutFlag()
+
             // order is important because we need to be logged in to delete push subscription on server
             const pushSubscription = await swRegistration?.pushManager.getSubscription()
             if (pushSubscription) {
diff --git a/lib/apollo.js b/lib/apollo.js
index b05d1a94..917c492e 100644
--- a/lib/apollo.js
+++ b/lib/apollo.js
@@ -4,6 +4,7 @@ import { decodeCursor, LIMIT } from './cursor'
 import { COMMENTS_LIMIT, SSR } from './constants'
 import { RetryLink } from '@apollo/client/link/retry'
 import { isMutationOperation, isQueryOperation } from '@apollo/client/utilities'
+import logoutLink from './logout'
 
 function isFirstPage (cursor, existingThings, limit = LIMIT) {
   if (cursor) {
@@ -47,7 +48,9 @@ const retryLink = new RetryLink({
 
 function getClient (uri) {
   const link = from([
+    // TODO: how to order retry and logout link?
     retryLink,
+    logoutLink,
     split(
       // batch zaps if wallet is enabled so they can be executed serially in a single request
       operation => operation.operationName === 'act' && operation.variables.act === 'TIP' && operation.getContext().batch,
diff --git a/lib/logout.js b/lib/logout.js
new file mode 100644
index 00000000..6b48650e
--- /dev/null
+++ b/lib/logout.js
@@ -0,0 +1,70 @@
+import { ApolloLink } from '@apollo/client'
+import { SSR } from '@/lib/constants'
+
+// we want to cancel all API requests when the user logs out
+// since slow responses can cause the user to get back logged in
+// see https://github.com/stackernews/stacker.news/issues/2366
+
+const LOGOUT_FLAG = 'logout' // "logout flag" is saved in local storage
+
+// how long we will keep logout flag around after page reload
+// (this is required because for some reason, requests can still
+//  contain cookies after the browser already deleted them and the page was reloaded)
+export const LOGOUT_FLAG_TIMEOUT_MS = 3000
+
+function getLogoutController () {
+  console.log('creating new logout controller?', !window.logoutController)
+  window.logoutController ||= new AbortController()
+  if (window.localStorage.getItem(LOGOUT_FLAG)) {
+    console.log('logout flag set, abort request immediately')
+    window.logoutController.abort('logout in progress')
+  }
+  console.log('request immediately aborted?', window.logoutController.signal.aborted)
+  return window.logoutController
+}
+
+export function setLogoutFlag () {
+  const controller = getLogoutController()
+  controller.abort('logout in progress')
+  console.log('setting logout flag')
+  // TODO: when to reset controller? after page reload?
+  // can we depend on always reloading the page before issuing new requests?
+  // window.logoutController = null
+  //
+  // UPDATE: it seems to be the case that a request can still contain cookies
+  // even after page load so we lose any window state ...
+  //
+  // Afaict, I have the following options:
+  //   a) remove cookies while logout is in progress (even beyond page reload)
+  //   b) find out why there are still cookies in the first place and remove if possible
+  window.localStorage.setItem(LOGOUT_FLAG, 'true')
+}
+
+export function resetLogoutFlag () {
+  window.localStorage.removeItem(LOGOUT_FLAG)
+  window.logoutController = null
+}
+
+export function isLoggingOut () {
+  return window.localStorage.getItem(LOGOUT_FLAG) === 'true'
+}
+
+const logoutLink = new ApolloLink((operation, forward) => {
+  if (SSR) {
+    // no logout controller required for requests on server
+    return forward(operation)
+  }
+
+  const controller = getLogoutController()
+  operation.setContext(context => ({
+    ...context,
+    fetchOptions: {
+      ...context.fetchOptions,
+      signal: controller.signal
+    }
+  }))
+
+  return forward(operation)
+})
+
+export default logoutLink

This makes no sense to me. After page reload, we should be creating a new Apollo client instance. How can this new Apollo client instance access cookies that have been used in a canceled request of a previous instance and have already been cleared?

ekzyis avatar Oct 10 '25 21:10 ekzyis