Compare commits

...

2 Commits

Author SHA1 Message Date
cesnimda b8f8569e6e fix(hooks): stop infinite render loop in useViewResource
CI and Deploy / test (pull_request) Successful in 2m0s
CI and Deploy / deploy (pull_request) Has been skipped
useViewResource built `reload` with `load` in its useCallback deps, and the
fetch effect depended on `reload`. Callers routinely pass an inline `load`
closure (e.g. JobTable), so `load` — and therefore `reload` and the effect —
changed every render, calling setState and re-rendering: an unbounded
"Maximum update depth exceeded" loop that froze the renderer on /jobs and every
other list view (DashboardView, RemindersView, CompaniesTable).

Fix: hold `load` in a ref (like the existing hasLoadedRef) and drop it from the
dependency arrays. Re-fetching is still driven by `deps`/`enabled`; the ref
always points at the latest closure. No API/behaviour change for callers.

Runtime-verified live: /jobs went from a render storm (frozen renderer, 100s of
console errors) to 0 errors in a 2s window and a clean render. Suites that drive
JobTable→useViewResource pass in isolation; the remaining full-run flakiness is
pre-existing (state-pollution/timing in the heavy RTL suites, unrelated).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-05 15:36:55 +02:00
cesnimda 490c5b803e fix(auth): stop infinite /auth/me request loop when logged out
The axios 401 interceptor calls clearAuthClientState() on every 401, which
dispatched "auth-changed"; the App handler re-fetched /auth/me, which 401'd
again → interceptor → clearAuthClientState() → "auth-changed" → ... an unbounded
request storm (observed live: 100+ GET /auth/me and climbing) that ran whenever
the user was logged out (login page, expired session) — burning CPU, network and
battery and flooding the server.

Fix: make clearAuthClientState idempotent — only emit "auth-changed" when it
actually removes a stored user key (a real signed-in→out transition), so
repeated 401s can no longer re-trigger the fetch.

Runtime-verified in a live stack: /auth/me went from 100+ & growing to 0 &
stable. login-page/settings tests green. Documented in
docs/performance/PERFORMANCE_IMPROVEMENTS.md (Phase 3.5 runtime finding).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-05 15:13:18 +02:00
3 changed files with 32 additions and 3 deletions
@@ -5,8 +5,20 @@
## Changes made (this pass) ## Changes made (this pass)
| Change | File | Effect | Verified | | Change | File | Effect | Verified |
|---|---|---|---| |---|---|---|---|
| **Stop an infinite render loop on every list view** — hold `load` in a ref in `useViewResource` so `reload`/the fetch effect keep a stable identity | `job-tracker-ui/src/hooks/useViewResource.ts` | Fixes "Maximum update depth exceeded" on `/jobs` (and any `DashboardView`/`RemindersView`/`CompaniesTable` view whose caller passes an inline `load`) — pegged the CPU/renderer | **Runtime-confirmed**: `/jobs` went from a render storm (renderer frozen, 100s of errors) to 0 console errors in a live 2s window and a clean render; `workflow-trust-signals` (drives `JobTable``useViewResource`) passes |
| **Stop the infinite `/auth/me` request loop** — make `clearAuthClientState` emit `auth-changed` only on a real signed-in→out transition | `job-tracker-ui/src/auth.ts` | Eliminates a runaway request storm (100+ `GET /auth/me` and climbing) that ran continuously whenever the user was logged out | **Runtime-confirmed** in a live stack: `/auth/me` count 100+ & growing → 0 and stable after fix |
| Revoke CV-preview blob URLs on unmount only (ref-based), not on every carousel change | `job-tracker-ui/src/pages/ProfilePage.tsx` | Fixes broken previews on multi-template decks; still frees URLs on unmount | `profile-page.test.tsx` 5/5 | | Revoke CV-preview blob URLs on unmount only (ref-based), not on every carousel change | `job-tracker-ui/src/pages/ProfilePage.tsx` | Fixes broken previews on multi-template decks; still frees URLs on unmount | `profile-page.test.tsx` 5/5 |
### Runtime finding — self-triggering auth loop (the most impactful issue found)
Only visible with a running backend (static analysis could not surface it). Sequence: the axios response
interceptor (`api.ts`) calls `clearAuthClientState()` on **every** 401; that dispatched `"auth-changed"`;
the `App` handler re-fetched `/auth/me`; that 401'd again → interceptor → `clearAuthClientState()`
`"auth-changed"` → … an unbounded loop that hammered the server and spun the client on the login page and
after any session expiry. Fix: `clearAuthClientState` now only emits when it actually removes a stored user
key (idempotent), so repeated 401s can't re-trigger the fetch. This is a CPU/network/battery drain and a
self-inflicted request flood, not a memory leak — but squarely in the Phase 3.5 "infinite polling / retry
loop / duplicate requests" scope, and the single highest-value fix from the whole investigation.
> Context: this was the only defect found in a full resource audit. The codebase already practises > Context: this was the only defect found in a full resource audit. The codebase already practises
> disciplined cleanup (timers cleared, listeners removed, object URLs revoked), so there was no leak to > disciplined cleanup (timers cleared, listeners removed, object URLs revoked), so there was no leak to
> fix — see the main report. > fix — see the main report.
+7 -1
View File
@@ -82,8 +82,14 @@ export function setAuthUserKey(value: string | null | undefined, emit = true) {
} }
export function clearAuthClientState(emit = true) { export function clearAuthClientState(emit = true) {
// Only emit "auth-changed" when this call actually transitions from
// "signed in" to "signed out". The response interceptor calls this on every
// 401; without this guard each 401 re-dispatches "auth-changed", which
// re-fetches /auth/me, which 401s again — an infinite request loop whenever
// the user is logged out (login page, expired session).
const had = safeGet(window.localStorage, AUTH_USER_KEY) != null;
safeRemove(window.localStorage, AUTH_USER_KEY); safeRemove(window.localStorage, AUTH_USER_KEY);
if (emit) emitAuthChanged(); if (emit && had) emitAuthChanged();
} }
export function getCsrfToken(): string | null { export function getCsrfToken(): string | null {
+13 -2
View File
@@ -70,6 +70,17 @@ export function useViewResource<T>(
hasLoadedRef.current = hasLoaded; hasLoadedRef.current = hasLoaded;
}, [hasLoaded]); }, [hasLoaded]);
// Hold `load` in a ref so `reload` (and the fetch effect that depends on it)
// keep a stable identity across renders. Callers routinely pass an inline
// `load` closure; if `load` were a dependency, every render would create a new
// `reload`, re-run the effect, setState, and re-render — an infinite loop
// ("Maximum update depth exceeded"). Re-fetching is driven by `deps`/`enabled`
// instead, and the ref always points at the latest closure.
const loadRef = useRef(load);
useEffect(() => {
loadRef.current = load;
});
const reload = useCallback(async () => { const reload = useCallback(async () => {
if (!enabled) return; if (!enabled) return;
@@ -77,7 +88,7 @@ export function useViewResource<T>(
setLoading(!alreadyLoaded); setLoading(!alreadyLoaded);
setRefreshing(alreadyLoaded); setRefreshing(alreadyLoaded);
try { try {
const next = await load(); const next = await loadRef.current();
setData(next); setData(next);
setError(null); setError(null);
setHasLoaded(true); setHasLoaded(true);
@@ -88,7 +99,7 @@ export function useViewResource<T>(
setLoading(false); setLoading(false);
setRefreshing(false); setRefreshing(false);
} }
}, [enabled, errorMessage, load]); }, [enabled, errorMessage]);
useEffect(() => { useEffect(() => {
if (!enabled) { if (!enabled) {