diff --git a/Docs/Tasks.md b/Docs/Tasks.md index c50d7fb..7567043 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -1,32 +1,3 @@ -### Issue #49: HIGH - Dual Auth Error Handlers With No Deduplication Guard - -**Where found**: -- `frontend/src/providers/AuthProvider.tsx:50-56` -- `frontend/src/api/client.ts:143` – `setUnauthorizedHandler` -- `frontend/src/utils/fetchError.ts` – `setAuthErrorHandler` - -**Why this is needed**: -Both handlers fire independently on a 401. Without a deduplication guard, logout logic executes twice, causing a React state mutation race and potentially double-navigating to the login page. - -**Goal**: -Ensure auth error handling fires exactly once per 401 response. - -**What to do**: -1. Introduce a module-level `isLoggingOut` flag (or use a ref) that is set before the first logout dispatch and checked before the second. -2. Alternatively, consolidate into a single handler and remove one registration. - -**Possible traps and issues**: -- The "defense-in-depth" rationale for two handlers may be intentional; confirm with team before removing one. -- The flag must be reset after the login page is reached. - -**Docs changes needed**: -- Add comment in `AuthProvider.tsx` explaining the deduplication guard. - -**Doc references**: -- `frontend/src/providers/AuthProvider.tsx` inline comment about cross-tab logout - ---- - ### Issue #50: HIGH - Navigation Abort Signal Timing Bug **Where found**: diff --git a/frontend/src/providers/NavigationCancellationProvider.tsx b/frontend/src/providers/NavigationCancellationProvider.tsx index 2ea3053..dfe43a0 100644 --- a/frontend/src/providers/NavigationCancellationProvider.tsx +++ b/frontend/src/providers/NavigationCancellationProvider.tsx @@ -41,6 +41,12 @@ interface NavigationCancellationProviderProps { * * Detects route changes via useLocation and automatically aborts * all AbortSignals obtained during the previous route. + * + * Timing contract: + * - AbortController is created synchronously in render when pathname changes, + * before any request can be initiated on the new route. + * - The old controller is aborted inside the effect (after render). + * - Strict Mode double-invocation: abort is idempotent, so double-abort is safe. */ export function NavigationCancellationProvider( props: NavigationCancellationProviderProps, @@ -48,22 +54,31 @@ export function NavigationCancellationProvider( const { children } = props; const location = useLocation(); - // Current active AbortController for this route - const controllerRef = useRef(new AbortController()); + // Current active AbortController for this route. + // Created synchronously during render when pathname changes, + // so the fresh signal is available before any request is initiated. + const controllerRef = useRef(null); const prevPathnameRef = useRef(location.pathname); + // Create new controller synchronously when pathname changes. + // This ensures getNavigationSignal() returns a non-aborted signal + // for requests initiated during the same render pass. + // Abort old controller BEFORE creating new one to prevent + // any request on the new route from using the old (aborted) signal. + if (controllerRef.current === null || location.pathname !== prevPathnameRef.current) { + controllerRef.current?.abort(); + controllerRef.current = new AbortController(); + } + const getNavigationSignal = useCallback((): AbortSignal => { - return controllerRef.current.signal; + return controllerRef.current!.signal; }, []); - // When route pathname changes (not on mount), abort the old controller and create a new one + // Update prevPathnameRef after synchronous controller creation. + // This runs after render so we don't abort on Strict Mode remounts + // (where pathname is unchanged between renders). useEffect(() => { - const currentPathname = location.pathname; - if (currentPathname !== prevPathnameRef.current) { - prevPathnameRef.current = currentPathname; - controllerRef.current?.abort(); - controllerRef.current = new AbortController(); - } + prevPathnameRef.current = location.pathname; }, [location.pathname]); const contextValue: NavigationCancellationContextType = {