Skip to content

Commit eb0b7f6

Browse files
committed
refactor: extract shared pagination infrastructure from duplicated code
Extract two pieces of duplicated pagination logic into shared infrastructure: 1. Add buildPaginationContextKey() to src/lib/db/pagination.ts - a generalized context key builder that encodes host, type, scope, and optional key-value params. Rewrite buildOrgContextKey() to delegate to it. 2. Add parseCursorFlag() to src/lib/list-command.ts - extracts the cursor flag validation logic (reject bare integers, accept 'last' keyword and opaque cursor strings). Update LIST_CURSOR_FLAG to use it. Simplify trace/list.ts: - Remove local ALL_DIGITS_RE, buildContextKey(), and inline cursor parse fn - Use buildPaginationContextKey() and LIST_CURSOR_FLAG instead - Remove unused escapeContextKeyValue and getApiBaseUrl imports Simplify issue/list.ts: - Remove local ALL_DIGITS_RE constant - Replace inline cursor parse function with parseCursorFlag - Replace manual context key construction in handleOrgAllIssues with buildPaginationContextKey() - Keep escapeContextKeyValue and getApiBaseUrl imports (still used by buildMultiTargetContextKey) Add tests for both new functions.
1 parent 99adeab commit eb0b7f6

File tree

7 files changed

+167
-75
lines changed

7 files changed

+167
-75
lines changed

AGENTS.md

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -632,7 +632,7 @@ mock.module("./some-module", () => ({
632632
* **cli.sentry.dev is served from gh-pages branch via GitHub Pages**: \`cli.sentry.dev\` is served from gh-pages branch via GitHub Pages. Craft's gh-pages target runs \`git rm -r -f .\` before extracting docs — persist extra files via \`postReleaseCommand\` in \`.craft.yml\`. Install script supports \`--channel nightly\`, downloading from the \`nightly\` release tag directly. version.json is only used by upgrade/version-check flow.
633633
634634
<!-- lore:2c3eb7ab-1341-4392-89fd-d81095cfe9c4 -->
635-
* **npm bundle requires Node.js >= 22 due to node:sqlite polyfill**: The Sentry CLI npm package (dist/bin.cjs) requires Node.js >= 22 because the bun:sqlite polyfill uses Node.js 22's built-in \`node:sqlite\` module. The \`require('node:sqlite')\` happens during module loading (via esbuild's inject option) — before any user code runs. package.json declares \`engines: { node: '>=22' }\` but pnpm/npm don't enforce this by default. A runtime version guard in the esbuild banner catches this early with a clear error message pointing users to either upgrade Node.js or use the standalone binary (\`curl -fsSL https://cli.sentry.dev/install | bash\`).
635+
* **npm bundle requires Node.js >= 22 due to node:sqlite polyfill**: The npm package (dist/bin.cjs) requires Node.js >= 22 because the bun:sqlite polyfill uses \`node:sqlite\`. A runtime version guard in the esbuild banner catches this early. When writing esbuild banner strings in TS template literals, double-escape: \`\\\\\\\n\` in TS → \`\\\n\` in output → newline at runtime. Single \`\\\n\` produces a literal newline inside a JS string, causing SyntaxError.
636636
637637
<!-- lore:019c972c-9f0f-75cd-9e24-9bdbb1ac03d6 -->
638638
* **Numeric issue ID resolution returns org:undefined despite API success**: Numeric issue ID resolution in \`resolveNumericIssue()\`: (1) try DSN/env/config for org, (2) if found use \`getIssueInOrg(org, id)\` with region routing, (3) else fall back to unscoped \`getIssue(id)\`, (4) extract org from \`issue.permalink\` via \`parseSentryUrl\` as final fallback. The \`explicit-org-numeric\` case uses \`getIssueInOrg\`. \`resolveOrgAndIssueId\` no longer throws for bare numeric IDs when permalink contains the org slug.
@@ -653,9 +653,6 @@ mock.module("./some-module", () => ({
653653
<!-- lore:019c9e98-7af4-7e25-95f4-fc06f7abf564 -->
654654
* **Bun binary build requires SENTRY\_CLIENT\_ID env var**: The build script (\`script/bundle.ts\`) requires \`SENTRY\_CLIENT\_ID\` environment variable and exits with code 1 if missing. When building locally, use \`bun run --env-file=.env.local build\` or set the env var explicitly. The binary build (\`bun run build\`) also needs it. Without it you get: \`Error: SENTRY\_CLIENT\_ID environment variable is required.\`
655655
656-
<!-- lore:4729229d-36b9-4118-b90b-ea8151e6928f -->
657-
* **Esbuild banner template literal double-escape for newlines**: When using esbuild's \`banner\` option with a TypeScript template literal containing string literals that need \`\n\` escape sequences: use \`\\\\\\\n\` in the TS source. The chain is: TS template literal \`\\\\\\\n\` → esbuild banner output \`\\\n\` → JS runtime interprets as newline. Using only \`\\\n\` in the TS source produces a literal newline character inside a JS double-quoted string, which is a SyntaxError. This applies to any esbuild banner/footer that injects JS strings containing escape sequences. Discovered in script/bundle.ts for the Node.js version guard error message.
658-
659656
<!-- lore:019c992d-8e38-7148-91db-baa5029eb2c3 -->
660657
* **ghcr.io blob download: curl -L breaks because auth header leaks to Azure redirect**: ghcr.io gotchas: (1) \`curl -L\` with Authorization header fails on blob downloads because curl forwards the auth header to the Azure redirect target (404). Fix: two-step curl — extract redirect URL without following, then fetch without auth. (2) New GHCR packages default to private. Anonymous pulls fail until manually made public via GitHub UI package settings. This is a one-time step per package.
661658

src/commands/issue/list.ts

Lines changed: 8 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
} from "../../lib/api-client.js";
1919
import { parseOrgProjectArg } from "../../lib/arg-parsing.js";
2020
import {
21+
buildPaginationContextKey,
2122
clearPaginationCursor,
2223
escapeContextKeyValue,
2324
resolveOrgCursor,
@@ -47,6 +48,7 @@ import {
4748
LIST_BASE_ALIASES,
4849
LIST_JSON_FLAG,
4950
LIST_TARGET_POSITIONAL,
51+
parseCursorFlag,
5052
targetPatternExplanation,
5153
} from "../../lib/list-command.js";
5254
import {
@@ -87,9 +89,6 @@ const VALID_SORT_VALUES: SortValue[] = ["date", "new", "freq", "user"];
8789
/** Usage hint for ContextError messages */
8890
const USAGE_HINT = "sentry issue list <org>/<project>";
8991

90-
/** Matches strings that are all digits — used to detect invalid cursor values */
91-
const ALL_DIGITS_RE = /^\d+$/;
92-
9392
/**
9493
* Maximum --limit value (user-facing ceiling for practical CLI response times).
9594
* Auto-pagination can theoretically fetch more, but 1000 keeps responses reasonable.
@@ -748,11 +747,11 @@ type OrgAllIssuesOptions = {
748747
async function handleOrgAllIssues(options: OrgAllIssuesOptions): Promise<void> {
749748
const { stdout, stderr, org, flags, setContext } = options;
750749
// Encode sort + query in context key so cursors from different searches don't collide.
751-
const escapedQuery = flags.query
752-
? escapeContextKeyValue(flags.query)
753-
: undefined;
754-
const escapedPeriod = escapeContextKeyValue(flags.period ?? "90d");
755-
const contextKey = `host:${getApiBaseUrl()}|type:org:${org}|sort:${flags.sort}|period:${escapedPeriod}${escapedQuery ? `|q:${escapedQuery}` : ""}`;
750+
const contextKey = buildPaginationContextKey("org", org, {
751+
sort: flags.sort,
752+
period: flags.period ?? "90d",
753+
q: flags.query,
754+
});
756755
const cursor = resolveOrgCursor(flags.cursor, PAGINATION_KEY, contextKey);
757756

758757
setContext([org], []);
@@ -1172,21 +1171,7 @@ export const listCommand = buildListCommand("issue", {
11721171
json: LIST_JSON_FLAG,
11731172
cursor: {
11741173
kind: "parsed",
1175-
parse: (value: string) => {
1176-
// "last" is the magic keyword to resume from the saved cursor
1177-
if (value === "last") {
1178-
return value;
1179-
}
1180-
// Sentry pagination cursors are opaque strings like "1735689600:0:0".
1181-
// Plain integers are not valid cursors — catch this early so the user
1182-
// gets a clear error rather than a cryptic 400 from the API.
1183-
if (ALL_DIGITS_RE.test(value)) {
1184-
throw new Error(
1185-
`'${value}' is not a valid cursor. Cursors look like "1735689600:0:0". Use "last" to continue from the previous page.`
1186-
);
1187-
}
1188-
return value;
1189-
},
1174+
parse: parseCursorFlag,
11901175
brief:
11911176
'Pagination cursor for <org>/ or multi-target modes (use "last" to continue)',
11921177
optional: true,

src/commands/trace/list.ts

Lines changed: 7 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ import type { SentryContext } from "../../context.js";
88
import { listTransactions } from "../../lib/api-client.js";
99
import { validateLimit } from "../../lib/arg-parsing.js";
1010
import {
11+
buildPaginationContextKey,
1112
clearPaginationCursor,
12-
escapeContextKeyValue,
1313
resolveOrgCursor,
1414
setPaginationCursor,
1515
} from "../../lib/db/pagination.js";
@@ -20,10 +20,10 @@ import {
2020
} from "../../lib/formatters/index.js";
2121
import {
2222
buildListCommand,
23+
LIST_CURSOR_FLAG,
2324
TARGET_PATTERN_NOTE,
2425
} from "../../lib/list-command.js";
2526
import { resolveOrgProjectFromArg } from "../../lib/resolve-target.js";
26-
import { getApiBaseUrl } from "../../lib/sentry-client.js";
2727

2828
type ListFlags = {
2929
readonly limit: number;
@@ -53,31 +53,6 @@ const COMMAND_NAME = "trace list";
5353
/** Command key for pagination cursor storage */
5454
export const PAGINATION_KEY = "trace-list";
5555

56-
/** Matches strings that are all digits — used to detect invalid cursor values */
57-
const ALL_DIGITS_RE = /^\d+$/;
58-
59-
/**
60-
* Build a context key for pagination cursor storage.
61-
*
62-
* Encodes the API host, org, project, sort, and query so cursors from
63-
* different searches are never mixed.
64-
*/
65-
function buildContextKey(
66-
org: string,
67-
project: string,
68-
flags: Pick<ListFlags, "sort" | "query">
69-
): string {
70-
const host = getApiBaseUrl();
71-
const escapedQuery = flags.query
72-
? escapeContextKeyValue(flags.query)
73-
: undefined;
74-
return (
75-
`host:${host}|type:trace:${org}/${project}` +
76-
`|sort:${flags.sort}` +
77-
(escapedQuery ? `|q:${escapedQuery}` : "")
78-
);
79-
}
80-
8156
/** Build the CLI hint for fetching the next page, preserving active flags. */
8257
function nextPageHint(org: string, project: string, flags: ListFlags): string {
8358
const base = `sentry trace list ${org}/${project} -c last`;
@@ -160,22 +135,7 @@ export const listCommand = buildListCommand("trace", {
160135
brief: "Sort by: date, duration",
161136
default: "date" as const,
162137
},
163-
cursor: {
164-
kind: "parsed",
165-
parse: (value: string) => {
166-
if (value === "last") {
167-
return value;
168-
}
169-
if (ALL_DIGITS_RE.test(value)) {
170-
throw new Error(
171-
`'${value}' is not a valid cursor. Cursors look like "1735689600:0:0". Use "last" to continue from the previous page.`
172-
);
173-
}
174-
return value;
175-
},
176-
brief: 'Pagination cursor (use "last" to continue from previous page)',
177-
optional: true,
178-
},
138+
cursor: LIST_CURSOR_FLAG,
179139
json: {
180140
kind: "boolean",
181141
brief: "Output as JSON",
@@ -200,7 +160,10 @@ export const listCommand = buildListCommand("trace", {
200160
setContext([org], [project]);
201161

202162
// Build context key and resolve cursor for pagination
203-
const contextKey = buildContextKey(org, project, flags);
163+
const contextKey = buildPaginationContextKey("trace", `${org}/${project}`, {
164+
sort: flags.sort,
165+
q: flags.query,
166+
});
204167
const cursor = resolveOrgCursor(flags.cursor, PAGINATION_KEY, contextKey);
205168

206169
const { data: traces, nextCursor } = await listTransactions(org, project, {

src/lib/db/pagination.ts

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,44 @@ export function escapeContextKeyValue(value: string): string {
118118
return value.replaceAll("|", "%7C");
119119
}
120120

121+
/**
122+
* Build a composite context key for pagination cursor storage.
123+
*
124+
* Encodes the API host, a type discriminant, a scope string, and optional
125+
* key-value params (sort, query, period, platform, …) so cursors from
126+
* different searches are never mixed.
127+
*
128+
* @param type - Discriminant for the kind of listing (e.g. "org", "trace", "multi")
129+
* @param scope - Scoping string (e.g. org slug, "org/project", sorted target fingerprint)
130+
* @param params - Optional extra segments. Only defined values are included;
131+
* each value is escaped via {@link escapeContextKeyValue}.
132+
* @returns Composite context key string
133+
*
134+
* @example
135+
* // Simple org-scoped key (equivalent to legacy buildOrgContextKey)
136+
* buildPaginationContextKey("org", "my-org")
137+
* // → "host:https://sentry.io|type:org:my-org"
138+
*
139+
* // Trace list with sort + query
140+
* buildPaginationContextKey("trace", "my-org/my-project", { sort: "date", q: "GET" })
141+
* // → "host:https://sentry.io|type:trace:my-org/my-project|sort:date|q:GET"
142+
*/
143+
export function buildPaginationContextKey(
144+
type: string,
145+
scope: string,
146+
params?: Record<string, string | undefined>
147+
): string {
148+
let key = `host:${getApiBaseUrl()}|type:${type}:${scope}`;
149+
if (params) {
150+
for (const [name, value] of Object.entries(params)) {
151+
if (value !== undefined) {
152+
key += `|${name}:${escapeContextKeyValue(value)}`;
153+
}
154+
}
155+
}
156+
return key;
157+
}
158+
121159
/**
122160
* Build a context key for org-scoped pagination cursor storage.
123161
*
@@ -128,7 +166,7 @@ export function escapeContextKeyValue(value: string): string {
128166
* @returns Composite context key string
129167
*/
130168
export function buildOrgContextKey(org: string): string {
131-
return `host:${getApiBaseUrl()}|type:org:${org}`;
169+
return buildPaginationContextKey("org", org);
132170
}
133171

134172
/**

src/lib/list-command.ts

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,34 @@ export const LIST_JSON_FLAG = {
8383
default: false,
8484
} as const;
8585

86+
/** Matches strings that are all digits — used to detect invalid cursor values */
87+
const ALL_DIGITS_RE = /^\d+$/;
88+
89+
/**
90+
* Parse and validate a `--cursor` flag value.
91+
*
92+
* Accepts the magic `"last"` keyword (resume from stored cursor) and opaque
93+
* Sentry cursor strings (e.g. `"1735689600:0:0"`). Rejects bare integers
94+
* early — they are never valid cursors and would produce a cryptic 400 from
95+
* the API.
96+
*
97+
* Shared by {@link LIST_CURSOR_FLAG} and commands that define their own
98+
* cursor flag with a custom `brief`.
99+
*
100+
* @throws Error when value is a bare integer
101+
*/
102+
export function parseCursorFlag(value: string): string {
103+
if (value === "last") {
104+
return value;
105+
}
106+
if (ALL_DIGITS_RE.test(value)) {
107+
throw new Error(
108+
`'${value}' is not a valid cursor. Cursors look like "1735689600:0:0". Use "last" to continue from the previous page.`
109+
);
110+
}
111+
return value;
112+
}
113+
86114
/**
87115
* The `--cursor` / `-c` flag shared by all list commands.
88116
*
@@ -91,7 +119,7 @@ export const LIST_JSON_FLAG = {
91119
*/
92120
export const LIST_CURSOR_FLAG = {
93121
kind: "parsed" as const,
94-
parse: String,
122+
parse: parseCursorFlag,
95123
brief: 'Pagination cursor (use "last" to continue from previous page)',
96124
optional: true as const,
97125
};

test/lib/db/pagination.test.ts

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
/**
2+
* Unit tests for pagination context key builders.
3+
*/
4+
5+
import { describe, expect, test } from "bun:test";
6+
import {
7+
buildOrgContextKey,
8+
buildPaginationContextKey,
9+
} from "../../../src/lib/db/pagination.js";
10+
11+
describe("buildPaginationContextKey", () => {
12+
test("builds simple org-scoped key", () => {
13+
const key = buildPaginationContextKey("org", "my-org");
14+
expect(key).toContain("type:org:my-org");
15+
expect(key).toMatch(/^host:.+\|type:org:my-org$/);
16+
});
17+
18+
test("includes optional params when defined", () => {
19+
const key = buildPaginationContextKey("trace", "my-org/my-proj", {
20+
sort: "date",
21+
q: "GET /api",
22+
});
23+
expect(key).toContain("type:trace:my-org/my-proj");
24+
expect(key).toContain("|sort:date");
25+
expect(key).toContain("|q:GET /api");
26+
});
27+
28+
test("omits undefined params", () => {
29+
const key = buildPaginationContextKey("trace", "my-org/my-proj", {
30+
sort: "date",
31+
q: undefined,
32+
});
33+
expect(key).toContain("|sort:date");
34+
expect(key).not.toContain("|q:");
35+
});
36+
37+
test("escapes pipe characters in param values", () => {
38+
const key = buildPaginationContextKey("org", "my-org", {
39+
q: "a|b",
40+
});
41+
expect(key).toContain("|q:a%7Cb");
42+
expect(key).not.toContain("|q:a|b");
43+
});
44+
});
45+
46+
describe("buildOrgContextKey", () => {
47+
test("delegates to buildPaginationContextKey", () => {
48+
const orgKey = buildOrgContextKey("test-org");
49+
const directKey = buildPaginationContextKey("org", "test-org");
50+
expect(orgKey).toBe(directKey);
51+
});
52+
});

test/lib/list-command.test.ts

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
LIST_JSON_FLAG,
1616
LIST_TARGET_POSITIONAL,
1717
type OrgListCommandDocs,
18+
parseCursorFlag,
1819
} from "../../src/lib/list-command.js";
1920
import type { OrgListConfig } from "../../src/lib/org-list.js";
2021
// biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking
@@ -43,10 +44,12 @@ describe("LIST_JSON_FLAG", () => {
4344
});
4445

4546
describe("LIST_CURSOR_FLAG", () => {
46-
test("is an optional parsed string flag", () => {
47+
test("is an optional parsed flag with cursor validation", () => {
4748
expect(LIST_CURSOR_FLAG.kind).toBe("parsed");
4849
expect(LIST_CURSOR_FLAG.optional).toBe(true);
49-
expect(LIST_CURSOR_FLAG.parse).toBe(String);
50+
expect(LIST_CURSOR_FLAG.parse("last")).toBe("last");
51+
expect(LIST_CURSOR_FLAG.parse("1735689600:0:0")).toBe("1735689600:0:0");
52+
expect(() => LIST_CURSOR_FLAG.parse("12345")).toThrow("not a valid cursor");
5053
expect(LIST_CURSOR_FLAG.brief).toContain('"last"');
5154
});
5255
});
@@ -83,6 +86,32 @@ describe("LIST_BASE_ALIASES", () => {
8386
});
8487
});
8588

89+
// ---------------------------------------------------------------------------
90+
// parseCursorFlag: shared cursor validation
91+
// ---------------------------------------------------------------------------
92+
93+
describe("parseCursorFlag", () => {
94+
test("passes through 'last' keyword", () => {
95+
expect(parseCursorFlag("last")).toBe("last");
96+
});
97+
98+
test("passes through valid cursor strings", () => {
99+
expect(parseCursorFlag("1735689600:0:0")).toBe("1735689600:0:0");
100+
expect(parseCursorFlag("abc:def:0")).toBe("abc:def:0");
101+
});
102+
103+
test("rejects bare integer strings", () => {
104+
expect(() => parseCursorFlag("12345")).toThrow("not a valid cursor");
105+
expect(() => parseCursorFlag("0")).toThrow("not a valid cursor");
106+
expect(() => parseCursorFlag("999999999")).toThrow("not a valid cursor");
107+
});
108+
109+
test("accepts strings with digits and other characters", () => {
110+
expect(parseCursorFlag("123abc")).toBe("123abc");
111+
expect(parseCursorFlag("abc123")).toBe("abc123");
112+
});
113+
});
114+
86115
// ---------------------------------------------------------------------------
87116
// buildOrgListCommand: integration with dispatchOrgScopedList
88117
// ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)