From 6f5b2db95e2b2598934c52dcc0eb6b30da1aeb53 Mon Sep 17 00:00:00 2001 From: anonpenguin23 Date: Fri, 12 Jun 2026 16:44:37 +0300 Subject: [PATCH] fix(sdk): surface typed SDKError on network/timeout failures (#129) The HTTP client re-threw the raw platform error on a transport failure, so callers (e.g. AnChat's JwtSession driving client.auth.refresh/challenge) could only tell "couldn't reach the gateway" from a real HTTP error by string- matching `TypeError: Network request failed`. The typed SDKError was built only for the onNetworkError callback, never for the thrown error, and native errors weren't retryable so they bubbled raw. normalizeError() now maps a fetch failure -> SDKError{code:"NETWORK_ERROR", httpStatus:0} and a timeout AbortError -> {code:"TIMEOUT",httpStatus:0}, and that typed error is thrown to the caller. Genuine HTTP errors pass through unchanged. httpStatus:0 is the stable "no HTTP response received" signal to branch on; the original platform message is preserved for diagnostics. Deliberately NOT auto-retrying network failures: a blind retry on a non- idempotent POST like /v1/auth/refresh could burn the rotated refresh token on a lost response. The SDK now just gives the app a typed error to drive its own retry/failover. Tests: tests/unit/http/network-errors-bug-129.test.ts (TypeError->NETWORK_ERROR, AbortError->TIMEOUT, real 401 passes through, callback gets the typed error). Full unit suite green, typecheck clean. --- sdk/src/core/http.ts | 55 +++++++++--- .../unit/http/network-errors-bug-129.test.ts | 88 +++++++++++++++++++ 2 files changed, 131 insertions(+), 12 deletions(-) create mode 100644 sdk/tests/unit/http/network-errors-bug-129.test.ts diff --git a/sdk/src/core/http.ts b/sdk/src/core/http.ts index 470def4..491b5df 100644 --- a/sdk/src/core/http.ts +++ b/sdk/src/core/http.ts @@ -167,6 +167,41 @@ export class HttpClient { return this.baseURL; } + /** + * Normalize any thrown error into a typed SDKError so callers can branch on + * `.code`/`.httpStatus` instead of string-matching a bare platform + * `TypeError: Network request failed` (bugboard #129). + * + * - SDKError (an HTTP error response) passes through unchanged. + * - An AbortError (our own per-request timeout firing) → code "TIMEOUT". + * - Anything else (fetch rejects with a TypeError on DNS failure, connection + * refused, offline, or TLS error) → code "NETWORK_ERROR". + * + * In every network case httpStatus is 0 (no HTTP response was received), which + * is how the app distinguishes "couldn't reach the gateway" from a real 4xx/5xx. + */ + private normalizeError(error: unknown, timeoutMs: number): SDKError { + if (error instanceof SDKError) { + return error; + } + const name = (error as { name?: string })?.name; + const message = error instanceof Error ? error.message : String(error); + if (name === "AbortError") { + return new SDKError( + `request timed out after ${timeoutMs}ms`, + 0, + "TIMEOUT", + { cause: name } + ); + } + return new SDKError( + message || "network request failed", + 0, + "NETWORK_ERROR", + { cause: name } + ); + } + async request( method: "GET" | "POST" | "PUT" | "DELETE", path: string, @@ -298,18 +333,14 @@ export class HttpClient { } } - // Call the network error callback if configured - // This allows the app to trigger gateway failover + // Normalize native errors (TypeError, AbortError) into a typed SDKError + // so the app gets a stable `.code`/`.httpStatus` instead of a bare + // platform "Network request failed" (bugboard #129). + const sdkError = this.normalizeError(error, requestTimeout); + + // Call the network error callback if configured. This allows the app to + // trigger gateway failover. if (this.onNetworkError) { - // Convert native errors (TypeError, AbortError) to SDKError for the callback - const sdkError = - error instanceof SDKError - ? error - : new SDKError( - error instanceof Error ? error.message : String(error), - 0, // httpStatus 0 indicates network-level failure - "NETWORK_ERROR" - ); this.onNetworkError(sdkError, { method, path, @@ -318,7 +349,7 @@ export class HttpClient { }); } - throw error; + throw sdkError; } finally { clearTimeout(timeoutId); } diff --git a/sdk/tests/unit/http/network-errors-bug-129.test.ts b/sdk/tests/unit/http/network-errors-bug-129.test.ts new file mode 100644 index 0000000..76a1af3 --- /dev/null +++ b/sdk/tests/unit/http/network-errors-bug-129.test.ts @@ -0,0 +1,88 @@ +import { describe, it, expect, vi } from "vitest"; +import { HttpClient } from "../../../src/core/http"; +import { SDKError } from "../../../src/errors"; + +/** + * Bugboard #129 — typed network errors. + * + * Before this fix the HttpClient re-threw the raw platform error on a + * network-level failure, so a caller (e.g. AnChat's JwtSession) could only + * tell "couldn't reach the gateway" apart from a real HTTP error by + * string-matching `TypeError: Network request failed`. These guards lock in + * that every transport failure surfaces as a typed SDKError with httpStatus 0 + * and a stable `.code`, while genuine HTTP errors keep their status/code. + */ +describe("Bug #129 — HttpClient surfaces typed network errors", () => { + function client(fetchImpl: typeof fetch, onNetworkError?: any) { + return new HttpClient({ + baseURL: "https://gw.example", + maxRetries: 0, + timeout: 5000, + fetch: fetchImpl, + onNetworkError, + }); + } + + it("maps a fetch TypeError (connection failure) to SDKError NETWORK_ERROR / status 0", async () => { + const fetchSpy = vi.fn(async () => { + throw new TypeError("Network request failed"); + }); + const err = await client(fetchSpy as any) + .post("/v1/auth/refresh", { refresh_token: "x" }) + .catch((e) => e); + + expect(err).toBeInstanceOf(SDKError); + expect(err.code).toBe("NETWORK_ERROR"); + expect(err.httpStatus).toBe(0); + // Original platform message is preserved for diagnostics. + expect(err.message).toContain("Network request failed"); + }); + + it("maps an AbortError (timeout) to SDKError TIMEOUT / status 0", async () => { + const fetchSpy = vi.fn(async () => { + const e = new Error("aborted"); + e.name = "AbortError"; + throw e; + }); + const err = await client(fetchSpy as any) + .get("/v1/auth/challenge") + .catch((e) => e); + + expect(err).toBeInstanceOf(SDKError); + expect(err.code).toBe("TIMEOUT"); + expect(err.httpStatus).toBe(0); + expect(err.message).toContain("5000ms"); + }); + + it("passes a real HTTP error through unchanged (not masked as NETWORK_ERROR)", async () => { + const fetchSpy = vi.fn( + async () => + new Response(JSON.stringify({ error: "nope", code: "BAD_TOKEN" }), { + status: 401, + headers: { "content-type": "application/json" }, + }) + ); + const err = await client(fetchSpy as any) + .post("/v1/auth/refresh", { refresh_token: "x" }) + .catch((e) => e); + + expect(err).toBeInstanceOf(SDKError); + expect(err.httpStatus).toBe(401); + expect(err.code).toBe("BAD_TOKEN"); + }); + + it("hands the typed error (not the raw TypeError) to the onNetworkError callback", async () => { + const seen: SDKError[] = []; + const fetchSpy = vi.fn(async () => { + throw new TypeError("Failed to fetch"); + }); + await client(fetchSpy as any, (e: SDKError) => seen.push(e)) + .get("/v1/db/read") + .catch(() => {}); + + expect(seen).toHaveLength(1); + expect(seen[0]).toBeInstanceOf(SDKError); + expect(seen[0].code).toBe("NETWORK_ERROR"); + expect(seen[0].httpStatus).toBe(0); + }); +});