mirror of
https://github.com/DeBrosOfficial/orama.git
synced 2026-06-16 22:54:12 +00:00
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.
This commit is contained in:
parent
e7ed718965
commit
6f5b2db95e
@ -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<T = any>(
|
||||
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);
|
||||
}
|
||||
|
||||
88
sdk/tests/unit/http/network-errors-bug-129.test.ts
Normal file
88
sdk/tests/unit/http/network-errors-bug-129.test.ts
Normal file
@ -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);
|
||||
});
|
||||
});
|
||||
Loading…
x
Reference in New Issue
Block a user