Переглянути джерело

fix(toast): guard setToasts against post-unmount async callbacks

  An async handler (e.g. LoginPage's catch-handler on a failed login
  request) can call showToast AFTER Vitest's afterEach has unmounted the
  ToastProvider. The existing unmount effect clears timers it knows about,
  but a showToast scheduled POST-unmount lands a fresh 3s setTimeout that
  the cleanup never saw. The callback then runs against a torn-down jsdom
  — setToasts → React scheduler → accesses window → ReferenceError as an
  uncaught exception in test output ("Vitest caught 1 unhandled error").

  Add an isMountedRef flipped to false in the unmount cleanup; every
  setToasts call (showToast, showPersistentToast, dismissToast, both
  dispatch-toast auto-dismiss timers) short-circuits when the provider is
  gone. The timer callback re-checks the ref as a belt-and-braces guard
  for timers scheduled after cleanup already ran.

  In production this only hardens the code; the provider lives at app
  root and does not unmount during normal navigation.

  Tests: 4 new regression cases in __tests__/contexts/ToastContext.test.tsx
  covering post-unmount showToast / showPersistentToast / dismissToast
  no-oping, and the auto-dismiss timer firing post-unmount without
  throwing. Full suite: 1381 pass, 0 unhandled errors (was 1377 + 1
  uncaught exception). TypeScript check clean.
maziggy 1 місяць тому
батько
коміт
58d33cdb9c

+ 96 - 0
frontend/src/__tests__/contexts/ToastContext.test.tsx

@@ -0,0 +1,96 @@
+/**
+ * Tests for ToastContext's post-unmount safety guards.
+ *
+ * Regression: a login response handler calling showToast AFTER the provider
+ * had already been unmounted by Vitest's afterEach scheduled a 3s setTimeout
+ * that fired during test teardown. The callback's setToasts then tried to
+ * schedule a React update against a torn-down jsdom, producing
+ * "window is not defined" as an uncaught exception.
+ *
+ * The provider now gates every setToasts call on an isMountedRef and
+ * re-checks inside the auto-dismiss setTimeout callback so stale async
+ * paths no-op instead of crashing.
+ */
+
+import { describe, it, expect, beforeEach, vi } from 'vitest';
+import { act, renderHook } from '@testing-library/react';
+import { type ReactNode } from 'react';
+import { ToastProvider, useToast } from '../../contexts/ToastContext';
+
+function Wrapper({ children }: { children: ReactNode }) {
+  return <ToastProvider>{children}</ToastProvider>;
+}
+
+describe('ToastContext post-unmount safety', () => {
+  beforeEach(() => {
+    vi.useRealTimers();
+  });
+
+  it('does not crash when showToast is called after unmount', () => {
+    const { result, unmount } = renderHook(() => useToast(), { wrapper: Wrapper });
+
+    // Capture the callbacks BEFORE unmount — a real stale-closure scenario.
+    // (Async handlers that kicked off before unmount keep their captured
+    // context value and will invoke this function after we tear down.)
+    const { showToast } = result.current;
+
+    unmount();
+
+    // Post-unmount invocation is now a no-op; must not throw.
+    expect(() => showToast('delayed error message', 'error')).not.toThrow();
+  });
+
+  it('does not invoke setToasts when the auto-dismiss timer fires after unmount', async () => {
+    vi.useFakeTimers();
+
+    const { result, unmount } = renderHook(() => useToast(), { wrapper: Wrapper });
+
+    act(() => {
+      result.current.showToast('will outlive the provider', 'error');
+    });
+
+    // Unmount BEFORE the 3s timer fires — the unmount effect clears pending
+    // timers, but a belt-and-braces check inside the timer callback (for
+    // cases where the timer was scheduled post-unmount) must also hold.
+    unmount();
+
+    // Advance past the 3s auto-dismiss window. If the guard isn't in place
+    // this would throw "window is not defined" in a torn-down jsdom; we
+    // simulate by asserting no error propagates.
+    expect(() => {
+      vi.advanceTimersByTime(5000);
+    }).not.toThrow();
+
+    vi.useRealTimers();
+  });
+
+  it('post-unmount showPersistentToast and dismissToast are no-ops', () => {
+    const { result, unmount } = renderHook(() => useToast(), { wrapper: Wrapper });
+    const { showPersistentToast, dismissToast } = result.current;
+    unmount();
+
+    // Both must short-circuit rather than attempt setState on a dead tree.
+    expect(() => showPersistentToast('orphan', 'still here', 'info')).not.toThrow();
+    expect(() => dismissToast('orphan')).not.toThrow();
+  });
+
+  it('normal showToast flow still displays and auto-dismisses while mounted', () => {
+    vi.useFakeTimers();
+    const { result } = renderHook(() => useToast(), { wrapper: Wrapper });
+
+    act(() => {
+      result.current.showToast('mounted path works', 'success');
+    });
+
+    // No easy way to read toast DOM from the hook alone; assert the timer
+    // ran without throwing — that proves the isMountedRef guard didn't
+    // incorrectly short-circuit the mounted path.
+    expect(() => {
+      act(() => {
+        vi.advanceTimersByTime(3500);
+      });
+    }).not.toThrow();
+
+    vi.useRealTimers();
+  });
+});

+ 15 - 0
frontend/src/contexts/ToastContext.tsx

@@ -78,22 +78,34 @@ export function ToastProvider({ children }: { children: ReactNode }) {
   const timeoutRefs = useRef<Map<string, ReturnType<typeof setTimeout>>>(new Map());
   const dispatchToastId = 'background-dispatch';
   const lastDispatchSummaryRef = useRef<string | null>(null);
+  // Tracks whether the provider is still mounted. A toast can be triggered by
+  // an async callback that resolves AFTER React has unmounted us (common in
+  // tests: `cleanup()` runs while a login promise is still in flight, then
+  // the error handler calls showToast). In that case, scheduling a setTimeout
+  // that later calls setToasts produces "window is not defined" once the jsdom
+  // environment is torn down. Guard every setToasts call behind this ref so a
+  // post-unmount showToast is a no-op instead of crashing.
+  const isMountedRef = useRef(true);
 
   // Clean up all timeouts on unmount
   useEffect(() => {
+    isMountedRef.current = true;
     const timeouts = timeoutRefs.current;
     return () => {
+      isMountedRef.current = false;
       timeouts.forEach((timeout) => clearTimeout(timeout));
       timeouts.clear();
     };
   }, []);
 
   const showToast = useCallback((message: string, type: ToastType = 'success') => {
+    if (!isMountedRef.current) return;
     const id = Math.random().toString(36).substr(2, 9);
     setToasts((prev) => [...prev, { id, message, type }]);
 
     // Auto-dismiss after 3 seconds
     const timeout = setTimeout(() => {
+      if (!isMountedRef.current) return;
       setToasts((prev) => prev.filter((t) => t.id !== id));
       timeoutRefs.current.delete(id);
     }, 3000);
@@ -101,6 +113,7 @@ export function ToastProvider({ children }: { children: ReactNode }) {
   }, []);
 
   const showPersistentToast = useCallback((id: string, message: string, type: ToastType = 'info') => {
+    if (!isMountedRef.current) return;
     setToasts((prev) => {
       // Update existing toast if same id, otherwise add new one
       const exists = prev.find((t) => t.id === id);
@@ -112,6 +125,7 @@ export function ToastProvider({ children }: { children: ReactNode }) {
   }, []);
 
   const dismissToast = useCallback((id: string) => {
+    if (!isMountedRef.current) return;
     // Clear any pending auto-dismiss timeout
     const timeout = timeoutRefs.current.get(id);
     if (timeout) {
@@ -268,6 +282,7 @@ export function ToastProvider({ children }: { children: ReactNode }) {
           const existingTimeout = timeoutRefs.current.get(dispatchToastId);
           if (existingTimeout) clearTimeout(existingTimeout);
           const timeout = setTimeout(() => {
+            if (!isMountedRef.current) return;
             setToasts((prev) => prev.filter((t) => t.id !== dispatchToastId));
             timeoutRefs.current.delete(dispatchToastId);
             lastDispatchSummaryRef.current = null;