Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
11 changes: 9 additions & 2 deletions desktop/src/components/AppErrorBoundary.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
* (We don't pretend to handle arbitrary failures; that's the job of
* the surrounding observability stack and per-app error states.)
*/
import { Component, type ReactNode } from "react";
import { Component, type ErrorInfo, type ReactNode } from "react";
import { Loader2, RefreshCw, AlertCircle } from "lucide-react";
import { BackendUnavailableError } from "@/lib/taos-fetch";

Expand Down Expand Up @@ -44,7 +44,14 @@ export class AppErrorBoundary extends Component<Props, State> {
return { mode: classify(err) };
}

componentDidCatch() {
componentDidCatch(error: Error, info: ErrorInfo) {
// The generic fallback intentionally hides the error from the user, but it
// must not be swallowed: log the error and the component stack so a crash
// is diagnosable from the console and reachable by the observability stack.
// Skip the "waiting"/"chunk" modes -- those are expected, handled states.
if (this.state.mode === "generic") {
console.error("[AppErrorBoundary]", error, info?.componentStack ?? "");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: Defensive info?.componentStack ?? "" is misleading

ErrorInfo from react types componentStack as a required non-optional string (see @types/react ErrorInfo). React never invokes componentDidCatch with a nullish info or with componentStack missing. The optional-chaining + nullish coalesce suggest these are realistic failure modes when they aren't, and they obscure the real contract from future readers. Drop the noise:

Suggested change
console.error("[AppErrorBoundary]", error, info?.componentStack ?? "");
console.error("[AppErrorBoundary]", error, info.componentStack);

Reply with @kilocode-bot fix it to have Kilo Code address this issue.

}
if (this.state.mode === "chunk" && !this.reloadTimer) {
this.reloadTimer = setTimeout(() => window.location.reload(), 5_000);
}
Expand Down
26 changes: 26 additions & 0 deletions desktop/src/components/__tests__/AppErrorBoundary.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -84,4 +84,30 @@ describe("<AppErrorBoundary />", () => {
expect(screen.queryByText(/taOS was updated/i)).toBeNull();
expect(screen.getByText(/something went wrong/i)).toBeInTheDocument();
});

it("logs the real error + component stack for a generic crash (diagnosable)", () => {
render(
<AppErrorBoundary>
<Thrower err={new ReferenceError("boom")} />
</AppErrorBoundary>
);
// The boundary surfaces the underlying error rather than swallowing it.
const logged = consoleErr.mock.calls.some(
(args) =>
args[0] === "[AppErrorBoundary]" &&
args[1] instanceof Error &&
(args[1] as Error).message === "boom",
);
expect(logged).toBe(true);
});

it("does not log for the expected waiting state", () => {
render(
<AppErrorBoundary>
<Thrower err={new BackendUnavailableError()} />
</AppErrorBoundary>
);
const loggedOurs = consoleErr.mock.calls.some((args) => args[0] === "[AppErrorBoundary]");
expect(loggedOurs).toBe(false);
});
});
Loading