diff --git a/CHANGELOG.md b/CHANGELOG.md index c14551b..cb8b20c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +### Updated + +- Improvements to error reporting including for fetch failures. + ## 0.6.1 - 2026-07-28 ### Updated diff --git a/src/commandHelpers.ts b/src/commandHelpers.ts index 8b5bd78..ce92030 100644 --- a/src/commandHelpers.ts +++ b/src/commandHelpers.ts @@ -65,11 +65,7 @@ export function handleError( } } else { if (error instanceof HttpError) { - process.stderr.write( - pc.red( - `The API returned an error with status code ${error.status}:\n\n${JSON.stringify(error.body, null, 2)}\n`, - ), - ); + process.stderr.write(pc.red(formatHttpError(error))); } else if ( error instanceof CommanderError && (error.code === "commander.help" || error.message === "(outputHelp)") @@ -96,6 +92,46 @@ export function handleError( process.exit(exitCode); } +/** + * `status` and `body` are only populated when the API actually answered. + * Network, TLS, and non-JSON failures carry their detail in `message` alone, so + * these helpers coalesce so we display full details. + */ +function formatHttpError(error: HttpError): string { + const heading = formatHttpErrorHeading(error); + const body = formatHttpErrorBody(error.body); + return body === null ? `${heading}\n` : `${heading}\n\n${body}\n`; +} + +function formatHttpErrorHeading(error: HttpError): string { + if (error.status === undefined) { + return error.message; + } + + // A successful or redirect status that still failed the request — an + // unparseable body, or a redirect missing its Location — isn't an error the + // API reported, so don't describe it as one. + if (error.status < 400) { + return `${error.message} (HTTP ${error.status})`; + } + + return `The API returned an error with status code ${error.status}: ${error.message}`; +} + +function formatHttpErrorBody(body: unknown): string | null { + if (body === undefined || body === "") { + return null; + } + + // Non-JSON bodies (proxy block pages, HTML error pages) are far more legible + // raw than as a JSON string literal full of escaped newlines. + if (typeof body === "string") { + return body; + } + + return JSON.stringify(body, null, 2) ?? String(body); +} + function printJson(value: Record): void { process.stdout.write(JSON.stringify(value, null, 2) + "\n"); } diff --git a/src/commands/studio/query.ts b/src/commands/studio/query.ts index 7fdc3dc..3e4ae6c 100644 --- a/src/commands/studio/query.ts +++ b/src/commands/studio/query.ts @@ -10,7 +10,7 @@ import { wrapAction, } from "../../commandHelpers.js"; import { CliError, EXIT_CODES, HttpError } from "../../errors.js"; -import { request } from "../../http.js"; +import { describeFetchFailure, request } from "../../http.js"; import { AsyncProgressReporter, renderJson, @@ -351,7 +351,9 @@ async function fetchResponse( try { response = await fetch(url, init); } catch (error) { - throw new HttpError(`Request failed: ${(error as Error).message}`); + throw new HttpError( + `Could not reach ${url}: ${describeFetchFailure(error)}`, + ); } return response; diff --git a/src/http.test.ts b/src/http.test.ts index a797f97..3712975 100644 --- a/src/http.test.ts +++ b/src/http.test.ts @@ -1,6 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { parseRetryAfterMs, request } from "./http.js"; +import { describeFetchFailure, parseRetryAfterMs, request } from "./http.js"; +import { HttpError } from "./errors.js"; import { createLogger } from "./logger.js"; import type { Runtime } from "./types.js"; @@ -88,6 +89,82 @@ describe("request", () => { }); }); +describe("describeFetchFailure", () => { + it("surfaces the cause behind Node's generic 'fetch failed'", () => { + const cause = Object.assign( + new Error("unable to verify the first certificate"), + { code: "UNABLE_TO_VERIFY_LEAF_SIGNATURE" }, + ); + + expect(describeFetchFailure(new TypeError("fetch failed", { cause }))).toBe( + "unable to verify the first certificate (UNABLE_TO_VERIFY_LEAF_SIGNATURE)", + ); + }); + + it("keeps the top-level message when there is no cause", () => { + expect( + describeFetchFailure( + new TypeError("Failed to parse URL from api.example.com/auth.info"), + ), + ).toBe("Failed to parse URL from api.example.com/auth.info"); + expect(describeFetchFailure(new TypeError("fetch failed"))).toBe( + "fetch failed", + ); + }); + + it("handles non-Error rejections", () => { + expect(describeFetchFailure("boom")).toBe("boom"); + }); +}); + +describe("request error details", () => { + it("includes the fetch cause in the thrown message", async () => { + const cause = Object.assign(new Error("getaddrinfo ENOTFOUND"), { + code: "ENOTFOUND", + }); + vi.stubGlobal( + "fetch", + vi.fn().mockRejectedValue(new TypeError("fetch failed", { cause })), + ); + + await expect(request(runtime, "/auth.info")).rejects.toThrow( + "Could not reach https://api.example.com/auth.info: getaddrinfo ENOTFOUND (ENOTFOUND)", + ); + }); + + it("attaches the status and raw body when a 2xx response is not JSON", async () => { + vi.stubGlobal( + "fetch", + vi + .fn() + .mockResolvedValue( + new Response("Blocked by proxy", { status: 200 }), + ), + ); + + const error = await request(runtime, "/auth.info").catch((e) => e); + + expect(error).toBeInstanceOf(HttpError); + expect((error as HttpError).status).toBe(200); + expect((error as HttpError).body).toBe("Blocked by proxy"); + expect((error as HttpError).message).toContain( + "Invalid JSON response from https://api.example.com/auth.info", + ); + }); + + it("reports an empty 2xx body distinctly", async () => { + vi.stubGlobal( + "fetch", + vi.fn().mockResolvedValue(new Response("", { status: 200 })), + ); + + const error = await request(runtime, "/auth.info").catch((e) => e); + + expect((error as HttpError).status).toBe(200); + expect((error as HttpError).message).toContain("Empty response body"); + }); +}); + describe("http logging", () => { it("logs request and response details with redacted authorization", async () => { const writes: string[] = []; diff --git a/src/http.ts b/src/http.ts index f92a521..8c24a4c 100644 --- a/src/http.ts +++ b/src/http.ts @@ -63,7 +63,9 @@ export async function request>( body: requestBody, }); } catch (error) { - throw new HttpError(`Request failed: ${(error as Error).message}`); + throw new HttpError( + `Could not reach ${url}: ${describeFetchFailure(error)}`, + ); } const responseBodyText = await response.text(); @@ -88,7 +90,16 @@ export async function request>( typeof parsedResponseBody === "string" && parsedResponseBody === responseBodyText ) { - throw new HttpError("Invalid JSON response: Unexpected token"); + // A 2xx that isn't JSON usually means something other than the API answered + // e.g. a proxy block page, a captive portal, etc. Pass the status + // and the raw text along so the user can see what actually came back. + throw new HttpError( + responseBodyText + ? `Invalid JSON response from ${url}` + : `Empty response body from ${url}`, + response.status, + responseBodyText, + ); } const retryAfterMs = parseRetryAfterMs(response.headers); @@ -96,6 +107,37 @@ export async function request>( return retryAfterMs === undefined ? { body } : { body, retryAfterMs }; } +/** + * Flatten an error thrown by `fetch` into a single readable message. + * + * Node reports every DNS, proxy, and TLS failure as `TypeError: fetch failed` + * and hangs the real reason off `cause`, so the top-level message alone can't + * distinguish an untrusted corporate root certificate from an unreachable host. + */ +export function describeFetchFailure(error: unknown): string { + if (!(error instanceof Error)) { + return String(error); + } + + const parts: string[] = []; + let current: unknown = error; + while (current instanceof Error) { + const { code } = current as NodeJS.ErrnoException; + const part = code ? `${current.message} (${code})` : current.message; + if (!parts.includes(part)) { + parts.push(part); + } + current = current.cause; + } + + // "fetch failed" carries no information once a cause is available. + const informative = parts.filter( + (part) => parts.length === 1 || part !== "fetch failed", + ); + + return informative.join(": "); +} + export function parseRetryAfterMs(headers: Headers): number | undefined { const retryAfter = headers.get("retry-after"); if (!retryAfter) {