Skip to content
Merged
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 52 additions & 3 deletions apps/shell-ui/src/hooks/useDashboardData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof setInterval> | null = null;
let _eventUnsub: (() => void) | null = null;
Expand All @@ -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());
}

Expand All @@ -111,10 +114,53 @@ function _fetchDashboard(): Promise<void> {
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<string, unknown> } | undefined)?.dapResult;
const status = Number(dapResult?.status ?? dapResult?.statusCode ?? dapResult?.code);
const denied =
status === 401 ||
status === 403 ||
/denied|permission|forbidden|unauthori[sz]ed|not\s*authori[sz]ed|\b40[13]\b/i.test(
message,
);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
// 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;
}
Expand Down Expand Up @@ -182,6 +228,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;
}
Expand Down Expand Up @@ -237,6 +285,7 @@ export function useDashboardData(): DashboardData {
return {
data: snapshot.data,
events: snapshot.events,
error: snapshot.error,
refresh,
};
}
Loading