fix(#411): classifyAuthError — return null for normal 401/session-expired instead of 'backend'
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 <noreply@anthropic.com>
This commit is contained in:
@@ -270,13 +270,9 @@ describe("AuthContext", (): void => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
describe("auth error handling", (): void => {
|
describe("auth error handling", (): void => {
|
||||||
it("should set authError to 'backend' for unrecognised auth errors (e.g. 401/403)", async (): Promise<void> => {
|
it("should set authError to null for normal 401 Unauthorized (not logged in)", async (): Promise<void> => {
|
||||||
const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {
|
// 401 Unauthorized is a normal condition (user not logged in), not an error.
|
||||||
// Intentionally empty
|
// classifyAuthError should return null so no "having trouble" banner appears.
|
||||||
});
|
|
||||||
|
|
||||||
// Error instances that don't match network/server keywords default to "backend"
|
|
||||||
// so they surface a banner rather than silently logging the user out
|
|
||||||
vi.mocked(apiGet).mockRejectedValueOnce(new Error("Unauthorized"));
|
vi.mocked(apiGet).mockRejectedValueOnce(new Error("Unauthorized"));
|
||||||
|
|
||||||
render(
|
render(
|
||||||
@@ -289,7 +285,63 @@ describe("AuthContext", (): void => {
|
|||||||
expect(screen.getByTestId("auth-status")).toHaveTextContent("Not Authenticated");
|
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<void> => {
|
||||||
|
// 403 Forbidden is also classified as invalid_credentials by parseAuthError
|
||||||
|
vi.mocked(apiGet).mockRejectedValueOnce(new Error("Forbidden"));
|
||||||
|
|
||||||
|
render(
|
||||||
|
<AuthProvider>
|
||||||
|
<TestComponent />
|
||||||
|
</AuthProvider>
|
||||||
|
);
|
||||||
|
|
||||||
|
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<void> => {
|
||||||
|
// "session expired" is a normal auth lifecycle event, not a backend error
|
||||||
|
vi.mocked(apiGet).mockRejectedValueOnce(new Error("Session expired"));
|
||||||
|
|
||||||
|
render(
|
||||||
|
<AuthProvider>
|
||||||
|
<TestComponent />
|
||||||
|
</AuthProvider>
|
||||||
|
);
|
||||||
|
|
||||||
|
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<void> => {
|
||||||
|
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(
|
||||||
|
<AuthProvider>
|
||||||
|
<TestComponent />
|
||||||
|
</AuthProvider>
|
||||||
|
);
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(screen.getByTestId("auth-status")).toHaveTextContent("Not Authenticated");
|
||||||
|
});
|
||||||
|
|
||||||
expect(screen.getByTestId("auth-error")).toHaveTextContent("backend");
|
expect(screen.getByTestId("auth-error")).toHaveTextContent("backend");
|
||||||
|
|
||||||
consoleErrorSpy.mockRestore();
|
consoleErrorSpy.mockRestore();
|
||||||
|
|||||||
@@ -41,6 +41,9 @@ const AuthContext = createContext<AuthContextValue | undefined>(undefined);
|
|||||||
* Classify an error into an {@link AuthErrorType} using the centralised
|
* Classify an error into an {@link AuthErrorType} using the centralised
|
||||||
* {@link parseAuthError} utility.
|
* {@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`
|
* Defaults unrecognised `Error` instances to `"backend"` rather than `null`
|
||||||
* so that unexpected failures surface a "having trouble connecting" banner
|
* so that unexpected failures surface a "having trouble connecting" banner
|
||||||
* instead of silently logging the user out.
|
* instead of silently logging the user out.
|
||||||
@@ -49,7 +52,10 @@ function classifyAuthError(error: unknown): AuthErrorType {
|
|||||||
const parsed = parseAuthError(error);
|
const parsed = parseAuthError(error);
|
||||||
if (parsed.code === "network_error") return "network";
|
if (parsed.code === "network_error") return "network";
|
||||||
if (parsed.code === "server_error") return "backend";
|
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)
|
// (safer to show "having trouble connecting" than silently log out)
|
||||||
if (error instanceof Error) return "backend";
|
if (error instanceof Error) return "backend";
|
||||||
return null;
|
return null;
|
||||||
|
|||||||
Reference in New Issue
Block a user