Skip to content

Commit 01f7b77

Browse files
fix(cli): Generate the idempotent operations list at build time
Go back to having generate:schema write the marked writes to the gitignored src/generated/idempotent-operations.json instead of shipping the whole @sentry/api catalog in the bundle. The apps/local tests import CLI modules that reach sentry-client.ts and ran before anything generated the file. Its test script now runs ensure-generated.mjs first, the same bootstrap the CLI's own scripts use. Chunk uploads keep their explicit opt-in: they post to the URL the server's chunk-upload options return, which need not match the catalog path.
1 parent df7b3e5 commit 01f7b77

7 files changed

Lines changed: 84 additions & 59 deletions

File tree

‎apps/local/package.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@
88
"build": "tsc -b && vite build",
99
"lint": "oxlint",
1010
"preview": "vite preview",
11-
"test": "vitest run",
12-
"test:integration": "vitest run src/App.integration.test.tsx",
11+
"test": "pnpm --filter sentry exec node script/ensure-generated.mjs schema && vitest run",
12+
"test:integration": "pnpm run test src/App.integration.test.tsx",
1313
"typecheck": "tsc -b",
1414
"sample": "tsx scripts/replay.ts"
1515
},

‎packages/cli/script/ensure-generated.mjs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,11 @@
1818
import { execSync } from "node:child_process";
1919
import { existsSync } from "node:fs";
2020

21-
const SCHEMA = "src/generated/api-schema.json";
21+
/** Produced by `generate:schema`; imported at runtime by `src/`. */
22+
const SCHEMA_OUTPUTS = [
23+
"src/generated/api-schema.json",
24+
"src/generated/idempotent-operations.json",
25+
];
2226
/** Produced by `generate:grammar`; imported by `src/lib/arg-parsing.ts`. */
2327
const GRAMMAR_OUTPUTS = [
2428
"src/generated/org-project-target-grammar.js",
@@ -35,7 +39,7 @@ function run(script) {
3539
execSync(`pnpm run ${script}`, { stdio: "inherit" });
3640
}
3741

38-
if (!existsSync(SCHEMA)) {
42+
if (!SCHEMA_OUTPUTS.every(existsSync)) {
3943
run("generate:schema");
4044
}
4145
if (process.argv[2] !== "schema" && !GRAMMAR_OUTPUTS.every(existsSync)) {

‎packages/cli/script/generate-api-schema.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,22 @@
1212
* Also reads SDK function names from the installed @sentry/api package to
1313
* map operationIds to their TypeScript SDK function names.
1414
*
15+
* Also reads the `@sentry/api/operations` catalog to list the non-safe
16+
* operations Sentry marks idempotent (`x-sentry-idempotent`), which the
17+
* authenticated fetch retries on transient failures.
18+
*
1519
* Usage:
1620
* tsx script/generate-api-schema.ts
1721
*
1822
* Output:
1923
* src/generated/api-schema.json
24+
* src/generated/idempotent-operations.json
2025
*/
2126

2227
import { mkdir, readFile, writeFile } from "node:fs/promises";
2328
import { dirname, resolve } from "node:path";
2429
import { fileURLToPath } from "node:url";
30+
import { operations } from "@sentry/api/operations";
2531

2632
/** Resolve the root directory of @sentry/api by finding its main entry point. */
2733
function sentryApiDir(): string {
@@ -32,6 +38,7 @@ function sentryApiDir(): string {
3238
}
3339

3440
const OUTPUT_PATH = "src/generated/api-schema.json";
41+
const IDEMPOTENT_OUTPUT_PATH = "src/generated/idempotent-operations.json";
3542

3643
/**
3744
* Build the OpenAPI spec URL from the installed @sentry/api version.
@@ -53,6 +60,8 @@ const PATH_PARAM_PATTERN = /\{(\w+)\}/g;
5360
export type { ApiEndpoint } from "../src/lib/api-schema.js";
5461

5562
import type { ApiEndpoint } from "../src/lib/api-schema.js";
63+
import type { IdempotentOperation } from "../src/lib/idempotent-operations.js";
64+
import { isRetryableMethod } from "../src/lib/retry-policy.js";
5665

5766
// ---------------------------------------------------------------------------
5867
// OpenAPI Types (minimal subset we need)
@@ -231,3 +240,21 @@ await writeFile(OUTPUT_PATH, JSON.stringify(endpoints, null, 2));
231240
console.log(
232241
`Generated ${OUTPUT_PATH} (${endpoints.length} endpoints, ${Math.round(JSON.stringify(endpoints).length / 1024)}KB)`,
233242
);
243+
244+
// Safe methods already retry, so only writes need listing; that keeps the
245+
// file the CLI loads on every request tiny.
246+
const idempotentOperations: IdempotentOperation[] = operations
247+
.filter((op) => op.idempotent && !isRetryableMethod(op.method))
248+
.map((op) => ({ method: op.method, path: op.path }))
249+
.sort(
250+
(a, b) => a.path.localeCompare(b.path) || a.method.localeCompare(b.method),
251+
);
252+
253+
await writeFile(
254+
IDEMPOTENT_OUTPUT_PATH,
255+
JSON.stringify(idempotentOperations, null, 2),
256+
);
257+
258+
console.log(
259+
`Generated ${IDEMPOTENT_OUTPUT_PATH} (${idempotentOperations.length} operations)`,
260+
);

‎packages/cli/src/global.d.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,3 +47,8 @@ declare module "*/generated/api-schema.json" {
4747
const endpoints: unknown;
4848
export default endpoints;
4949
}
50+
51+
declare module "*/generated/idempotent-operations.json" {
52+
const operations: unknown;
53+
export default operations;
54+
}
Lines changed: 22 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,28 @@
11
/**
22
* Writes Sentry declares safe to replay.
33
*
4-
* Read from the `@sentry/api` operation catalog, which marks an operation
5-
* `idempotent` when Sentry sets the `x-sentry-idempotent` OpenAPI extension.
6-
* Endpoints opt in on the server, so the CLI does not hard-code which
7-
* assemble calls may retry.
4+
* `script/generate-api-schema.ts` copies the non-safe operations that the
5+
* `@sentry/api` catalog marks `idempotent` (from Sentry's
6+
* `x-sentry-idempotent` OpenAPI extension). Endpoints opt in on the server,
7+
* so the CLI does not hard-code which assemble calls may retry.
88
*/
99

10-
import { isRetryableMethod } from "./retry-policy.js";
10+
import generated from "../generated/idempotent-operations.json";
11+
12+
/** A non-safe operation Sentry marks idempotent. */
13+
export type IdempotentOperation = {
14+
/** Uppercase HTTP method */
15+
method: string;
16+
/** OpenAPI path template, such as `/api/0/organizations/{org}/chunk-upload/` */
17+
path: string;
18+
};
1119

1220
type Matcher = { method: string; pattern: RegExp };
1321

1422
const TEMPLATE_SEGMENT = /^\{[^/]+\}$/;
1523
const REGEXP_SPECIAL = /[.*+?^${}()|[\]\\]/g;
1624

17-
function toMatcher(method: string, path: string): Matcher {
25+
function toMatcher({ method, path }: IdempotentOperation): Matcher {
1826
const source = path
1927
.split("/")
2028
.map((segment) =>
@@ -23,34 +31,22 @@ function toMatcher(method: string, path: string): Matcher {
2331
: segment.replace(REGEXP_SPECIAL, "\\$&"),
2432
)
2533
.join("/");
26-
return { method, pattern: new RegExp(`^${source}$`) };
34+
return { method: method.toUpperCase(), pattern: new RegExp(`^${source}$`) };
2735
}
2836

29-
/** Loaded on the first write so read-only commands never parse the catalog. */
30-
let matchers: Promise<Matcher[]> | undefined;
31-
32-
async function loadMatchers(): Promise<Matcher[]> {
33-
const { operations } = await import("@sentry/api/operations");
34-
return operations
35-
.filter((op) => op.idempotent && !isRetryableMethod(op.method))
36-
.map((op) => toMatcher(op.method, op.path));
37-
}
37+
let matchers: Matcher[] | undefined;
3838

3939
/**
40-
* Whether Sentry marks this write's operation idempotent.
40+
* Whether Sentry marks this request's operation idempotent.
4141
*
4242
* Matches from the `/api/` segment on, so a self-hosted install served under
43-
* a path prefix still matches. Safe methods return false without loading the
44-
* catalog; they retry regardless.
43+
* a path prefix still matches.
4544
*
4645
* @param method - HTTP method (any case)
4746
* @param url - Absolute request URL
4847
*/
49-
export async function isIdempotentOperation(
50-
method: string,
51-
url: string,
52-
): Promise<boolean> {
53-
if (isRetryableMethod(method) || !URL.canParse(url)) {
48+
export function isIdempotentOperation(method: string, url: string): boolean {
49+
if (!URL.canParse(url)) {
5450
return false;
5551
}
5652
const pathname = new URL(url).pathname;
@@ -62,8 +58,8 @@ export async function isIdempotentOperation(
6258
const normalized = apiPath.endsWith("/") ? apiPath : `${apiPath}/`;
6359
const upper = method.toUpperCase();
6460

65-
matchers ??= loadMatchers();
66-
return (await matchers).some(
61+
matchers ??= (generated as IdempotentOperation[]).map(toMatcher);
62+
return matchers.some(
6763
(matcher) => matcher.method === upper && matcher.pattern.test(normalized),
6864
);
6965
}

‎packages/cli/src/lib/sentry-client.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -708,9 +708,10 @@ async function fetchWithRetry({
708708
}
709709
const attemptFactory = await buildAttemptFactory(input, init);
710710
const timeoutMs = resolveTimeoutMs(fullUrl);
711-
const methodRetryable =
712-
isRetryableMethod(method, idempotent) ||
713-
(await isIdempotentOperation(method, fullUrl));
711+
const methodRetryable = isRetryableMethod(
712+
method,
713+
idempotent || isIdempotentOperation(method, fullUrl),
714+
);
714715
const urlPath = extractUrlPath(fullUrl);
715716

716717
for (let attempt = 0; attempt <= MAX_RETRIES; attempt++) {

‎packages/cli/test/lib/idempotent-operations.test.ts‎

Lines changed: 18 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -8,82 +8,74 @@ describe("isIdempotentOperation", () => {
88
"/api/0/organizations/acme/chunk-upload/",
99
"/api/0/organizations/acme/artifactbundle/assemble/",
1010
"/api/0/projects/acme/web/files/difs/assemble/",
11-
])("matches POST %s", async (path) => {
12-
expect(await isIdempotentOperation("POST", `${REGION_URL}${path}`)).toBe(
13-
true,
14-
);
11+
])("matches POST %s", (path) => {
12+
expect(isIdempotentOperation("POST", `${REGION_URL}${path}`)).toBe(true);
1513
});
1614

17-
test("ignores method case and a missing trailing slash", async () => {
15+
test("ignores method case and a missing trailing slash", () => {
1816
expect(
19-
await isIdempotentOperation(
17+
isIdempotentOperation(
2018
"post",
2119
`${REGION_URL}/api/0/organizations/acme/chunk-upload`,
2220
),
2321
).toBe(true);
2422
});
2523

26-
test("matches under a self-hosted path prefix", async () => {
24+
test("matches under a self-hosted path prefix", () => {
2725
expect(
28-
await isIdempotentOperation(
26+
isIdempotentOperation(
2927
"POST",
3028
"https://sentry.example.com/sentry/api/0/organizations/acme/chunk-upload/",
3129
),
3230
).toBe(true);
3331
});
3432

35-
test("ignores the query string", async () => {
33+
test("ignores the query string", () => {
3634
expect(
37-
await isIdempotentOperation(
35+
isIdempotentOperation(
3836
"POST",
3937
`${REGION_URL}/api/0/organizations/acme/chunk-upload/?foo=bar`,
4038
),
4139
).toBe(true);
4240
});
4341

44-
test("does not match other methods on a marked path", async () => {
42+
test("does not match other methods on a marked path", () => {
4543
expect(
46-
await isIdempotentOperation(
44+
isIdempotentOperation(
4745
"PUT",
4846
`${REGION_URL}/api/0/organizations/acme/chunk-upload/`,
4947
),
5048
).toBe(false);
5149
});
5250

53-
test("does not match writes Sentry leaves unmarked", async () => {
51+
test("does not match writes Sentry leaves unmarked", () => {
5452
expect(
55-
await isIdempotentOperation(
53+
isIdempotentOperation(
5654
"POST",
5755
`${REGION_URL}/api/0/projects/acme/web/files/preprodartifacts/assemble/`,
5856
),
5957
).toBe(false);
6058
expect(
61-
await isIdempotentOperation(
59+
isIdempotentOperation(
6260
"POST",
6361
`${REGION_URL}/api/0/organizations/acme/issues/1/autofix/`,
6462
),
6563
).toBe(false);
6664
});
6765

68-
test("does not let a template segment span several path segments", async () => {
66+
test("does not let a template segment span several path segments", () => {
6967
expect(
70-
await isIdempotentOperation(
68+
isIdempotentOperation(
7169
"POST",
7270
`${REGION_URL}/api/0/organizations/acme/extra/chunk-upload/`,
7371
),
7472
).toBe(false);
7573
});
7674

77-
test("returns false for safe methods, unparseable URLs, and non-API paths", async () => {
75+
test("returns false for URLs it cannot parse or that are not API paths", () => {
76+
expect(isIdempotentOperation("POST", "not a url")).toBe(false);
7877
expect(
79-
await isIdempotentOperation(
80-
"GET",
81-
`${REGION_URL}/api/0/organizations/acme/chunk-upload/`,
82-
),
83-
).toBe(false);
84-
expect(await isIdempotentOperation("POST", "not a url")).toBe(false);
85-
expect(
86-
await isIdempotentOperation("POST", `${REGION_URL}/organizations/acme/`),
78+
isIdempotentOperation("POST", `${REGION_URL}/organizations/acme/`),
8779
).toBe(false);
8880
});
8981
});

0 commit comments

Comments
 (0)