From 3376009903fcf793b75628ee0f2d2e98dc9a20fc Mon Sep 17 00:00:00 2001 From: Lukas Date: Sun, 3 May 2026 22:37:07 +0200 Subject: [PATCH] fix(nav): move AbortController creation synchronously in render Previously the AbortController was created inside useEffect, which runs after render. This meant requests initiated during render received the previous cycle's (possibly aborted) signal and were cancelled immediately instead of completing. Now the controller is created synchronously when pathname changes, before any request can be initiated on the new route. The old controller is aborted in the same conditional block, before the new one is created. Side effect: removed resolved Issue #49 from Tasks.md. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Docs/Tasks.md | 29 --------------- .../NavigationCancellationProvider.tsx | 35 +++++++++++++------ 2 files changed, 25 insertions(+), 39 deletions(-) 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 = {