diff --git a/docs/contributing/api-patterns.md b/docs/contributing/api-patterns.md index e0db72ea5..34eee1a65 100644 --- a/docs/contributing/api-patterns.md +++ b/docs/contributing/api-patterns.md @@ -260,8 +260,11 @@ See error patterns in [common-patterns.md](common-patterns.md#error-handling). ### Automatic Retries -`SentryApiService.request()` transparently retries failed requests using the -shared policy in `@sentry/toolkit-core/api-retry`. This page describes the +`SentryApiService` sends every request, including `@sentry/api` operations, +through the shared transport in `@sentry/toolkit-core/api-transport`, which +retries failed requests using the shared policy in +`@sentry/toolkit-core/api-retry`. `SentryApiService` never retries on top, so +a request is retried by one layer only. This page describes the behavior; the values live only in code, so read them there instead of copying them here: @@ -288,8 +291,8 @@ Retries are: are not retried. A rate-limited tool call returns a **Rate Limited** message with the retry time, not an input error. -- **Bypassed by `allowStatuses`** — a status the caller opts into is returned - as-is from `requestOnce`, never retried. +- **Before `allowStatuses`** — a status the caller opts into is returned + as-is, after the transport has finished retrying it. Callers and tools need no changes: a retried request either resolves normally or throws the same error it would have thrown without retries. diff --git a/packages/mcp-cloudflare/src/server/lib/mcp-handler.test.ts b/packages/mcp-cloudflare/src/server/lib/mcp-handler.test.ts index 237e12bd4..cda732f28 100644 --- a/packages/mcp-cloudflare/src/server/lib/mcp-handler.test.ts +++ b/packages/mcp-cloudflare/src/server/lib/mcp-handler.test.ts @@ -634,13 +634,10 @@ describe("MCP Handler", () => { ]); expect(env.MCP_CACHE.put).not.toHaveBeenCalled(); expect(apiFetch).toHaveBeenCalledOnce(); - expect(apiFetch).toHaveBeenCalledWith( - profileUrl, - expect.objectContaining({ - headers: expect.objectContaining({ - Authorization: `Bearer ${ctx.props.accessToken}`, - }), - }), + const [url, init] = apiFetch.mock.calls[0] ?? []; + expect(url).toBe(profileUrl); + expect(new Headers(init?.headers).get("Authorization")).toBe( + `Bearer ${ctx.props.accessToken}`, ); } finally { apiFetch.mockRestore(); diff --git a/packages/mcp-core/src/api-client/client.test.ts b/packages/mcp-core/src/api-client/client.test.ts index 9e41f36fb..976f4914f 100644 --- a/packages/mcp-core/src/api-client/client.test.ts +++ b/packages/mcp-core/src/api-client/client.test.ts @@ -1503,7 +1503,7 @@ describe("transient 5xx retry", () => { expect(fetchMock).toHaveBeenCalledTimes(1); }); - it("does not retry a 5xx that the caller opted into via allowStatuses", async () => { + it("returns an allowed 5xx as-is once the transport stops retrying", async () => { const fetchMock = vi.fn().mockResolvedValue(serviceUnavailable()); globalThis.fetch = fetchMock; @@ -1513,8 +1513,8 @@ describe("transient 5xx retry", () => { }); // getEventAttachment passes allowStatuses so the caller can inspect the raw - // response; such requests must be returned as-is, never retried. - const response = await ( + // response. Retries happen in the transport, before the status is checked. + const pending = ( apiService as unknown as { request: ( path: string, @@ -1523,9 +1523,10 @@ describe("transient 5xx retry", () => { ) => Promise; } ).request("/auth/", undefined, { allowStatuses: [503] }); + await vi.runAllTimersAsync(); - expect(response.status).toBe(503); - expect(fetchMock).toHaveBeenCalledTimes(1); + expect((await pending).status).toBe(503); + expect(fetchMock).toHaveBeenCalledTimes(3); }); }); @@ -1780,6 +1781,20 @@ describe("rate limit retry", () => { } }); + it("sends a still-limited GET at most maxRetries + 1 times", async () => { + const fetchMock = vi + .fn() + .mockImplementation(async () => rateLimited(nowSeconds + 1)); + globalThis.fetch = fetchMock; + + const promise = apiService().getAuthenticatedUser(); + promise.catch(() => {}); + await vi.runAllTimersAsync(); + + await expect(promise).rejects.toBeInstanceOf(ApiRateLimitError); + expect(fetchMock).toHaveBeenCalledTimes(3); + }); + it("stops waiting when the request is aborted", async () => { const fetchMock = vi.fn().mockResolvedValue(rateLimited(nowSeconds + 1)); globalThis.fetch = fetchMock; @@ -1796,6 +1811,30 @@ describe("rate limit retry", () => { await expect(promise).rejects.toBe(reason); expect(fetchMock).toHaveBeenCalledTimes(1); }); + + it("keeps the caller's abort reason for an in-flight request", async () => { + const fetchMock = vi.fn( + (_input: Request | string | URL, init?: RequestInit) => + new Promise((_, reject) => { + init?.signal?.addEventListener("abort", () => + reject(init.signal?.reason), + ); + }), + ); + globalThis.fetch = fetchMock; + const controller = new AbortController(); + const reason = new Error("tool call cancelled"); + + const promise = rawRequest(apiService())("/auth/", { + signal: controller.signal, + }); + promise.catch(() => {}); + await vi.waitFor(() => expect(fetchMock).toHaveBeenCalledOnce()); + controller.abort(reason); + + await expect(promise).rejects.toBe(reason); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); }); describe("request headers", () => { @@ -1837,12 +1876,18 @@ describe("request headers", () => { const [, requestInit] = (globalThis.fetch as ReturnType).mock .calls[0]; - expect(requestInit.headers["X-Sentry-MCP-Client-Id"]).toBe("abc123"); - expect(requestInit.headers["X-Sentry-MCP-Client-Name"]).toBe("Claude Code"); - expect(requestInit.headers["X-Sentry-MCP-Client-Family"]).toBe( - "claude-code", + expect(new Headers(requestInit.headers).get("X-Sentry-MCP-Client-Id")).toBe( + "abc123", ); - expect(requestInit.headers["X-Sentry-MCP-Utm-Source"]).toBe("plugin"); + expect( + new Headers(requestInit.headers).get("X-Sentry-MCP-Client-Name"), + ).toBe("Claude Code"); + expect( + new Headers(requestInit.headers).get("X-Sentry-MCP-Client-Family"), + ).toBe("claude-code"); + expect( + new Headers(requestInit.headers).get("X-Sentry-MCP-Utm-Source"), + ).toBe("plugin"); }); it("should not send MCP client headers when clientId and clientName are not set", async () => { @@ -1869,10 +1914,18 @@ describe("request headers", () => { const [, requestInit] = (globalThis.fetch as ReturnType).mock .calls[0]; - expect(requestInit.headers["X-Sentry-MCP-Client-Id"]).toBeUndefined(); - expect(requestInit.headers["X-Sentry-MCP-Client-Name"]).toBeUndefined(); - expect(requestInit.headers["X-Sentry-MCP-Client-Family"]).toBeUndefined(); - expect(requestInit.headers["X-Sentry-MCP-Utm-Source"]).toBeUndefined(); + expect( + new Headers(requestInit.headers).get("X-Sentry-MCP-Client-Id"), + ).toBeNull(); + expect( + new Headers(requestInit.headers).get("X-Sentry-MCP-Client-Name"), + ).toBeNull(); + expect( + new Headers(requestInit.headers).get("X-Sentry-MCP-Client-Family"), + ).toBeNull(); + expect( + new Headers(requestInit.headers).get("X-Sentry-MCP-Utm-Source"), + ).toBeNull(); }); }); @@ -2526,6 +2579,7 @@ describe("API query builders", () => { // Mock the API response globalThis.fetch = vi.fn().mockResolvedValue({ ok: true, + status: 200, headers: { get: (key: string) => key === "content-type" ? "application/json" : null, @@ -2565,6 +2619,7 @@ describe("API query builders", () => { // Mock the API response globalThis.fetch = vi.fn().mockResolvedValue({ ok: true, + status: 200, headers: { get: (key: string) => key === "content-type" ? "application/json" : null, @@ -2602,6 +2657,7 @@ describe("API query builders", () => { globalThis.fetch = vi.fn().mockResolvedValue({ ok: true, + status: 200, headers: { get: (key: string) => key === "content-type" ? "application/json" : null, @@ -2640,6 +2696,7 @@ describe("API query builders", () => { globalThis.fetch = vi.fn().mockResolvedValue({ ok: true, + status: 200, headers: { get: (key: string) => key === "content-type" ? "application/json" : null, @@ -2678,6 +2735,7 @@ describe("API query builders", () => { return Promise.resolve({ ok: true, + status: 200, headers: { get: (key: string) => key === "content-type" ? "application/json" : null, @@ -2773,6 +2831,7 @@ describe("API query builders", () => { return Promise.resolve({ ok: true, + status: 200, headers: { get: (key: string) => key === "content-type" ? "application/json" : null, @@ -3200,6 +3259,7 @@ describe("API query builders", () => { it("should call the repos endpoint without query", async () => { globalThis.fetch = vi.fn().mockResolvedValue({ ok: true, + status: 200, headers: { get: (key: string) => key === "content-type" ? "application/json" : null, @@ -3236,6 +3296,7 @@ describe("API query builders", () => { .fn() .mockResolvedValueOnce({ ok: true, + status: 200, headers: new Headers({ "content-type": "application/json", link: '; rel="next"; results="true"; cursor="cursor-2"', @@ -3252,6 +3313,7 @@ describe("API query builders", () => { }) .mockResolvedValueOnce({ ok: true, + status: 200, headers: new Headers({ "content-type": "application/json", }), @@ -3283,6 +3345,7 @@ describe("API query builders", () => { it("should include query parameter when provided", async () => { globalThis.fetch = vi.fn().mockResolvedValue({ ok: true, + status: 200, headers: { get: (key: string) => key === "content-type" ? "application/json" : null, @@ -3312,6 +3375,7 @@ describe("API query builders", () => { it("should return empty array when no repos exist", async () => { globalThis.fetch = vi.fn().mockResolvedValue({ ok: true, + status: 200, headers: { get: (key: string) => key === "content-type" ? "application/json" : null, @@ -3329,6 +3393,7 @@ describe("API query builders", () => { it("should accept repos with null externalSlug and externalId", async () => { globalThis.fetch = vi.fn().mockResolvedValue({ ok: true, + status: 200, headers: { get: (key: string) => key === "content-type" ? "application/json" : null, @@ -3373,6 +3438,7 @@ describe("API query builders", () => { it("should POST to the code mappings bulk endpoint", async () => { globalThis.fetch = vi.fn().mockResolvedValue({ ok: true, + status: 200, headers: { get: (key: string) => key === "content-type" ? "application/json" : null, @@ -3423,6 +3489,7 @@ describe("API query builders", () => { it("should handle updated mappings response", async () => { globalThis.fetch = vi.fn().mockResolvedValue({ ok: true, + status: 200, headers: { get: (key: string) => key === "content-type" ? "application/json" : null, @@ -3481,6 +3548,7 @@ describe("API query builders", () => { return Promise.resolve({ ok: true, + status: 200, headers: { get: (key: string) => { if (key === "content-type") { @@ -3526,6 +3594,7 @@ describe("API query builders", () => { requestUrl = url; return Promise.resolve({ ok: true, + status: 200, headers: { get: (key: string) => key === "content-type" ? "application/json" : null, @@ -3552,6 +3621,7 @@ describe("API query builders", () => { requestUrl = url; return Promise.resolve({ ok: true, + status: 200, headers: { get: (key: string) => key === "content-type" ? "application/json" : null, @@ -3578,6 +3648,7 @@ describe("API query builders", () => { requestUrl = url; return Promise.resolve({ ok: true, + status: 200, headers: { get: (key: string) => key === "content-type" ? "application/json" : null, diff --git a/packages/mcp-core/src/api-client/client.ts b/packages/mcp-core/src/api-client/client.ts index 0c2579b2f..b099a9d7b 100644 --- a/packages/mcp-core/src/api-client/client.ts +++ b/packages/mcp-core/src/api-client/client.ts @@ -22,11 +22,10 @@ import { import { buildSentryApiUrl } from "@sentry/toolkit-core/api-request"; import { sentryBearerHeader } from "@sentry/toolkit-core/auth-token"; import { - nextRetryDelayMs, parseRetryDelayMs, type RetryPolicy, - sleep, } from "@sentry/toolkit-core/api-retry"; +import { createApiTransport } from "@sentry/toolkit-core/api-transport"; import { z } from "zod"; import { DEFAULT_SEARCH_ISSUES_PERIOD } from "../constants"; import { ConfigurationError } from "../errors"; @@ -332,11 +331,11 @@ const REQUEST_RETRY_POLICY: RetryPolicy = { }; /** - * Errors thrown because `fetch` itself failed (DNS, connection refused or - * reset, socket timeout), as opposed to configuration checks before the - * request. The shared policy retries these for idempotent requests only. + * Every request goes through the shared transport. `SentryApiService` sets its + * own headers, including Authorization, so the transport only retries; as + * before, it adds no timeout. */ -const fetchFailures = new WeakSet(); +const transport = createApiTransport({ retry: REQUEST_RETRY_POLICY }); const API_MAX_PER_PAGE = 100; const MAX_LIST_PAGES = 50; @@ -787,64 +786,7 @@ export class SentryApiService { } /** - * Makes an authenticated request to the Sentry API, transparently retrying - * rate-limited (429) requests, and GET and HEAD requests that hit a - * 500/502/503/504 or a network failure, under `REQUEST_RETRY_POLICY`. - * - * A 429 waits for its `Retry-After` or `X-Sentry-Rate-Limit-Reset` delay; - * everything else uses jittered backoff. Writes that hit a 5xx or a network - * failure, other 4xx errors, and configuration checks before the request - * fail fast. Callers that opt - * a status into `allowStatuses` receive the raw response from `requestOnce` - * and are never retried. Aborting `options.signal` ends any wait. - * - * @param path API endpoint path (without /api/0 prefix) - * @param options Fetch options - * @param requestOptions Additional request configuration - * @returns Promise resolving to Response object - * @throws {ApiError} Enhanced API errors with user-friendly messages - * @throws {Error} Network or parsing errors - */ - private async request( - path: string, - options: RequestInit = {}, - requestOptions: { host?: string; allowStatuses?: number[] } = {}, - ): Promise { - const method = options.method ?? "GET"; - let waitedMs = 0; - - for (let attempt = 0; ; attempt++) { - try { - return await this.requestOnce(path, options, requestOptions); - } catch (error) { - const status = error instanceof ApiError ? error.status : undefined; - const networkFailure = - error instanceof Error && - fetchFailures.has(error) && - !options.signal?.aborted; - if (status === undefined && !networkFailure) { - throw error; - } - const delayMs = nextRetryDelayMs({ - policy: REQUEST_RETRY_POLICY, - method, - status, - serverDelayMs: - error instanceof ApiRateLimitError ? error.retryAfterMs : undefined, - attempt, - waitedMs, - }); - if (delayMs === undefined) { - throw error; - } - await sleep(delayMs, options.signal); - waitedMs += delayMs; - } - } - } - - /** - * Internal method for making authenticated requests to Sentry API. + * Makes an authenticated request to the Sentry API through the shared transport. * * Handles: * - Bearer token authentication @@ -852,6 +794,13 @@ export class SentryApiService { * - Multi-region host overrides * - Fetch availability validation * + * Retries happen in the transport, never here, so they do not stack: a + * rate-limited (429) request, or a GET that hits a 500/502/503/504 or a + * network failure, is retried under `REQUEST_RETRY_POLICY`, and the final + * failure is thrown here, with + * `ApiRateLimitError.retryAfterMs` from its headers. + * Callers that opt a status into `allowStatuses` receive the raw response. + * * @param path API endpoint path (without /api/0 prefix) * @param options Fetch options * @param requestOptions Additional request configuration @@ -859,7 +808,7 @@ export class SentryApiService { * @throws {ApiError} Enhanced API errors with user-friendly messages * @throws {Error} Network or parsing errors */ - private async requestOnce( + private async request( path: string, options: RequestInit = {}, { host, allowStatuses }: { host?: string; allowStatuses?: number[] } = {}, @@ -903,11 +852,15 @@ export class SentryApiService { // logger.info(logger.fmt`[sentryApi] ${options.method || "GET"} ${url}`); let response: Response; try { - response = await fetch(url, { + response = await transport.request(url, { ...options, headers, }); } catch (error) { + // An abort, including one during a retry wait, keeps the caller's reason. + if (options.signal?.aborted) { + throw error; + } // Extract the root cause from the error chain let rootCause = error; while (rootCause instanceof Error && rootCause.cause) { @@ -938,16 +891,10 @@ export class SentryApiService { errorCode === "ETIMEDOUT" || errorMessage.includes("Connect Timeout Error") ) { - const configError = new ConfigurationError(friendlyMessage, { - cause: error, - }); - fetchFailures.add(configError); - throw configError; + throw new ConfigurationError(friendlyMessage, { cause: error }); } - const networkError = new Error(friendlyMessage, { cause: error }); - fetchFailures.add(networkError); - throw networkError; + throw new Error(friendlyMessage, { cause: error }); } // Handle error responses generically diff --git a/packages/toolkit-core/README.md b/packages/toolkit-core/README.md index 5b2ce5bb9..f3f94b631 100644 --- a/packages/toolkit-core/README.md +++ b/packages/toolkit-core/README.md @@ -37,6 +37,11 @@ fatal fetch errors such as TLS failures, a logger, and a span hook. The CLI's `packages/cli/src/lib/sentry-client.ts` wires in its stored credentials, host-scope checks, disk cache, and telemetry. +Credentials and timeouts are optional. Without a `CredentialProvider`, the +transport keeps the request's own Authorization header, never refreshes, and +skips the cache, since entries are scoped to a credential. The MCP's +`SentryApiService` uses it that way, with retries only. + ## Pagination `pagination` walks Sentry list endpoints up to a row limit. `paginate` splits diff --git a/packages/toolkit-core/src/api-transport.test.ts b/packages/toolkit-core/src/api-transport.test.ts index 1492ff57d..471bce4e3 100644 --- a/packages/toolkit-core/src/api-transport.test.ts +++ b/packages/toolkit-core/src/api-transport.test.ts @@ -331,6 +331,29 @@ describe("timeouts and fetch errors", () => { ); }); + it("rejects an in-flight request with the caller's abort reason", async () => { + // Like fetch, reject with the signal's reason once it aborts. + const fetch = vi.fn( + (_input: Request | string | URL, init?: RequestInit) => + new Promise((_, reject) => { + init?.signal?.addEventListener("abort", () => + reject(init.signal?.reason), + ); + }), + ); + const controller = new AbortController(); + const reason = new Error("tool call cancelled"); + + const pending = transport(fetch) + .request(ISSUES_URL, { signal: controller.signal }) + .catch((error: unknown) => error); + await vi.waitFor(() => expect(fetch).toHaveBeenCalledOnce()); + controller.abort(reason); + + expect(await pending).toBe(reason); + expect(fetch).toHaveBeenCalledOnce(); + }); + it("throws the host timeout error for a POST without retrying", async () => { const fetch = vi.fn(hang); @@ -406,6 +429,17 @@ describe("headers", () => { "user-agent": "test-agent", }); }); + + it("leaves Authorization to the request without credentials", async () => { + const { fetch, sent } = scriptedFetch([Response.json({})]); + const api = createApiTransport({ retry: policy, fetch }); + + await api.request(ISSUES_URL, { + headers: { Authorization: "Bearer from-request" }, + }); + + expect(sent[0]?.headers.get("Authorization")).toBe("Bearer from-request"); + }); }); describe("validated redirects", () => { @@ -567,6 +601,24 @@ describe("cache", () => { expect(sent).toHaveLength(2); }); + it("is not consulted without credentials", async () => { + const lookup = vi.fn(); + const store = vi.fn(); + const { fetch, sent } = scriptedFetch([Response.json({})]); + const api = createApiTransport({ + retry: policy, + fetch, + cache: { lookup, store, invalidate: vi.fn() }, + }); + + await (await api.request(ISSUES_URL)).json(); + await (await api.request(ISSUES_URL)).json(); + + expect(sent).toHaveLength(2); + expect(lookup).not.toHaveBeenCalled(); + expect(store).not.toHaveBeenCalled(); + }); + it("revalidates a stale entry with its ETag", async () => { const response = await transport( scriptedFetch([new Response(null, { status: 304 })]).fetch, diff --git a/packages/toolkit-core/src/api-transport.ts b/packages/toolkit-core/src/api-transport.ts index 5c0ac1388..6be6f5171 100644 --- a/packages/toolkit-core/src/api-transport.ts +++ b/packages/toolkit-core/src/api-transport.ts @@ -64,10 +64,15 @@ type TransportLogger = { }; export type ApiTransportOptions = { - credentials: CredentialProvider; + /** + * Without credentials the transport leaves Authorization to the request, + * does not refresh, and does not cache, since entries are scoped to the + * credential's identity. + */ + credentials?: CredentialProvider; retry: RetryPolicy; - /** Per-attempt timeout, or a function of the request URL. */ - timeoutMs: number | ((url: string) => number); + /** Per-attempt timeout, or a function of the request URL. Default: none. */ + timeoutMs?: number | ((url: string) => number); /** Underlying fetch, read on each call. Defaults to `globalThis.fetch`. */ fetch?: (input: FetchInput, init?: RequestInit) => Promise; /** Sent when the request sets no `User-Agent`. */ @@ -164,6 +169,9 @@ export class ApiTimeoutError extends Error { const MAX_REDIRECTS = 20; +/** Stands in for the credential when the host sets Authorization itself. */ +const ANONYMOUS: TransportCredential = { token: "", identity: "" }; + /** Where a response fetched with `validateRedirects` came from. */ export type ValidatedResponseSource = { origin: string; identity: string }; @@ -202,8 +210,10 @@ class Rejection { constructor(readonly error: unknown) {} } -/** One attempt: a response, or a thrown error and whether our timeout caused it. */ -type Outcome = { response: Response } | { failed: unknown; timedOut: boolean }; +/** One attempt: a response, or a thrown error and our timeout if it caused it. */ +type Outcome = + | { response: Response } + | { failed: unknown; timedOutAfterMs?: number }; type AttemptResult = | { action: "done"; response: Response } @@ -226,12 +236,16 @@ type Send = { export function createApiTransport( config: ApiTransportOptions, ): ApiTransport { - const { credentials, retry, cache, logger } = config; + const { credentials, retry, logger } = config; + const cache = credentials && config.cache; const baseFetch = (input: FetchInput, init?: RequestInit) => (config.fetch ?? globalThis.fetch)(input, init); - function authorization(credential: C): string { + function authorization(credential: C): string | undefined { + if (!credentials) { + return; + } if (credentials.authorization) { return credentials.authorization(credential); } @@ -242,10 +256,10 @@ export function createApiTransport( return header; } - function timeoutFor(url: string): number { - return typeof config.timeoutMs === "number" - ? config.timeoutMs - : config.timeoutMs(url); + function timeoutFor(url: string): number | undefined { + return typeof config.timeoutMs === "function" + ? config.timeoutMs(url) + : config.timeoutMs; } /** @@ -263,7 +277,7 @@ export function createApiTransport( init?.headers ?? (input instanceof Request ? input.headers : undefined), ); config.prepareHeaders?.(headers, { input, credential }); - headers.set("Authorization", authorization(credential)); + setAuthorization(headers, authorization(credential)); if (config.userAgent && !headers.has("User-Agent")) { headers.set("User-Agent", config.userAgent); } @@ -292,18 +306,22 @@ export function createApiTransport( input: FetchInput, init: RequestInit, signal: AbortSignal | null | undefined, - timeoutMs: number, + timeoutMs: number | undefined, mode: ResponseBodyMode, ): Promise { const controller = new AbortController(); let timedOut = false; - const timer = setTimeout(() => { - timedOut = true; - controller.abort(); - }, timeoutMs); - const onAbort = () => controller.abort(); + const timer = + timeoutMs === undefined + ? undefined + : setTimeout(() => { + timedOut = true; + controller.abort(); + }, timeoutMs); + // Forward the caller's reason, so an aborted fetch rejects with it. + const onAbort = () => controller.abort(signal?.reason); if (signal?.aborted) { - controller.abort(); + onAbort(); } else { signal?.addEventListener("abort", onAbort, { once: true }); } @@ -318,7 +336,7 @@ export function createApiTransport( return { response }; } catch (error) { if (timedOut) { - return { failed: error, timedOut: true }; + return { failed: error, timedOutAfterMs: timeoutMs }; } throw error; } finally { @@ -386,7 +404,7 @@ export function createApiTransport( init: RequestInit | undefined, headers: Headers, credential: C, - timeoutMs: number, + timeoutMs: number | undefined, mode: ResponseBodyMode, ): Promise { const original = new Request(input, init); @@ -438,7 +456,7 @@ export function createApiTransport( /** Send with refresh, timeouts, and retries; the cache is handled by the caller. */ async function send(sendArgs: Send): Promise { const { input, init, method, url, path, options } = sendArgs; - let credential = credentials.prepare + let credential = credentials?.prepare ? await credentials.prepare(sendArgs.credential) : sendArgs.credential; const headers = buildHeaders(input, init, credential); @@ -483,7 +501,7 @@ export function createApiTransport( ): Promise { if (response.status === 401 && attempt < retry.maxRetries) { const next = - refreshed || !credentials.refresh + refreshed || !credentials?.refresh ? undefined : await credentials.refresh(credential); if (!next) { @@ -491,7 +509,7 @@ export function createApiTransport( } refreshed = true; credential = next; - headers.set("Authorization", authorization(next)); + setAuthorization(headers, authorization(next)); if (config.refreshedRequestHeader) { headers.set(config.refreshedRequestHeader, "1"); } @@ -521,7 +539,7 @@ export function createApiTransport( } function onFailure( - { failed, timedOut }: Extract, + { failed, timedOutAfterMs }: Extract, attempt: number, ): AttemptResult { if (failed instanceof Rejection) { @@ -538,7 +556,7 @@ export function createApiTransport( if (delayMs !== undefined) { return { action: "retry", delayMs }; } - if (!timedOut) { + if (timedOutAfterMs === undefined) { return { action: "throw", error: failed }; } // Whether the endpoint may have run, for the host's error wording. @@ -546,8 +564,10 @@ export function createApiTransport( return { action: "throw", error: - config.errors?.timeout?.({ timeoutMs, idempotent: replayable }) ?? - new ApiTimeoutError(timeoutMs, replayable), + config.errors?.timeout?.({ + timeoutMs: timedOutAfterMs, + idempotent: replayable, + }) ?? new ApiTimeoutError(timedOutAfterMs, replayable), }; } @@ -572,7 +592,7 @@ export function createApiTransport( mode, ); } catch (error) { - outcome = { failed: error, timedOut: false }; + outcome = { failed: error }; } // Outside the try: handling a response is not a fetch failure. const result = @@ -665,7 +685,9 @@ export function createApiTransport( const run = async (): Promise => { const startTime = performance.now(); const elapsed = () => (performance.now() - startTime).toFixed(0); - const credential = await credentials.current(); + const credential = credentials + ? await credentials.current() + : (ANONYMOUS as C); // A cached response has no validated origin, so discovery always // goes to the network. const lookup = @@ -715,11 +737,12 @@ export function createApiTransport( return { request }; } +/** Whether `error` is the caller's abort, with the default or its own reason. */ function isAbort(error: unknown, signal: AbortSignal | null | undefined) { return ( - error instanceof DOMException && - error.name === "AbortError" && - Boolean(signal?.aborted) + Boolean(signal?.aborted) && + (error === signal?.reason || + (error instanceof DOMException && error.name === "AbortError")) ); } @@ -758,6 +781,12 @@ async function replayableInput( return () => ({ input, init }); } +function setAuthorization(headers: Headers, value: string | undefined): void { + if (value !== undefined) { + headers.set("Authorization", value); + } +} + /** The origin of a credential-free HTTP(S) URL, or undefined. */ function httpOrigin(raw: string): string | undefined { const url = new URL(raw);