Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions apps/local/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand Down
8 changes: 6 additions & 2 deletions packages/cli/script/ensure-generated.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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)) {
Expand Down
27 changes: 27 additions & 0 deletions packages/cli/script/generate-api-schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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.
Expand All @@ -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)
Expand Down Expand Up @@ -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)`,
);
5 changes: 5 additions & 0 deletions packages/cli/src/global.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
8 changes: 4 additions & 4 deletions packages/cli/src/lib/api/chunk-upload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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,
},
);

Expand Down
3 changes: 0 additions & 3 deletions packages/cli/src/lib/api/dart-symbols.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
);

Expand Down Expand Up @@ -196,7 +194,6 @@ export async function uploadDartSymbolMap(
method: "POST",
body: assembleBody,
schema: DifAssembleResponseSchema,
idempotent: true,
},
);

Expand Down
2 changes: 0 additions & 2 deletions packages/cli/src/lib/api/debug-files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -272,15 +272,13 @@ async function postAssemble(
endpoint: string,
body: unknown,
): Promise<DifAssembleResponse> {
// Assembly state is keyed by checksum; re-sending only polls it.
const { data } = await apiRequestToRegion<DifAssembleResponse>(
regionUrl,
endpoint,
{
method: "POST",
body,
schema: DifAssembleResponseSchema,
idempotent: true,
},
);
return data;
Expand Down
3 changes: 0 additions & 3 deletions packages/cli/src/lib/api/proguard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
);

Expand Down Expand Up @@ -237,7 +235,6 @@ async function pollDifAssembly(params: {
method: "POST",
body,
schema: DifAssembleResponseSchema,
idempotent: true,
},
);

Expand Down
2 changes: 0 additions & 2 deletions packages/cli/src/lib/api/sourcemaps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
);

Expand Down
65 changes: 65 additions & 0 deletions packages/cli/src/lib/idempotent-operations.ts
Original file line number Diff line number Diff line change
@@ -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),
);
}
6 changes: 4 additions & 2 deletions packages/cli/src/lib/retry-policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 10 additions & 4 deletions packages/cli/src/lib/sentry-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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++) {
Expand Down Expand Up @@ -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}
Expand All @@ -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;
};
Expand Down
81 changes: 81 additions & 0 deletions packages/cli/test/lib/idempotent-operations.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
Loading
Loading