From bd9f6347bc26fa38ddc1d59c133530a95033e6dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20Beteg=C3=B3n?= Date: Tue, 29 Sep 2026 08:37:42 +0200 Subject: [PATCH 1/2] fix(api): return a final-attempt 401 instead of an internal error A 401 on the last retry still refreshed the token and asked for another attempt, so the loop ended with "Exhausted all retry attempts" instead of the 401 and its auth guidance. Co-authored-by: Cursor --- packages/cli/src/lib/sentry-client.ts | 4 ++- packages/cli/test/lib/sentry-client.test.ts | 40 ++++++++++++++++++++- 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/lib/sentry-client.ts b/packages/cli/src/lib/sentry-client.ts index a35c466466..c1bd02724d 100644 --- a/packages/cli/src/lib/sentry-client.ts +++ b/packages/cli/src/lib/sentry-client.ts @@ -315,13 +315,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 }; } diff --git a/packages/cli/test/lib/sentry-client.test.ts b/packages/cli/test/lib/sentry-client.test.ts index 2caaf9e330..b9ee298c42 100644 --- a/packages/cli/test/lib/sentry-client.test.ts +++ b/packages/cli/test/lib/sentry-client.test.ts @@ -12,7 +12,12 @@ import { 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 +40,39 @@ 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([]); + }); +}); + describe("fetchWithRetry / buildAttemptFactory", () => { test("retries a POST with a string body without re-consuming the body", async () => { const marker = "__test_string_body__"; From 77e4bda9c118c5b52127b105215f025f6ba01cab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20Beteg=C3=B3n?= Date: Tue, 29 Sep 2026 08:39:21 +0200 Subject: [PATCH 2/2] feat(api): add per-request retry and cache controls Callers can send a request exactly once (retry: false) for mutations whose side effects happen before a failure can be reported, and bypass the local response cache (cache: "no-store") for preflight reads that authorize a mutation. The default shared fetch is unchanged. Co-authored-by: Cursor --- packages/cli/src/lib/sentry-client.ts | 69 +++++++++++------ packages/cli/test/lib/sentry-client.test.ts | 84 +++++++++++++++++++++ 2 files changed, 130 insertions(+), 23 deletions(-) diff --git a/packages/cli/src/lib/sentry-client.ts b/packages/cli/src/lib/sentry-client.ts index c1bd02724d..033b4da863 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; @@ -529,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, @@ -551,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; } @@ -566,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); } @@ -594,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 @@ -626,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( @@ -639,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}` }); @@ -728,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 @@ -736,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; @@ -745,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 b9ee298c42..7ca2c1f77c 100644 --- a/packages/cli/test/lib/sentry-client.test.ts +++ b/packages/cli/test/lib/sentry-client.test.ts @@ -6,6 +6,10 @@ 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, @@ -71,6 +75,86 @@ describe("401 replay", () => { 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", () => {