From 8f4a022bddd8cfcaaacfbc76c38ccf19549b8745 Mon Sep 17 00:00:00 2001 From: JonathanLab Date: Thu, 7 May 2026 15:13:57 +0200 Subject: [PATCH] feat: suppress NotAuthenticatedError in ErrorBoundary and reset on auth state change Generated-By: PostHog Code Task-Id: dd3912cb-28de-4bd9-a74b-daef7ae8a22c --- apps/code/src/main/services/auth/service.ts | 5 +- apps/code/src/renderer/App.tsx | 7 +- .../components/ErrorBoundary.test.tsx | 162 ++++++++++++++++++ .../src/renderer/components/ErrorBoundary.tsx | 89 ++++++---- .../features/auth/hooks/authClient.ts | 3 +- apps/code/src/shared/errors.ts | 15 ++ 6 files changed, 242 insertions(+), 39 deletions(-) create mode 100644 apps/code/src/renderer/components/ErrorBoundary.test.tsx diff --git a/apps/code/src/main/services/auth/service.ts b/apps/code/src/main/services/auth/service.ts index fcd8c4d75..e59051aa1 100644 --- a/apps/code/src/main/services/auth/service.ts +++ b/apps/code/src/main/services/auth/service.ts @@ -1,5 +1,6 @@ import type { IPowerManager } from "@posthog/platform/power-manager"; import { OAUTH_SCOPE_VERSION } from "@shared/constants/oauth"; +import { NotAuthenticatedError } from "@shared/errors"; import type { CloudRegion } from "@shared/types/regions"; import { type BackoffOptions, sleepWithBackoff } from "@shared/utils/backoff"; import { getCloudUrlFromRegion } from "@shared/utils/urls"; @@ -336,7 +337,7 @@ export class AuthService extends TypedEventEmitter { const storedSession = this.resolveStoredSession(); if (!storedSession) { - throw new Error("Not authenticated"); + throw new NotAuthenticatedError(); } return storedSession; @@ -549,7 +550,7 @@ export class AuthService extends TypedEventEmitter { } private requireSession(): InMemorySession { if (!this.session) { - throw new Error("Not authenticated"); + throw new NotAuthenticatedError(); } return this.session; } diff --git a/apps/code/src/renderer/App.tsx b/apps/code/src/renderer/App.tsx index a3eea63c7..49095de09 100644 --- a/apps/code/src/renderer/App.tsx +++ b/apps/code/src/renderer/App.tsx @@ -21,6 +21,7 @@ import { useFocusStore } from "@renderer/stores/focusStore"; import { useThemeStore } from "@renderer/stores/themeStore"; import { initializeUpdateStore } from "@renderer/stores/updateStore"; import { trpcClient, useTRPC } from "@renderer/trpc/client"; +import { isNotAuthenticatedError } from "@shared/errors"; import { ANALYTICS_EVENTS } from "@shared/types/analytics"; import { useQueryClient } from "@tanstack/react-query"; import { useSubscription } from "@trpc/tanstack-react-query"; @@ -284,7 +285,11 @@ function App() { const content = renderContent(); return ( - + {isAuthenticated ? ( {content} ) : ( diff --git a/apps/code/src/renderer/components/ErrorBoundary.test.tsx b/apps/code/src/renderer/components/ErrorBoundary.test.tsx new file mode 100644 index 000000000..9dfe6b795 --- /dev/null +++ b/apps/code/src/renderer/components/ErrorBoundary.test.tsx @@ -0,0 +1,162 @@ +import { Theme } from "@radix-ui/themes"; +import { isNotAuthenticatedError, NotAuthenticatedError } from "@shared/errors"; +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import type { ReactNode } from "react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { ErrorBoundary } from "./ErrorBoundary"; + +vi.mock("@utils/analytics", () => ({ + captureException: vi.fn(), +})); + +vi.mock("@utils/logger", () => ({ + logger: { + scope: () => ({ + error: vi.fn(), + warn: vi.fn(), + info: vi.fn(), + debug: vi.fn(), + }), + }, +})); + +import { captureException } from "@utils/analytics"; + +function Thrower({ error }: { error: Error | null }) { + if (error) throw error; + return
ok
; +} + +function Boundary(props: { + children: ReactNode; + resetKey?: unknown; + shouldSuppress?: (e: Error) => boolean; + fallback?: ReactNode; +}) { + return ( + + + {props.children} + + + ); +} + +beforeEach(() => { + vi.spyOn(console, "error").mockImplementation(() => {}); +}); + +afterEach(() => { + vi.restoreAllMocks(); + vi.mocked(captureException).mockClear(); +}); + +describe("ErrorBoundary", () => { + it("renders children when no error is thrown", () => { + render( + + + , + ); + expect(screen.getByText("ok")).toBeInTheDocument(); + }); + + it("renders the default fallback UI on error and reports telemetry", () => { + render( + + + , + ); + expect(screen.getByText("Something went wrong")).toBeInTheDocument(); + expect(screen.getByText("boom")).toBeInTheDocument(); + expect(captureException).toHaveBeenCalledTimes(1); + }); + + it("renders custom fallback when provided", () => { + render( + custom fallback}> + + , + ); + expect(screen.getByText("custom fallback")).toBeInTheDocument(); + }); + + it("suppresses errors that match shouldSuppress (renders null, no telemetry)", () => { + render( + + + , + ); + expect(screen.queryByText("Something went wrong")).not.toBeInTheDocument(); + expect(screen.queryByText("ok")).not.toBeInTheDocument(); + expect(captureException).not.toHaveBeenCalled(); + }); + + it("does not suppress non-matching errors", () => { + render( + + + , + ); + expect(screen.getByText("Something went wrong")).toBeInTheDocument(); + expect(captureException).toHaveBeenCalledTimes(1); + }); + + it("clears error state when resetKey changes", () => { + const { rerender } = render( + + + , + ); + expect(screen.getByText("Something went wrong")).toBeInTheDocument(); + + rerender( + + + , + ); + expect(screen.queryByText("Something went wrong")).not.toBeInTheDocument(); + expect(screen.getByText("ok")).toBeInTheDocument(); + }); + + it("recovers via retry button", async () => { + const user = userEvent.setup(); + const { rerender } = render( + + + , + ); + expect(screen.getByText("Something went wrong")).toBeInTheDocument(); + + rerender( + + + , + ); + await user.click(screen.getByRole("button", { name: /try again/i })); + expect(screen.getByText("ok")).toBeInTheDocument(); + }); +}); + +describe("isNotAuthenticatedError", () => { + it("matches NotAuthenticatedError instances", () => { + expect(isNotAuthenticatedError(new NotAuthenticatedError())).toBe(true); + }); + + it("matches plain objects with the same name (e.g. tRPC-serialized errors)", () => { + expect(isNotAuthenticatedError({ name: "NotAuthenticatedError" })).toBe( + true, + ); + }); + + it("does not match unrelated errors", () => { + expect(isNotAuthenticatedError(new Error("Not authenticated"))).toBe(false); + expect(isNotAuthenticatedError(null)).toBe(false); + expect(isNotAuthenticatedError("Not authenticated")).toBe(false); + }); +}); diff --git a/apps/code/src/renderer/components/ErrorBoundary.tsx b/apps/code/src/renderer/components/ErrorBoundary.tsx index 2d2963216..4dabd1f96 100644 --- a/apps/code/src/renderer/components/ErrorBoundary.tsx +++ b/apps/code/src/renderer/components/ErrorBoundary.tsx @@ -11,24 +11,46 @@ interface Props { fallback?: ReactNode; /** Optional name to identify which boundary caught the error */ name?: string; + /** When this value changes, the boundary clears its error state. */ + resetKey?: unknown; + /** + * If returns true for a caught error, the boundary renders nothing, + * skips telemetry, and waits for `resetKey` to change before recovering. + * Use to handle transient errors that the surrounding tree will resolve + * (e.g. auth state about to flip to unauthenticated). + */ + shouldSuppress?: (error: Error) => boolean; } interface State { - hasError: boolean; error: Error | null; + lastResetKey: unknown; } export class ErrorBoundary extends Component { - constructor(props: Props) { - super(props); - this.state = { hasError: false, error: null }; + state: State = { error: null, lastResetKey: this.props.resetKey }; + + static getDerivedStateFromError(error: Error): Partial { + return { error }; } - static getDerivedStateFromError(error: Error): State { - return { hasError: true, error }; + static getDerivedStateFromProps( + props: Props, + state: State, + ): Partial | null { + if (props.resetKey === state.lastResetKey) return null; + return { error: null, lastResetKey: props.resetKey }; } componentDidCatch(error: Error, errorInfo: ErrorInfo) { + if (this.props.shouldSuppress?.(error)) { + log.warn("Suppressed error in boundary", { + name: this.props.name, + error: error.message, + }); + return; + } + log.error("Error caught by boundary", { name: this.props.name, error: error.message, @@ -44,39 +66,36 @@ export class ErrorBoundary extends Component { } handleRetry = () => { - this.setState({ hasError: false, error: null }); + this.setState({ error: null }); }; render() { - if (this.state.hasError) { - if (this.props.fallback) { - return this.props.fallback; - } + const { error } = this.state; + if (!error) return this.props.children; + if (this.props.shouldSuppress?.(error)) return null; + if (this.props.fallback) return this.props.fallback; - return ( - - - - - - - - Something went wrong - - {this.state.error?.message || "An unexpected error occurred"} - - - - + return ( + + + + + + + + Something went wrong + + {error.message || "An unexpected error occurred"} + + + - - - - ); - } - - return this.props.children; + + + + + ); } } diff --git a/apps/code/src/renderer/features/auth/hooks/authClient.ts b/apps/code/src/renderer/features/auth/hooks/authClient.ts index 2f88f54b2..42d23a199 100644 --- a/apps/code/src/renderer/features/auth/hooks/authClient.ts +++ b/apps/code/src/renderer/features/auth/hooks/authClient.ts @@ -1,5 +1,6 @@ import { PostHogAPIClient } from "@renderer/api/posthogClient"; import { trpcClient } from "@renderer/trpc/client"; +import { NotAuthenticatedError } from "@shared/errors"; import { getCloudUrlFromRegion } from "@shared/utils/urls"; import { useMemo } from "react"; import { @@ -56,7 +57,7 @@ export function useAuthenticatedClient(): PostHogAPIClient { const client = useOptionalAuthenticatedClient(); if (!client) { - throw new Error("Not authenticated"); + throw new NotAuthenticatedError(); } return client; diff --git a/apps/code/src/shared/errors.ts b/apps/code/src/shared/errors.ts index ec6687b04..37a9c727d 100644 --- a/apps/code/src/shared/errors.ts +++ b/apps/code/src/shared/errors.ts @@ -1,3 +1,18 @@ +export class NotAuthenticatedError extends Error { + constructor(message = "Not authenticated") { + super(message); + this.name = "NotAuthenticatedError"; + } +} + +export function isNotAuthenticatedError(error: unknown): boolean { + return ( + typeof error === "object" && + error !== null && + (error as { name?: unknown }).name === "NotAuthenticatedError" + ); +} + const AUTH_ERROR_PATTERNS = [ "authentication required", "failed to authenticate",