From d7de20e586adeeab2c8a76c4e2a4690c046f8741 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Mon, 16 Feb 2026 15:42:44 -0600 Subject: [PATCH] =?UTF-8?q?fix(#411):=20classifyAuthError=20=E2=80=94=20re?= =?UTF-8?q?turn=20null=20for=20normal=20401/session-expired=20instead=20of?= =?UTF-8?q?=20'backend'?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Normal authentication failures (401 Unauthorized, 403 Forbidden, session expired) are not backend errors — they simply mean the user isn't logged in. Previously these fell through to the `instanceof Error` catch-all and returned "backend", causing a misleading "having trouble connecting" banner. Now classifyAuthError explicitly checks for invalid_credentials and session_expired codes from parseAuthError and returns null, so the UI shows the logged-out state cleanly without an error banner. Co-Authored-By: Claude Opus 4.6 --- apps/web/src/lib/auth/auth-context.test.tsx | 68 ++++++++++++++++++--- apps/web/src/lib/auth/auth-context.tsx | 8 ++- 2 files changed, 67 insertions(+), 9 deletions(-) diff --git a/apps/web/src/lib/auth/auth-context.test.tsx b/apps/web/src/lib/auth/auth-context.test.tsx index 2a0b013..74727a0 100644 --- a/apps/web/src/lib/auth/auth-context.test.tsx +++ b/apps/web/src/lib/auth/auth-context.test.tsx @@ -270,13 +270,9 @@ describe("AuthContext", (): void => { }); describe("auth error handling", (): void => { - it("should set authError to 'backend' for unrecognised auth errors (e.g. 401/403)", async (): Promise => { - const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => { - // Intentionally empty - }); - - // Error instances that don't match network/server keywords default to "backend" - // so they surface a banner rather than silently logging the user out + it("should set authError to null for normal 401 Unauthorized (not logged in)", async (): Promise => { + // 401 Unauthorized is a normal condition (user not logged in), not an error. + // classifyAuthError should return null so no "having trouble" banner appears. vi.mocked(apiGet).mockRejectedValueOnce(new Error("Unauthorized")); render( @@ -289,7 +285,63 @@ describe("AuthContext", (): void => { expect(screen.getByTestId("auth-status")).toHaveTextContent("Not Authenticated"); }); - // With classifyAuthError, unrecognised Error instances default to "backend" + // authError should be null (displayed as "none" by TestComponent) + expect(screen.getByTestId("auth-error")).toHaveTextContent("none"); + }); + + it("should set authError to null for 403 Forbidden", async (): Promise => { + // 403 Forbidden is also classified as invalid_credentials by parseAuthError + vi.mocked(apiGet).mockRejectedValueOnce(new Error("Forbidden")); + + render( + + + + ); + + await waitFor(() => { + expect(screen.getByTestId("auth-status")).toHaveTextContent("Not Authenticated"); + }); + + expect(screen.getByTestId("auth-error")).toHaveTextContent("none"); + }); + + it("should set authError to null for session expired errors", async (): Promise => { + // "session expired" is a normal auth lifecycle event, not a backend error + vi.mocked(apiGet).mockRejectedValueOnce(new Error("Session expired")); + + render( + + + + ); + + await waitFor(() => { + expect(screen.getByTestId("auth-status")).toHaveTextContent("Not Authenticated"); + }); + + expect(screen.getByTestId("auth-error")).toHaveTextContent("none"); + }); + + it("should set authError to 'backend' for truly unrecognised Error instances", async (): Promise => { + const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => { + // Intentionally empty + }); + + // An Error that doesn't match any known pattern (parseAuthError returns "unknown") + // should fall through to the instanceof Error catch-all and return "backend" + vi.mocked(apiGet).mockRejectedValueOnce(new Error("Something completely unexpected happened")); + + render( + + + + ); + + await waitFor(() => { + expect(screen.getByTestId("auth-status")).toHaveTextContent("Not Authenticated"); + }); + expect(screen.getByTestId("auth-error")).toHaveTextContent("backend"); consoleErrorSpy.mockRestore(); diff --git a/apps/web/src/lib/auth/auth-context.tsx b/apps/web/src/lib/auth/auth-context.tsx index 1602590..1b25acc 100644 --- a/apps/web/src/lib/auth/auth-context.tsx +++ b/apps/web/src/lib/auth/auth-context.tsx @@ -41,6 +41,9 @@ const AuthContext = createContext(undefined); * Classify an error into an {@link AuthErrorType} using the centralised * {@link parseAuthError} utility. * + * Normal authentication failures (401 Unauthorized, session expired) return + * `null` so the UI simply shows the logged-out state without a banner. + * * Defaults unrecognised `Error` instances to `"backend"` rather than `null` * so that unexpected failures surface a "having trouble connecting" banner * instead of silently logging the user out. @@ -49,7 +52,10 @@ function classifyAuthError(error: unknown): AuthErrorType { const parsed = parseAuthError(error); if (parsed.code === "network_error") return "network"; if (parsed.code === "server_error") return "backend"; - // For unrecognised errors, default to "backend" rather than null + // Normal auth failures (not logged in, session expired) are not errors — + // return null so the UI shows logged-out state without a banner + if (parsed.code === "invalid_credentials" || parsed.code === "session_expired") return null; + // For truly unrecognised errors, default to "backend" rather than null // (safer to show "having trouble connecting" than silently log out) if (error instanceof Error) return "backend"; return null;