Compare commits
2 Commits
39266c0935
...
b8f8569e6e
| Author | SHA1 | Date | |
|---|---|---|---|
| b8f8569e6e | |||
| 490c5b803e |
@@ -5,8 +5,20 @@
|
||||
## Changes made (this pass)
|
||||
| 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 |
|
||||
|
||||
### 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
|
||||
> disciplined cleanup (timers cleared, listeners removed, object URLs revoked), so there was no leak to
|
||||
> fix — see the main report.
|
||||
|
||||
@@ -82,8 +82,14 @@ export function setAuthUserKey(value: string | null | undefined, 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);
|
||||
if (emit) emitAuthChanged();
|
||||
if (emit && had) emitAuthChanged();
|
||||
}
|
||||
|
||||
export function getCsrfToken(): string | null {
|
||||
|
||||
@@ -70,6 +70,17 @@ export function useViewResource<T>(
|
||||
hasLoadedRef.current = 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 () => {
|
||||
if (!enabled) return;
|
||||
|
||||
@@ -77,7 +88,7 @@ export function useViewResource<T>(
|
||||
setLoading(!alreadyLoaded);
|
||||
setRefreshing(alreadyLoaded);
|
||||
try {
|
||||
const next = await load();
|
||||
const next = await loadRef.current();
|
||||
setData(next);
|
||||
setError(null);
|
||||
setHasLoaded(true);
|
||||
@@ -88,7 +99,7 @@ export function useViewResource<T>(
|
||||
setLoading(false);
|
||||
setRefreshing(false);
|
||||
}
|
||||
}, [enabled, errorMessage, load]);
|
||||
}, [enabled, errorMessage]);
|
||||
|
||||
useEffect(() => {
|
||||
if (!enabled) {
|
||||
|
||||
Reference in New Issue
Block a user