diff --git a/packages/cli/src/lib/sentry-client.ts b/packages/cli/src/lib/sentry-client.ts index a35c46646..033b4da86 100644 --- a/packages/cli/src/lib/sentry-client.ts +++ b/packages/cli/src/lib/sentry-client.ts @@ -75,6 +75,14 @@ const ENDPOINT_TIMEOUT_OVERRIDES: TimeoutOverride[] = [ /** Maximum retry attempts for failed requests */ const MAX_RETRIES = 2; +/** Per-request controls for mutations with external effects and fresh preflights. */ +export type SentryRequestOptions = { + /** False sends exactly one request, without HTTP, network, timeout, or 401 replay. */ + retry?: boolean; + /** Bypass both reads and writes of the local response cache. */ + cache?: "no-store"; +}; + /** Maximum backoff delay between retries in milliseconds */ const MAX_BACKOFF_MS = 10_000; @@ -315,13 +323,15 @@ type AttemptResult = /** * Decide what to do with a successful HTTP response. * Returns 'done' for final responses, 'retry' for retryable errors and 401s. + * The last attempt always returns 'done': a refreshed token there would have + * no attempt left to use it. */ async function handleResponse( response: Response, headers: Headers, isLastAttempt: boolean ): Promise { - if (response.status === 401) { + if (response.status === 401 && !isLastAttempt) { const refreshed = await handleUnauthorized(headers); return refreshed ? { action: "retry" } : { action: "done", response }; } @@ -527,16 +537,17 @@ async function buildAttemptFactory( async function fetchWithRetry( input: Request | string | URL, init: RequestInit | undefined, - method: string, - fullUrl: string + request: { method: string; fullUrl: string; options: SentryRequestOptions } ): Promise { + const { method, fullUrl, options } = request; const { token } = await refreshToken(); const headers = prepareHeaders(input, init, token); const attemptFactory = await buildAttemptFactory(input, init); const timeoutMs = resolveTimeoutMs(fullUrl); + const maxRetries = options.retry === false ? 0 : MAX_RETRIES; - for (let attempt = 0; attempt <= MAX_RETRIES; attempt++) { - const isLastAttempt = attempt === MAX_RETRIES; + for (let attempt = 0; attempt <= maxRetries; attempt++) { + const isLastAttempt = attempt === maxRetries; const { input: attemptInput, init: attemptInit } = attemptFactory(); const result = await executeAttempt({ input: attemptInput, @@ -549,12 +560,14 @@ async function fetchWithRetry( if (result.action === "done") { // Use getAuthToken() instead of captured `token` — after a 401 refresh, // handleUnauthorized stores a new token in the DB - cacheResponse( - method, - fullUrl, - authHeaders(getAuthToken()), - result.response - ); + if (options.cache !== "no-store") { + cacheResponse( + method, + fullUrl, + authHeaders(getAuthToken()), + result.response + ); + } await invalidateAfterMutation(method, fullUrl, result.response); return result.response; } @@ -564,7 +577,7 @@ async function fetchWithRetry( const delay = backoffDelay(attempt); log.debug( - `${method} ${new URL(fullUrl).pathname} → retry ${attempt + 1}/${MAX_RETRIES} after ${delay}ms` + `${method} ${new URL(fullUrl).pathname} → retry ${attempt + 1}/${maxRetries} after ${delay}ms` ); await sleepMs(delay); } @@ -592,10 +605,9 @@ async function fetchWithRetry( * * @returns A fetch-compatible function for use with @sentry/api SDK functions */ -function createAuthenticatedFetch(): ( - input: Request | string | URL, - init?: RequestInit -) => Promise { +function createAuthenticatedFetch( + options: SentryRequestOptions = {} +): (input: Request | string | URL, init?: RequestInit) => Promise { return function authenticatedFetch( input: Request | string | URL, init?: RequestInit @@ -624,11 +636,10 @@ function createAuthenticatedFetch(): ( // Check cache before auth/retry for GET requests. // Uses current token (no refresh) so lookups are fast but Vary-correct. - const cached = await tryCacheHit( - method, - fullUrl, - authHeaders(getAuthToken()) - ); + const cached = + options.cache === "no-store" + ? undefined + : await tryCacheHit(method, fullUrl, authHeaders(getAuthToken())); if (cached) { span.setAttribute("http.response.status_code", cached.status); log.debug( @@ -637,7 +648,14 @@ function createAuthenticatedFetch(): ( return cached; } - const response = await fetchWithRetry(input, init, method, fullUrl); + const requestInit = options.cache + ? { ...init, cache: options.cache } + : init; + const response = await fetchWithRetry(input, requestInit, { + method, + fullUrl, + options, + }); span.setAttribute("http.response.status_code", response.status); if (!response.ok) { span.setStatus({ code: 2, message: `${response.status}` }); @@ -726,6 +744,7 @@ export function getControlSiloUrl(): string { * - `throwOnError`: Always false (we handle errors ourselves) * * @param regionUrl - The base URL for the target region (e.g., https://us.sentry.io) + * @param options - Per-request retry and cache controls; omit for the shared fetch * @returns Configuration object to spread into SDK function options * * @example @@ -734,7 +753,10 @@ export function getControlSiloUrl(): string { * const result = await listOrganizations({ ...config }); * ``` */ -export function getSdkConfig(regionUrl: string) { +export function getSdkConfig( + regionUrl: string, + options: SentryRequestOptions = {} +) { const normalizedBase = regionUrl.endsWith("/") ? regionUrl.slice(0, -1) : regionUrl; @@ -743,7 +765,10 @@ export function getSdkConfig(regionUrl: string) { // SDK functions already include /api/0/ in their URL paths, // so baseUrl should be the plain region URL without /api/0. baseUrl: normalizedBase, - fetch: getAuthenticatedFetch(), + fetch: + options.retry !== undefined || options.cache !== undefined + ? (createAuthenticatedFetch(options) as typeof fetch) + : getAuthenticatedFetch(), throwOnError: false as const, }; } diff --git a/packages/cli/test/lib/sentry-client.test.ts b/packages/cli/test/lib/sentry-client.test.ts index 2caaf9e33..7ca2c1f77 100644 --- a/packages/cli/test/lib/sentry-client.test.ts +++ b/packages/cli/test/lib/sentry-client.test.ts @@ -6,13 +6,22 @@ import { afterEach, beforeEach, describe, expect, test } from "vitest"; import { setAuthToken } from "../../src/lib/db/auth.js"; import { TimeoutError } from "../../src/lib/errors.js"; +import { + getCachedResponse, + storeCachedResponse, +} from "../../src/lib/response-cache.js"; import { __injectTimeoutOverrideForTests, __resolveRequestTimeoutMsForTests, getSdkConfig, resetAuthenticatedFetch, } from "../../src/lib/sentry-client.js"; -import { mockFetch, useTestConfigDir } from "../helpers.js"; +import { + extractFetchUrl, + mockFetch, + useEnvSandbox, + useTestConfigDir, +} from "../helpers.js"; useTestConfigDir("sentry-client-"); @@ -35,6 +44,119 @@ function getAuthenticatedFetch(): typeof fetch { return getSdkConfig(REGION_URL).fetch as typeof fetch; } +describe("401 replay", () => { + useEnvSandbox(["SENTRY_CLIENT_ID"]); + + /** Answer resource requests with `statuses` in order while OAuth refresh succeeds. */ + async function mockRefreshableSession(statuses: number[]): Promise { + process.env.SENTRY_CLIENT_ID = "synthetic-client-id"; + await setAuthToken("stored-token", 3600, "synthetic-refresh-token"); + const urls: string[] = []; + globalThis.fetch = mockFetch(async (input) => { + const url = extractFetchUrl(input); + urls.push(url); + if (url.endsWith("/oauth/token/")) { + return Response.json({ + access_token: "refreshed-token", + token_type: "bearer", + expires_in: 3600, + }); + } + return new Response("{}", { status: statuses.shift() ?? 200 }); + }); + return urls; + } + + test("returns a 401 from the final attempt instead of replaying it", async () => { + const urls = await mockRefreshableSession([503, 503, 401]); + const response = await getAuthenticatedFetch()( + `${REGION_URL}/api/0/organizations/` + ); + expect(response.status).toBe(401); + expect(urls.filter((url) => url.endsWith("/oauth/token/"))).toEqual([]); + }); + + test("retry false returns a 401 without refreshing the token", async () => { + const urls = await mockRefreshableSession([401]); + const url = `${REGION_URL}/api/0/issue-link/`; + const fetchOnce = getSdkConfig(REGION_URL, { retry: false }).fetch; + const response = await fetchOnce(url, { method: "POST" }); + expect(response.status).toBe(401); + expect(urls).toEqual([url]); + }); +}); + +describe("per-request transport controls", () => { + test("retry false sends a mutation once for transient HTTP and network failures", async () => { + let callCount = 0; + globalThis.fetch = mockFetch(async () => { + callCount += 1; + return new Response("provider unavailable", { status: 503 }); + }); + const fetchOnce = getSdkConfig(REGION_URL, { retry: false }).fetch; + expect( + (await fetchOnce(`${REGION_URL}/api/0/issue-link/`, { method: "POST" })) + .status + ).toBe(503); + expect(callCount).toBe(1); + + const networkError = new TypeError( + "connection reset after request body sent" + ); + globalThis.fetch = mockFetch(async () => { + callCount += 1; + throw networkError; + }); + await expect( + fetchOnce(`${REGION_URL}/api/0/issue-link/`, { method: "POST" }) + ).rejects.toBe(networkError); + expect(callCount).toBe(2); + }); + + test("no-store preflight bypasses both cache lookup and cache storage", async () => { + const url = `${REGION_URL}/api/0/organizations/example/issues/1/external-issues/`; + const headers = { Authorization: "Bearer test-token" }; + await storeCachedResponse( + "GET", + url, + headers, + Response.json( + { source: "cached" }, + { + headers: { + "Cache-Control": "max-age=600", + Date: new Date().toUTCString(), + }, + } + ) + ); + expect( + await (await getCachedResponse("GET", url, headers))?.json() + ).toEqual({ source: "cached" }); + let callCount = 0; + globalThis.fetch = mockFetch(async (_input, init) => { + callCount += 1; + expect(init?.cache).toBe("no-store"); + return Response.json( + { source: "fresh" }, + { + headers: { + "Cache-Control": "max-age=600", + Date: new Date().toUTCString(), + }, + } + ); + }); + const freshFetch = getSdkConfig(REGION_URL, { cache: "no-store" }).fetch; + expect(await (await freshFetch(url)).json()).toEqual({ source: "fresh" }); + expect(await (await freshFetch(url)).json()).toEqual({ source: "fresh" }); + expect(callCount).toBe(2); + expect( + await (await getCachedResponse("GET", url, headers))?.json() + ).toEqual({ source: "cached" }); + }); +}); + describe("fetchWithRetry / buildAttemptFactory", () => { test("retries a POST with a string body without re-consuming the body", async () => { const marker = "__test_string_body__";