fix(auth): stop infinite SSE retry on 401/403, surface auth failures (#79)
* fix(auth): stop infinite SSE retry on 401/403 and surface auth failures Root cause (Sentry OPENCODE-MOBILE-1, 309 events / 65 users): auth is static HTTP Basic and no code path treated 401 specially. The SSE reconnect loop in events.ts retried on a fixed backoff regardless of cause, so a bad password spammed Sentry and drained battery forever with zero user feedback. Advanced-mode connection save also had no pre-flight check and silently persisted bad credentials as the active connection. - src/lib/api-error.ts: new pure ApiAuthError/isAuthStatus/isAuthError module (node --test covered) so 401/403 are distinguishable from other failures. - src/lib/sdk.ts: request()/events() now throw ApiAuthError for 401/403 instead of a generic Error. - src/stores/events.ts: the SSE loop stops retrying on an auth error and sets a new `authError` flag instead of reconnecting forever; other errors keep the existing backoff. Fires connection_failed (source: sse, error_class: unauthorized) so it's visible in the existing funnel. - app/(tabs)/index.tsx: sessions screen shows an "Authentication Failed" state with a link to the connection edit screen when authError is set. - app/connection/[id].tsx: saving edited credentials for the active connection now reconnects SSE immediately instead of requiring an app restart. - app/connection/add.tsx: Advanced-mode save now runs the same testConnection pre-flight as Quick Connect and shows the same "Connection Failed" alert (with diagnostics/share-report) instead of silently saving bad credentials. Closes #76 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NJKAQ6HAikWGQK7PGZ5Y4E * fix(auth): add Retry button on 401 error state, widen ConnectionTestSource - Authentication Failed screen now offers Retry alongside Check Credentials, calling events store's connect() directly to restart the SSE state machine on transient 401s without leaving the app. - Widen ConnectionTestSource to include 'sse' (events.ts:389's connection_failed track call) and note the activation funnel only filters on source=onboarding. Addresses PR #79 review follow-ups. --------- Co-authored-by: engineer <engineer@gray-knight-m1.local> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -87,11 +87,12 @@ export enum AnalyticsEvent {
|
||||
ResponseReceived = "response_received",
|
||||
}
|
||||
|
||||
/** Where a connection test was initiated from. The activation funnel filters
|
||||
* to source=onboarding; edit_test covers the Test button on the existing-
|
||||
* connection edit screen, which would otherwise pollute the funnel with
|
||||
* repeat-tester noise. */
|
||||
export type ConnectionTestSource = "onboarding" | "edit_test"
|
||||
/** Where a connection test/failure was initiated from. The activation funnel
|
||||
* filters to source=onboarding only; edit_test (Test button on the
|
||||
* existing-connection edit screen) and sse (background reconnect loop,
|
||||
* see events.ts) would otherwise pollute the funnel with repeat-tester and
|
||||
* post-activation noise. */
|
||||
export type ConnectionTestSource = "onboarding" | "edit_test" | "sse"
|
||||
|
||||
export function initAnalytics() {
|
||||
if (enabled) return
|
||||
|
||||
42
src/lib/api-error.test.ts
Normal file
42
src/lib/api-error.test.ts
Normal file
@@ -0,0 +1,42 @@
|
||||
import { test } from "node:test"
|
||||
import assert from "node:assert/strict"
|
||||
import { ApiAuthError, apiErrorFor, isAuthError, isAuthStatus } from "./api-error.ts"
|
||||
|
||||
test("isAuthStatus: 401 and 403 are auth failures", () => {
|
||||
assert.equal(isAuthStatus(401), true)
|
||||
assert.equal(isAuthStatus(403), true)
|
||||
})
|
||||
|
||||
test("isAuthStatus: other statuses are not auth failures", () => {
|
||||
assert.equal(isAuthStatus(200), false)
|
||||
assert.equal(isAuthStatus(404), false)
|
||||
assert.equal(isAuthStatus(500), false)
|
||||
assert.equal(isAuthStatus(503), false)
|
||||
assert.equal(isAuthStatus(0), false)
|
||||
})
|
||||
|
||||
test("apiErrorFor: 401/403 produce an ApiAuthError carrying the status", () => {
|
||||
const err401 = apiErrorFor(401, "API Error: 401 - Unauthorized")
|
||||
assert.ok(err401 instanceof ApiAuthError)
|
||||
assert.equal(err401.status, 401)
|
||||
assert.equal(err401.message, "API Error: 401 - Unauthorized")
|
||||
|
||||
const err403 = apiErrorFor(403, "API Error: 403 - Forbidden")
|
||||
assert.ok(err403 instanceof ApiAuthError)
|
||||
assert.equal(err403.status, 403)
|
||||
})
|
||||
|
||||
test("apiErrorFor: other statuses produce a plain Error, not ApiAuthError", () => {
|
||||
const err = apiErrorFor(500, "API Error: 500 - Internal Server Error")
|
||||
assert.ok(err instanceof Error)
|
||||
assert.equal(err instanceof ApiAuthError, false)
|
||||
assert.equal(err.message, "API Error: 500 - Internal Server Error")
|
||||
})
|
||||
|
||||
test("isAuthError: type guard matches only ApiAuthError instances", () => {
|
||||
assert.equal(isAuthError(new ApiAuthError(401, "nope")), true)
|
||||
assert.equal(isAuthError(new Error("some other error")), false)
|
||||
assert.equal(isAuthError("401"), false)
|
||||
assert.equal(isAuthError(undefined), false)
|
||||
assert.equal(isAuthError(null), false)
|
||||
})
|
||||
38
src/lib/api-error.ts
Normal file
38
src/lib/api-error.ts
Normal file
@@ -0,0 +1,38 @@
|
||||
// Pure classification of HTTP auth failures, extracted so it's unit-testable
|
||||
// under plain `node --test` without pulling in expo/fetch (sdk.ts is RN-only)
|
||||
// — same pattern as analytics-classify.ts / diagnostics-classify.ts.
|
||||
//
|
||||
// Why this exists: sdk.ts's request()/events() used to throw a generic Error
|
||||
// for every non-2xx response, so call sites (the SSE reconnect loop, screen
|
||||
// error states) had no way to tell "your password is wrong" apart from "the
|
||||
// server is briefly unreachable" — a 401 got treated like any transient
|
||||
// failure and retried forever (see events.ts's reconnect loop / issue #76).
|
||||
|
||||
/** Thrown by sdk.ts's request()/events() when the server responds 401/403,
|
||||
* so call sites can distinguish "bad credentials" from any other failure
|
||||
* (network error, 5xx, timeout) instead of catching a generic Error. */
|
||||
export class ApiAuthError extends Error {
|
||||
readonly status: number
|
||||
constructor(status: number, message: string) {
|
||||
super(message)
|
||||
this.name = "ApiAuthError"
|
||||
this.status = status
|
||||
}
|
||||
}
|
||||
|
||||
/** True for HTTP statuses that mean "your credentials are wrong", as opposed
|
||||
* to transient/network/server failures that should keep retrying. */
|
||||
export function isAuthStatus(status: number): boolean {
|
||||
return status === 401 || status === 403
|
||||
}
|
||||
|
||||
/** Build the right error type for a failed HTTP response. */
|
||||
export function apiErrorFor(status: number, message: string): Error {
|
||||
return isAuthStatus(status) ? new ApiAuthError(status, message) : new Error(message)
|
||||
}
|
||||
|
||||
/** Type guard for call sites (e.g. the SSE reconnect loop) that need to branch
|
||||
* on whether a caught error was an auth failure. */
|
||||
export function isAuthError(error: unknown): error is ApiAuthError {
|
||||
return error instanceof ApiAuthError
|
||||
}
|
||||
@@ -5,6 +5,9 @@
|
||||
import { fetch as expoFetch } from "expo/fetch"
|
||||
import { buildRequestHeaders } from "./headers"
|
||||
import { SSEParser } from "./sse"
|
||||
import { apiErrorFor } from "./api-error"
|
||||
|
||||
export { ApiAuthError, isAuthError } from "./api-error"
|
||||
|
||||
export interface ClientConfig {
|
||||
baseUrl: string
|
||||
@@ -196,7 +199,7 @@ async function request<T>(config: ClientConfig, path: string, options: RequestIn
|
||||
|
||||
if (!response.ok) {
|
||||
const error = await response.text()
|
||||
throw new ApiError(response.status, error)
|
||||
throw apiErrorFor(response.status, `API Error: ${response.status} - ${error}`)
|
||||
}
|
||||
|
||||
return response.json()
|
||||
@@ -243,7 +246,7 @@ export function createClient(config: ClientConfig) {
|
||||
// Must use expo/fetch for ReadableStream support on native
|
||||
const response = await expoFetch(url, { headers, signal })
|
||||
if (!response.ok || !response.body) {
|
||||
throw new Error(`Failed to connect to event stream: ${response.status}`)
|
||||
throw apiErrorFor(response.status, `Failed to connect to event stream: ${response.status}`)
|
||||
}
|
||||
|
||||
const reader = response.body.getReader()
|
||||
|
||||
Reference in New Issue
Block a user