diff --git a/apps/local/package.json b/apps/local/package.json index eab1c58ea..45e9fc771 100644 --- a/apps/local/package.json +++ b/apps/local/package.json @@ -8,8 +8,8 @@ "build": "tsc -b && vite build", "lint": "oxlint", "preview": "vite preview", - "test": "vitest run", - "test:integration": "vitest run src/App.integration.test.tsx", + "test": "pnpm --filter sentry exec node script/ensure-generated.mjs schema && vitest run", + "test:integration": "pnpm run test src/App.integration.test.tsx", "typecheck": "tsc -b", "sample": "tsx scripts/replay.ts" }, diff --git a/packages/cli/script/ensure-generated.mjs b/packages/cli/script/ensure-generated.mjs index 2a7cd780f..79bdeb2e5 100644 --- a/packages/cli/script/ensure-generated.mjs +++ b/packages/cli/script/ensure-generated.mjs @@ -18,7 +18,11 @@ import { execSync } from "node:child_process"; import { existsSync } from "node:fs"; -const SCHEMA = "src/generated/api-schema.json"; +/** Produced by `generate:schema`; imported at runtime by `src/`. */ +const SCHEMA_OUTPUTS = [ + "src/generated/api-schema.json", + "src/generated/idempotent-operations.json", +]; /** Produced by `generate:grammar`; imported by `src/lib/arg-parsing.ts`. */ const GRAMMAR_OUTPUTS = [ "src/generated/org-project-target-grammar.js", @@ -35,7 +39,7 @@ function run(script) { execSync(`pnpm run ${script}`, { stdio: "inherit" }); } -if (!existsSync(SCHEMA)) { +if (!SCHEMA_OUTPUTS.every(existsSync)) { run("generate:schema"); } if (process.argv[2] !== "schema" && !GRAMMAR_OUTPUTS.every(existsSync)) { diff --git a/packages/cli/script/generate-api-schema.ts b/packages/cli/script/generate-api-schema.ts index 7c86e6644..cf128099d 100644 --- a/packages/cli/script/generate-api-schema.ts +++ b/packages/cli/script/generate-api-schema.ts @@ -12,16 +12,22 @@ * Also reads SDK function names from the installed @sentry/api package to * map operationIds to their TypeScript SDK function names. * + * Also reads the `@sentry/api/operations` catalog to list the non-safe + * operations Sentry marks idempotent (`x-sentry-idempotent`), which the + * authenticated fetch retries on transient failures. + * * Usage: * tsx script/generate-api-schema.ts * * Output: * src/generated/api-schema.json + * src/generated/idempotent-operations.json */ import { mkdir, readFile, writeFile } from "node:fs/promises"; import { dirname, resolve } from "node:path"; import { fileURLToPath } from "node:url"; +import { operations } from "@sentry/api/operations"; /** Resolve the root directory of @sentry/api by finding its main entry point. */ function sentryApiDir(): string { @@ -32,6 +38,7 @@ function sentryApiDir(): string { } const OUTPUT_PATH = "src/generated/api-schema.json"; +const IDEMPOTENT_OUTPUT_PATH = "src/generated/idempotent-operations.json"; /** * Build the OpenAPI spec URL from the installed @sentry/api version. @@ -53,6 +60,8 @@ const PATH_PARAM_PATTERN = /\{(\w+)\}/g; export type { ApiEndpoint } from "../src/lib/api-schema.js"; import type { ApiEndpoint } from "../src/lib/api-schema.js"; +import type { IdempotentOperation } from "../src/lib/idempotent-operations.js"; +import { isRetryableMethod } from "../src/lib/retry-policy.js"; // --------------------------------------------------------------------------- // OpenAPI Types (minimal subset we need) @@ -231,3 +240,21 @@ await writeFile(OUTPUT_PATH, JSON.stringify(endpoints, null, 2)); console.log( `Generated ${OUTPUT_PATH} (${endpoints.length} endpoints, ${Math.round(JSON.stringify(endpoints).length / 1024)}KB)`, ); + +// Safe methods already retry, so only writes need listing; that keeps the +// file the CLI loads on every request tiny. +const idempotentOperations: IdempotentOperation[] = operations + .filter((op) => op.idempotent && !isRetryableMethod(op.method)) + .map((op) => ({ method: op.method, path: op.path })) + .sort( + (a, b) => a.path.localeCompare(b.path) || a.method.localeCompare(b.method), + ); + +await writeFile( + IDEMPOTENT_OUTPUT_PATH, + JSON.stringify(idempotentOperations, null, 2), +); + +console.log( + `Generated ${IDEMPOTENT_OUTPUT_PATH} (${idempotentOperations.length} operations)`, +); diff --git a/packages/cli/src/global.d.ts b/packages/cli/src/global.d.ts index 0d36faf7e..1433484f6 100644 --- a/packages/cli/src/global.d.ts +++ b/packages/cli/src/global.d.ts @@ -47,3 +47,8 @@ declare module "*/generated/api-schema.json" { const endpoints: unknown; export default endpoints; } + +declare module "*/generated/idempotent-operations.json" { + const operations: unknown; + export default operations; +} diff --git a/packages/cli/src/lib/api/chunk-upload.ts b/packages/cli/src/lib/api/chunk-upload.ts index a96ecfc56..426069ed8 100644 --- a/packages/cli/src/lib/api/chunk-upload.ts +++ b/packages/cli/src/lib/api/chunk-upload.ts @@ -395,7 +395,8 @@ export async function uploadMissingChunks(params: { } const limit = pLimit(serverOptions.concurrency); - // Chunks are stored by SHA-1, so a replayed upload is a no-op. + // Chunks are stored by SHA-1, so a replayed upload is a no-op. Opt in + // explicitly: the server-provided URL need not match the catalog path. const { fetch: authFetch } = getSdkConfig(regionUrl, { idempotent: true }); await Promise.all( @@ -488,7 +489,8 @@ export async function uploadMissingBufferChunks(params: { } const limit = pLimit(serverOptions.concurrency); - // Chunks are stored by SHA-1, so a replayed upload is a no-op. + // Chunks are stored by SHA-1, so a replayed upload is a no-op. Opt in + // explicitly: the server-provided URL need not match the catalog path. const { fetch: authFetch } = getSdkConfig(regionUrl, { idempotent: true }); await Promise.all( @@ -552,8 +554,6 @@ export async function pollAssembly(params: { method: "POST", body, schema, - // Assembly state is keyed by checksum; re-sending only polls it. - idempotent: true, }, ); diff --git a/packages/cli/src/lib/api/dart-symbols.ts b/packages/cli/src/lib/api/dart-symbols.ts index eb7858a80..6ca5212e8 100644 --- a/packages/cli/src/lib/api/dart-symbols.ts +++ b/packages/cli/src/lib/api/dart-symbols.ts @@ -159,8 +159,6 @@ export async function uploadDartSymbolMap( method: "POST", body: assembleBody, schema: DifAssembleResponseSchema, - // Assembly state is keyed by checksum; re-sending only polls it. - idempotent: true, }, ); @@ -196,7 +194,6 @@ export async function uploadDartSymbolMap( method: "POST", body: assembleBody, schema: DifAssembleResponseSchema, - idempotent: true, }, ); diff --git a/packages/cli/src/lib/api/debug-files.ts b/packages/cli/src/lib/api/debug-files.ts index b66494da5..a990b1f5b 100644 --- a/packages/cli/src/lib/api/debug-files.ts +++ b/packages/cli/src/lib/api/debug-files.ts @@ -272,7 +272,6 @@ async function postAssemble( endpoint: string, body: unknown, ): Promise { - // Assembly state is keyed by checksum; re-sending only polls it. const { data } = await apiRequestToRegion( regionUrl, endpoint, @@ -280,7 +279,6 @@ async function postAssemble( method: "POST", body, schema: DifAssembleResponseSchema, - idempotent: true, }, ); return data; diff --git a/packages/cli/src/lib/api/proguard.ts b/packages/cli/src/lib/api/proguard.ts index 5cb1a1d99..5f2147488 100644 --- a/packages/cli/src/lib/api/proguard.ts +++ b/packages/cli/src/lib/api/proguard.ts @@ -175,8 +175,6 @@ export async function uploadProguardMappings( method: "POST", body: assembleBody, schema: DifAssembleResponseSchema, - // Assembly state is keyed by checksum; re-sending only polls it. - idempotent: true, }, ); @@ -237,7 +235,6 @@ async function pollDifAssembly(params: { method: "POST", body, schema: DifAssembleResponseSchema, - idempotent: true, }, ); diff --git a/packages/cli/src/lib/api/sourcemaps.ts b/packages/cli/src/lib/api/sourcemaps.ts index 2421bbffb..67425a1ad 100644 --- a/packages/cli/src/lib/api/sourcemaps.ts +++ b/packages/cli/src/lib/api/sourcemaps.ts @@ -340,8 +340,6 @@ async function uploadArtifactBundle(opts: { method: "POST", body: assembleBody, schema: AssembleResponseSchema, - // Assembly state is keyed by checksum; re-sending only polls it. - idempotent: true, }, ); diff --git a/packages/cli/src/lib/idempotent-operations.ts b/packages/cli/src/lib/idempotent-operations.ts new file mode 100644 index 000000000..31df37154 --- /dev/null +++ b/packages/cli/src/lib/idempotent-operations.ts @@ -0,0 +1,65 @@ +/** + * Writes Sentry declares safe to replay. + * + * `script/generate-api-schema.ts` copies the non-safe operations that the + * `@sentry/api` catalog marks `idempotent` (from Sentry's + * `x-sentry-idempotent` OpenAPI extension). Endpoints opt in on the server, + * so the CLI does not hard-code which assemble calls may retry. + */ + +import generated from "../generated/idempotent-operations.json"; + +/** A non-safe operation Sentry marks idempotent. */ +export type IdempotentOperation = { + /** Uppercase HTTP method */ + method: string; + /** OpenAPI path template, such as `/api/0/organizations/{org}/chunk-upload/` */ + path: string; +}; + +type Matcher = { method: string; pattern: RegExp }; + +const TEMPLATE_SEGMENT = /^\{[^/]+\}$/; +const REGEXP_SPECIAL = /[.*+?^${}()|[\]\\]/g; + +function toMatcher({ method, path }: IdempotentOperation): Matcher { + const source = path + .split("/") + .map((segment) => + TEMPLATE_SEGMENT.test(segment) + ? "[^/]+" + : segment.replace(REGEXP_SPECIAL, "\\$&"), + ) + .join("/"); + return { method: method.toUpperCase(), pattern: new RegExp(`^${source}$`) }; +} + +let matchers: Matcher[] | undefined; + +/** + * Whether Sentry marks this request's operation idempotent. + * + * Matches from the `/api/` segment on, so a self-hosted install served under + * a path prefix still matches. + * + * @param method - HTTP method (any case) + * @param url - Absolute request URL + */ +export function isIdempotentOperation(method: string, url: string): boolean { + if (!URL.canParse(url)) { + return false; + } + const pathname = new URL(url).pathname; + const apiIndex = pathname.indexOf("/api/"); + if (apiIndex === -1) { + return false; + } + const apiPath = pathname.slice(apiIndex); + const normalized = apiPath.endsWith("/") ? apiPath : `${apiPath}/`; + const upper = method.toUpperCase(); + + matchers ??= (generated as IdempotentOperation[]).map(toMatcher); + return matchers.some( + (matcher) => matcher.method === upper && matcher.pattern.test(normalized), + ); +} diff --git a/packages/cli/src/lib/retry-policy.ts b/packages/cli/src/lib/retry-policy.ts index aeebfbde9..bb9ec4c36 100644 --- a/packages/cli/src/lib/retry-policy.ts +++ b/packages/cli/src/lib/retry-policy.ts @@ -10,8 +10,10 @@ * server already applied it, so replaying could duplicate the write. * PUT and DELETE are excluded too: Sentry uses PUT for partial updates with * side effects (issue merges, activity, notifications), and a replayed - * DELETE turns a successful delete into a 404. Callers that know a write is - * safe to replay (content-addressed uploads) opt in per request. + * DELETE turns a successful delete into a 404. Writes Sentry marks + * idempotent (`x-sentry-idempotent`, see idempotent-operations.ts) replay, + * and callers that know an unmarked write is safe to replay opt in per + * request. * * A 429 is the exception and retries for any method: Sentry's rate-limit * middleware rejects the request before the view runs, so nothing was diff --git a/packages/cli/src/lib/sentry-client.ts b/packages/cli/src/lib/sentry-client.ts index e3543441a..0b780e8d1 100644 --- a/packages/cli/src/lib/sentry-client.ts +++ b/packages/cli/src/lib/sentry-client.ts @@ -45,6 +45,7 @@ import { TimeoutError, } from "./errors.js"; import { extractInputUrl } from "./fetch-input.js"; +import { isIdempotentOperation } from "./idempotent-operations.js"; import { logger } from "./logger.js"; import { type CacheLookup, @@ -707,7 +708,10 @@ async function fetchWithRetry({ } const attemptFactory = await buildAttemptFactory(input, init); const timeoutMs = resolveTimeoutMs(fullUrl); - const methodRetryable = isRetryableMethod(method, idempotent); + const methodRetryable = isRetryableMethod( + method, + idempotent || isIdempotentOperation(method, fullUrl), + ); const urlPath = extractUrlPath(fullUrl); for (let attempt = 0; attempt <= MAX_RETRIES; attempt++) { @@ -763,7 +767,8 @@ async function fetchWithRetry({ * - Auth token injection (Bearer token) * - Request timeout via AbortController * - Automatic retry on transient HTTP errors (408, 429, 5xx), network errors, - * and timeouts — for GET/HEAD/OPTIONS, or any method with `idempotent` + * and timeouts — for GET/HEAD/OPTIONS, operations Sentry marks idempotent + * (see idempotent-operations.ts), or any method with `idempotent` * - 429 handling (any method; rejected before processing): waits for the * server's rate-limit reset when it fits the cap, otherwise throws * {@link RateLimitError} @@ -784,8 +789,9 @@ type AuthenticatedFetchOptions = { bufferResponseBody?: boolean; /** * Caller asserts that replaying the request is harmless, so transient - * failures retry for any method. Only for writes the server dedupes, such - * as content-addressed chunk uploads and checksum-keyed assemble calls. + * failures retry for any method. Operations Sentry marks idempotent retry + * without this; set it for writes the server dedupes that the catalog + * cannot match, such as chunk uploads to the server-provided URL. */ idempotent?: boolean; }; diff --git a/packages/cli/test/lib/idempotent-operations.test.ts b/packages/cli/test/lib/idempotent-operations.test.ts new file mode 100644 index 000000000..67f4c5eb5 --- /dev/null +++ b/packages/cli/test/lib/idempotent-operations.test.ts @@ -0,0 +1,81 @@ +import { describe, expect, test } from "vitest"; +import { isIdempotentOperation } from "../../src/lib/idempotent-operations.js"; + +const REGION_URL = "https://us.sentry.io"; + +describe("isIdempotentOperation", () => { + test.each([ + "/api/0/organizations/acme/chunk-upload/", + "/api/0/organizations/acme/artifactbundle/assemble/", + "/api/0/projects/acme/web/files/difs/assemble/", + ])("matches POST %s", (path) => { + expect(isIdempotentOperation("POST", `${REGION_URL}${path}`)).toBe(true); + }); + + test("ignores method case and a missing trailing slash", () => { + expect( + isIdempotentOperation( + "post", + `${REGION_URL}/api/0/organizations/acme/chunk-upload`, + ), + ).toBe(true); + }); + + test("matches under a self-hosted path prefix", () => { + expect( + isIdempotentOperation( + "POST", + "https://sentry.example.com/sentry/api/0/organizations/acme/chunk-upload/", + ), + ).toBe(true); + }); + + test("ignores the query string", () => { + expect( + isIdempotentOperation( + "POST", + `${REGION_URL}/api/0/organizations/acme/chunk-upload/?foo=bar`, + ), + ).toBe(true); + }); + + test("does not match other methods on a marked path", () => { + expect( + isIdempotentOperation( + "PUT", + `${REGION_URL}/api/0/organizations/acme/chunk-upload/`, + ), + ).toBe(false); + }); + + test("does not match writes Sentry leaves unmarked", () => { + expect( + isIdempotentOperation( + "POST", + `${REGION_URL}/api/0/projects/acme/web/files/preprodartifacts/assemble/`, + ), + ).toBe(false); + expect( + isIdempotentOperation( + "POST", + `${REGION_URL}/api/0/organizations/acme/issues/1/autofix/`, + ), + ).toBe(false); + }); + + test("does not let a template segment span several path segments", () => { + expect( + isIdempotentOperation( + "POST", + `${REGION_URL}/api/0/organizations/acme/extra/chunk-upload/`, + ), + ).toBe(false); + }); + + test("returns false for URLs it cannot parse or that are not API paths", () => { + expect(isIdempotentOperation("POST", "not a url")).toBe(false); + expect( + isIdempotentOperation("POST", `${REGION_URL}/organizations/acme/`), + ).toBe(false); + }); +}); diff --git a/packages/cli/test/lib/sentry-client.test.ts b/packages/cli/test/lib/sentry-client.test.ts index 12fe0e291..b86907027 100644 --- a/packages/cli/test/lib/sentry-client.test.ts +++ b/packages/cli/test/lib/sentry-client.test.ts @@ -244,6 +244,45 @@ describe("fetchWithRetry / buildAttemptFactory", () => { }); }); +describe("operations Sentry marks idempotent", () => { + test("retries a marked POST on a transient status without an opt-in", async () => { + let calls = 0; + globalThis.fetch = mockFetch(async () => { + calls += 1; + return calls === 1 + ? new Response("busy", { status: 503 }) + : jsonResponse({ state: "ok", missingChunks: [] }); + }); + + const res = await getAuthenticatedFetch()( + `${REGION_URL}/api/0/organizations/acme/artifactbundle/assemble/`, + { method: "POST", body: "{}" }, + ); + + expect(res.status).toBe(200); + expect(calls).toBe(2); + }); + + test("retries a marked POST after a network error without an opt-in", async () => { + let calls = 0; + globalThis.fetch = mockFetch(async () => { + calls += 1; + if (calls === 1) { + throw new TypeError("fetch failed"); + } + return jsonResponse({}); + }); + + const res = await getAuthenticatedFetch()( + `${REGION_URL}/api/0/projects/acme/web/files/difs/assemble/`, + { method: "POST", body: "{}" }, + ); + + expect(res.status).toBe(200); + expect(calls).toBe(2); + }); +}); + describe("non-idempotent requests", () => { test("does not retry a POST on a transient status", async () => { let calls = 0; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index e0cc31f2b..32f73fd4d 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -40,8 +40,8 @@ catalogs: specifier: ^1.2.3 version: 1.2.3 '@sentry/api': - specifier: 0.400.0 - version: 0.400.0 + specifier: 0.420.0 + version: 0.420.0 '@sentry/cloudflare': specifier: 11.6.0 version: 11.6.0 @@ -467,7 +467,7 @@ importers: version: 1.46.0(ai@6.0.230(zod@4.6.7))(express@5.3.0)(zod@4.6.7) '@sentry/api': specifier: 'catalog:' - version: 0.400.0(valibot@1.4.2(typescript@5.9.3))(zod@4.6.7) + version: 0.420.0(valibot@1.4.2(typescript@5.9.3))(zod@4.6.7) '@sentry/core': specifier: 'catalog:' version: 11.6.0 @@ -801,7 +801,7 @@ importers: version: 2.3.0 '@sentry/api': specifier: 'catalog:' - version: 0.400.0(valibot@1.4.2(typescript@5.9.3))(zod@4.6.7) + version: 0.420.0(valibot@1.4.2(typescript@5.9.3))(zod@4.6.7) '@sentry/core': specifier: 'catalog:' version: 11.6.0 @@ -4317,8 +4317,8 @@ packages: '@sec-ant/readable-stream@0.4.1': resolution: {integrity: sha512-831qok9r2t8AlxLko40y2ebgSDhenenCatLVeW/uBtnHPyhHOvG0C7TvfgecV+wHzIm5KUICgzmVpWS+IMEAeg==} - '@sentry/api@0.400.0': - resolution: {integrity: sha512-JEffW2hxsRTr3WuP0t/XW9lUdhZRR7RN+rOLQC7UwlqDzz7F893oIK4/8Ba7mNwtUeiYQ/4pl9rc7gG/iuItyQ==} + '@sentry/api@0.420.0': + resolution: {integrity: sha512-s5FOQbBy9qVdZGvZrBIfjJEInXPRW+mqWXFi/2qLqzqPY3tv0YD9tQDGozjShO03AOGV3WwHrUdIIcgFMOZ7vw==} engines: {node: '>=22'} peerDependencies: valibot: '*' @@ -12508,7 +12508,7 @@ snapshots: '@sec-ant/readable-stream@0.4.1': {} - '@sentry/api@0.400.0(valibot@1.4.2(typescript@5.9.3))(zod@4.6.7)': + '@sentry/api@0.420.0(valibot@1.4.2(typescript@5.9.3))(zod@4.6.7)': optionalDependencies: valibot: 1.4.2(typescript@5.9.3) zod: 4.6.7 diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index 7f8f8dd0a..86bda1339 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -51,7 +51,7 @@ catalog: "@modelcontextprotocol/server": 2.3.0 "@radix-ui/react-accordion": ^1.2.20 "@radix-ui/react-slot": ^1.2.3 - "@sentry/api": 0.400.0 + "@sentry/api": 0.420.0 "@sentry/cloudflare": 11.6.0 "@sentry/core": 11.6.0 "@sentry/node": 11.6.0 @@ -163,6 +163,6 @@ minimumReleaseAgeExclude: - "@oxlint/binding-win32-x64-msvc@1.87.0" - oxfmt@0.72.0 - oxlint@1.87.0 - - "@sentry/api@0.400.0" + - "@sentry/api@0.420.0" - miniflare@5.20261019.1-beta - wrangler@4.151.3