diff --git a/apps/shell-ui/src/hooks/useDashboardData.ts b/apps/shell-ui/src/hooks/useDashboardData.ts index 19ee757e4..a82765811 100644 --- a/apps/shell-ui/src/hooks/useDashboardData.ts +++ b/apps/shell-ui/src/hooks/useDashboardData.ts @@ -54,6 +54,7 @@ const MAX_EVENTS = 200; let _data: DashboardResponse | null = null; let _events: ActivityEvent[] = []; +let _error: string | null = null; let _refCount = 0; let _intervalId: ReturnType | null = null; let _eventUnsub: (() => void) | null = null; @@ -76,18 +77,20 @@ interface DashboardSnapshot { data: DashboardResponse | null; /** Activity events (newest first). */ events: ActivityEvent[]; + /** Last fetch failure, or null when healthy. */ + error: string | null; } // Explicitly annotated so control-flow analysis does not narrow the initial // `data` to `null` at this declaration (the function that assigns `_data` is // defined further down); without the annotation, the reassignment in `_emit` // below would not type-check. Runtime behavior is unchanged. -/** Stable snapshot object — only replaced when data or events change. */ -let _snapshot: DashboardSnapshot = { data: _data, events: _events }; +/** Stable snapshot object — replaced whenever _emit() publishes a state update. */ +let _snapshot: DashboardSnapshot = { data: _data, events: _events, error: _error }; /** Notify all subscribed React components that data changed. */ function _emit(): void { - _snapshot = { data: _data, events: _events }; + _snapshot = { data: _data, events: _events, error: _error }; _listeners.forEach(fn => fn()); } @@ -111,10 +114,58 @@ function _fetchDashboard(): Promise { const dashboard = await client.getDashboard(); if (dashboard?.overview) { _data = dashboard; + _error = null; _emit(); } } catch (err) { + // Surface the failure rather than only logging it. Swallowing it left + // `_data` null, and consumers render their loading state whenever data + // is null — so a permission denial was indistinguishable from a slow + // load and the view sat on "Loading..." forever (saas #373). console.log('[useDashboardData] Dashboard fetch failed:', err); + const message = err instanceof Error ? err.message : String(err); + + // getDashboard() goes through call(), so a structured DAP error arrives + // as DAPException with the detail in `dapResult` — the human message is + // not guaranteed to carry it. Matching prose alone therefore misses the + // case this branch exists for: a real denial would keep rendering the + // previous session's numbers under an "access denied" banner. Read the + // structured status first and fall back to the text. + const dapResult = + (err as { dapResult?: Record } | undefined)?.dapResult; + const status = Number(dapResult?.status ?? dapResult?.statusCode ?? dapResult?.code); + // `code` is not always numeric — the server may send a symbolic + // 'UNAUTHORIZED' / 'FORBIDDEN', which Number() turns into NaN. Testing + // only the numeric status and the prose would then miss the denial + // whenever the human message is generic, which is precisely the failure + // this branch exists to prevent: the previous session's figures would + // stay on screen under a vague error. Match the code as text too. + const code = String(dapResult?.code ?? ''); + const DENIAL = + /denied|permission|forbidden|unauthori[sz]ed|not\s*authori[sz]ed|\b40[13]\b/i; + const denied = + status === 401 || status === 403 || DENIAL.test(code) || DENIAL.test(message); + // The raw message already says "permission denied" in the case we most + // want to explain, so prefixing it produced "You do not have permission + // to view the dashboard. Permission denied". Replace it instead of + // appending — the verbatim text stays in the console line above for + // anyone debugging. + _error = denied + ? 'You do not have permission to view the dashboard.' + : message; + // Deliberate: `_data` is NOT cleared on a transient failure. A poll that + // blips should leave the last good numbers on screen with an error + // alongside them, not blank the dashboard every time the network hiccups + // — going empty on each failed poll would be its own bug. + // + // A denial is different. It means this user is not entitled to these + // numbers, and continuing to display a previous session's data under an + // "access denied" banner is both confusing and the wrong default for + // something access-scoped. So clear it in that case only. + if (denied) { + _data = null; + } + _emit(); } finally { _fetchPromise = null; } @@ -182,6 +233,8 @@ export interface DashboardData { data: DashboardResponse | null; /** Activity events (newest first). */ events: ActivityEvent[]; + /** Last fetch failure (e.g. a permission denial), or null when healthy. */ + error: string | null; /** Trigger a manual refresh. */ refresh: () => void; } @@ -237,6 +290,7 @@ export function useDashboardData(): DashboardData { return { data: snapshot.data, events: snapshot.events, + error: snapshot.error, refresh, }; }