Skip to content

Commit c22c4a8

Browse files
committed
fix(api): convert --data to query params for GET requests
When using `sentry api` with `--data` and a GET method (the default), the data was always sent as a request body. The Fetch API spec forbids bodies on GET/HEAD/OPTIONS requests, causing: TypeError: fetch() request with GET/HEAD/OPTIONS method cannot have body. Now `--data` on GET requests converts the data to query parameters: - URL-encoded strings (e.g. `stat=received&resolution=1d`) are parsed via URLSearchParams - JSON objects have their values stringified as query params - JSON arrays throw a clear ValidationError with guidance This matches how `--field`/`--raw-field` already handle GET requests via `prepareRequestOptions()`, and aligns with `gh api` behavior. Fixes CLI-CJ
1 parent 58d703f commit c22c4a8

File tree

3 files changed

+158
-3
lines changed

3 files changed

+158
-3
lines changed

AGENTS.md

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -635,7 +635,7 @@ mock.module("./some-module", () => ({
635635
* **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.
636636
637637
<!-- lore:019cbe93-19b8-7776-9705-20bbde226599 -->
638-
* **Nightly delta upgrade buildNightlyPatchGraph fetches ALL patch tags — O(N) HTTP calls**: Delta upgrade in \`src/lib/delta-upgrade.ts\` supports stable (GitHub Releases) and nightly (GHCR) channels. Lazy chain resolution: \`filterAndSortChainTags\` filters \`patch-\*\` tags by version range using \`Bun.semver.order()\`, fetches only 1-2 chain manifests. \`resolveNightlyDelta\` runs \`fetchManifest(targetTag)\` and \`listTags()\` in parallel. GHCR uses \`fetchWithRetry\`: 10s timeout + 1 retry; blobs get 30s. Manifest caching rejected (~0% hit rate). \*\*Patch pre-fetch\*\*: background version check (\`version-check.ts\`) downloads ~50-80KB patches to \`~/.sentry/patch-cache/\` via \`prefetchNightlyPatches\`/\`prefetchStablePatches\`. File-based cache (not SQLite) stores \`\<from>-\<to>.patch\` + \`chain-\<from>-\<to>.json\` metadata. \`loadCachedChain\` stitches patches across runs for multi-hop offline upgrades. 7-day TTL cleanup. \`delta.source\` span attribute distinguishes \`cache\` vs \`network\`. GHCR functions accept optional \`signal?: AbortSignal\`; \`fetchWithRetry\` combines it with timeout via \`AbortSignal.any()\`. \`isExternalAbort(error, signal)\` skips retries for external aborts. Critical for background prefetch where \`abortPendingVersionCheck()\` fires on process exit.
638+
* **Nightly delta upgrade buildNightlyPatchGraph fetches ALL patch tags — O(N) HTTP calls**: Delta upgrade in \`src/lib/delta-upgrade.ts\` supports stable (GitHub Releases) and nightly (GHCR) channels. \`filterAndSortChainTags\` filters \`patch-\*\` tags by version range using \`Bun.semver.order()\`, fetches only 1-2 chain manifests. GHCR uses \`fetchWithRetry\` (10s timeout + 1 retry; blobs 30s) with optional \`signal?: AbortSignal\` combined via \`AbortSignal.any()\`. \`isExternalAbort(error, signal)\` skips retries for external aborts — critical for background prefetch where \`abortPendingVersionCheck()\` fires on exit. Patch pre-fetch downloads ~50-80KB patches to \`~/.sentry/patch-cache/\` (file-based, not SQLite) with 7-day TTL. \`loadCachedChain\` stitches patches across runs for multi-hop offline upgrades. \`delta.source\` span attribute distinguishes \`cache\` vs \`network\`.
639639
640640
<!-- lore:2c3eb7ab-1341-4392-89fd-d81095cfe9c4 -->
641641
* **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.
@@ -662,6 +662,9 @@ mock.module("./some-module", () => ({
662662
<!-- lore:019c9e98-7af4-7e25-95f4-fc06f7abf564 -->
663663
* **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.\`
664664
665+
<!-- lore:019cc40e-e56e-71e9-bc5d-545f97df732b -->
666+
* **Consola prompt cancel returns truthy Symbol, not false**: When a user cancels a \`consola\` / \`@clack/prompts\` confirmation prompt (Ctrl+C), the return value is \`Symbol(clack:cancel)\`, not \`false\`. Since Symbols are truthy in JavaScript, checking \`!confirmed\` will be \`false\` and the code falls through as if the user confirmed. Fix: use \`confirmed !== true\` (strict equality) instead of \`!confirmed\` to correctly handle cancel, false, and any other non-true values.
667+
665668
<!-- lore:019c9776-e3dd-7632-88b8-358a19506218 -->
666669
* **GitHub immutable releases prevent rolling nightly tag pattern**: getsentry/cli has immutable GitHub releases — assets can't be modified and tags can NEVER be reused. Nightly builds are published to GHCR with versioned tags like \`nightly-0.14.0-dev.1772661724\`, NOT to GitHub Releases or npm. \`versionExists()\` routes nightly versions to GHCR manifest checks. \`downloadNightlyToPath()\` accepts optional \`version\` param for pinned versioned tags vs rolling \`:nightly\`. \`fetchManifest()\` throws \`UpgradeError("network\_error")\` for both network failures and non-200 — callers must check message for HTTP 404/403 to distinguish "not found" from real errors. Craft with no \`preReleaseCommand\` silently skips \`bump-version.sh\` if only target is \`github\` — must explicitly set it.
667670
@@ -677,20 +680,26 @@ mock.module("./some-module", () => ({
677680
<!-- lore:019c9741-d78e-73b1-87c2-e360ef6c7475 -->
678681
* **useTestConfigDir without isolateProjectRoot causes DSN scanning of repo tree**: \`useTestConfigDir()\` creates temp dirs under \`.test-tmp/\` in the repo tree. Without \`{ isolateProjectRoot: true }\`, \`findProjectRoot\` walks up and finds the repo's \`.git\`, causing DSN detection to scan real source code and trigger network calls against test mocks (timeouts). Always pass \`isolateProjectRoot: true\` when tests exercise \`resolveOrg\`, \`detectDsn\`, or \`findProjectRoot\`.
679682
683+
<!-- lore:019cc303-e397-75b9-9762-6f6ad108f50a -->
684+
* **Zod z.coerce.number() converts null to 0 silently**: Zod gotchas in this codebase: (1) \`z.coerce.number()\` passes input through \`Number()\`, so \`null\` silently becomes \`0\`. Be aware if \`null\` vs \`0\` distinction matters. (2) Zod v4 \`.default({})\` short-circuits — it returns the default value without parsing through inner schema defaults. So \`.object({ enabled: z.boolean().default(true) }).default({})\` returns \`{}\`, not \`{ enabled: true }\`. Fix: provide fully-populated default objects. This affected nested config sections in src/config.ts during the v3→v4 upgrade.
685+
680686
### Pattern
681687
682688
<!-- lore:019c972c-9f11-7c0d-96ce-3f8cc2641175 -->
683689
* **Org-scoped SDK calls follow getOrgSdkConfig + unwrapResult pattern**: All org-scoped API calls in src/lib/api-client.ts: (1) call \`getOrgSdkConfig(orgSlug)\` for regional URL + SDK config, (2) spread into SDK function: \`{ ...config, path: { organization\_id\_or\_slug: orgSlug, ... } }\`, (3) pass to \`unwrapResult(result, errorContext)\`. Shared helpers \`resolveAllTargets\`/\`resolveOrgAndProject\` must NOT call \`fetchProjectId\` — commands that need it enrich targets themselves.
684690
685691
<!-- lore:5ac4e219-ea1f-41cb-8e97-7e946f5848c0 -->
686-
* **PR workflow: wait for Seer and Cursor BugBot before resolving**: After pushing a PR in the getsentry/cli repo, the CI pipeline includes Seer Code Review and Cursor Bugbot as required or advisory checks. Both typically take 2-3 minutes. The workflow is: push → wait for all CI (including npm build jobs which test the actual bundle) → check for inline review comments from Seer/BugBot → fix if needed → repeat. Use \`gh pr checks \<PR> --watch\` to monitor. Review comments are fetched via \`gh api repos/OWNER/REPO/pulls/NUM/comments\` and \`gh api repos/OWNER/REPO/pulls/NUM/reviews\`.
692+
* **PR workflow: wait for Seer and Cursor BugBot before resolving**: After pushing a PR in the getsentry/cli repo, the CI pipeline includes Seer Code Review and Cursor Bugbot as advisory checks. Both typically take 2-3 minutes but may not trigger on draft PRs — only ready-for-review PRs reliably get bot reviews. The workflow is: push → wait for all CI (including npm build jobs which test the actual bundle) → check for inline review comments from Seer/BugBot → fix if needed → repeat. Use \`gh pr checks \<PR> --watch\` to monitor. Review comments are fetched via \`gh api repos/OWNER/REPO/pulls/NUM/comments\` and \`gh api repos/OWNER/REPO/pulls/NUM/reviews\`.
687693
688694
<!-- lore:019cb162-d3ad-7b05-ab4f-f87892d517a6 -->
689695
* **Shared pagination infrastructure: buildPaginationContextKey and parseCursorFlag**: List commands with cursor pagination use \`buildPaginationContextKey(type, identifier, flags)\` for composite context keys and \`parseCursorFlag(value)\` accepting \`"last"\` magic value. Critical: \`resolveCursor()\` must be called inside the \`org-all\` override closure, not before \`dispatchOrgScopedList\` — otherwise cursor validation errors fire before the correct mode-specific error.
690696
691697
<!-- lore:019cbd5f-ec35-7e2d-8386-6d3a67adf0cf -->
692698
* **Telemetry instrumentation pattern: withTracingSpan + captureException for handled errors**: For graceful-fallback operations, use \`withTracingSpan\` from \`src/lib/telemetry.ts\` for child spans and \`captureException\` from \`@sentry/bun\` (named import — Biome forbids namespace imports) with \`level: 'warning'\` for non-fatal errors. \`withTracingSpan\` uses \`onlyIfParent: true\` so it's a no-op without active transaction. When returning \`withTracingSpan(...)\` directly, drop \`async\` and use \`Promise.resolve(null)\` for early returns. User-visible fallbacks should use \`log.warn()\` not \`log.debug()\` — debug is invisible at default level. Also: several commands bypass telemetry by importing \`buildCommand\` from \`@stricli/core\` directly instead of \`../../lib/command.js\`. Affected: trace/list, trace/view, log/view, api.ts, help.ts.
693699
700+
<!-- lore:019cc43d-e651-7154-a88e-1309c4a2a2b6 -->
701+
* **Testing Stricli command func() bodies via spyOn mocking**: To unit-test a Stricli command's \`func()\` body: (1) \`const func = await cmd.loader()\`, (2) \`func.call(mockContext, flags, ...args)\` with mock \`stdout\`, \`stderr\`, \`cwd\`, \`setContext\`. (3) \`spyOn\` namespace imports to mock dependencies (e.g., \`spyOn(apiClient, 'getLogs')\`). The \`loader()\` return type union causes \`.call()\` LSP errors — these are false positives that pass \`tsc --noEmit\`. When API functions are renamed (e.g., \`getLog\`\`getLogs\`), update both spy target name AND mock return shape (single → array). Slug normalization (\`normalizeSlug\`) replaces underscores with dashes but does NOT lowercase — test assertions must match original casing (e.g., \`'CAM-82X'\` not \`'cam-82x'\`).
702+
694703
### Preference
695704
696705
<!-- lore:019c9700-0fc3-730c-82c3-a290d5ecc2ea -->

src/commands/api.ts

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -626,6 +626,59 @@ export function parseDataBody(
626626
}
627627
}
628628

629+
/**
630+
* Convert `--data` content to query parameters for bodyless HTTP methods
631+
* (GET, HEAD, OPTIONS).
632+
*
633+
* Handles two formats:
634+
* - URL-encoded strings: `"stat=received&resolution=1d"` → `{ stat: "received", resolution: "1d" }`
635+
* - JSON objects: `{ "stat": "received" }` → `{ stat: "received" }`
636+
*
637+
* Duplicate keys in URL-encoded strings are collected into arrays.
638+
*
639+
* @param data - Parsed output from {@link parseDataBody}
640+
* @returns Query parameter map suitable for `rawApiRequest`'s `params` option
641+
* @throws {ValidationError} When data is a JSON array (cannot be represented as query params)
642+
* @internal Exported for testing
643+
*/
644+
export function dataToQueryParams(
645+
data: Record<string, unknown> | unknown[] | string
646+
): Record<string, string | string[]> {
647+
// String data: parse as URL-encoded query string
648+
if (typeof data === "string") {
649+
const params: Record<string, string | string[]> = {};
650+
const searchParams = new URLSearchParams(data);
651+
for (const [key, value] of searchParams) {
652+
const existing = params[key];
653+
if (existing !== undefined) {
654+
params[key] = Array.isArray(existing)
655+
? [...existing, value]
656+
: [existing, value];
657+
} else {
658+
params[key] = value;
659+
}
660+
}
661+
return params;
662+
}
663+
664+
// JSON arrays can't be represented as query params
665+
if (Array.isArray(data)) {
666+
throw new ValidationError(
667+
"Cannot use --data with a JSON array for GET requests. " +
668+
"JSON arrays cannot be converted to query parameters. " +
669+
"Use --method POST or pass individual values with --field/-F.",
670+
"data"
671+
);
672+
}
673+
674+
// JSON object: stringify non-string values
675+
const params: Record<string, string | string[]> = {};
676+
for (const [key, value] of Object.entries(data)) {
677+
params[key] = typeof value === "string" ? value : JSON.stringify(value);
678+
}
679+
return params;
680+
}
681+
629682
/**
630683
* Try to parse a single field as a bare JSON **object or array** body.
631684
*
@@ -980,7 +1033,14 @@ export async function resolveBody(
9801033
}
9811034

9821035
if (flags.data !== undefined) {
983-
return { body: parseDataBody(flags.data) };
1036+
const parsed = parseDataBody(flags.data);
1037+
1038+
// GET/HEAD/OPTIONS cannot have a body — convert data to query params
1039+
if (flags.method === "GET") {
1040+
return { params: dataToQueryParams(parsed) };
1041+
}
1042+
1043+
return { body: parsed };
9841044
}
9851045

9861046
if (flags.input !== undefined) {

test/commands/api.test.ts

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
buildQueryParams,
1515
buildQueryParamsFromFields,
1616
buildRawQueryParams,
17+
dataToQueryParams,
1718
extractJsonBody,
1819
handleResponse,
1920
normalizeEndpoint,
@@ -1575,4 +1576,89 @@ describe("resolveBody", () => {
15751576
expect(result.body).toBeUndefined();
15761577
expect(result.params).toEqual({ query: "is:unresolved" });
15771578
});
1579+
1580+
test("GET --data converts URL-encoded string to query params", async () => {
1581+
const stderr = createMockWriter();
1582+
const result = await resolveBody(
1583+
{ method: "GET", data: "stat=received&resolution=1d" },
1584+
MOCK_STDIN,
1585+
stderr
1586+
);
1587+
expect(result.body).toBeUndefined();
1588+
expect(result.params).toEqual({ stat: "received", resolution: "1d" });
1589+
});
1590+
1591+
test("GET --data converts JSON object to query params", async () => {
1592+
const stderr = createMockWriter();
1593+
const result = await resolveBody(
1594+
{ method: "GET", data: '{"stat":"received","resolution":"1d"}' },
1595+
MOCK_STDIN,
1596+
stderr
1597+
);
1598+
expect(result.body).toBeUndefined();
1599+
expect(result.params).toEqual({ stat: "received", resolution: "1d" });
1600+
});
1601+
1602+
test("GET --data with JSON array throws ValidationError", async () => {
1603+
const stderr = createMockWriter();
1604+
await expect(
1605+
resolveBody({ method: "GET", data: "[1,2,3]" }, MOCK_STDIN, stderr)
1606+
).rejects.toThrow(ValidationError);
1607+
await expect(
1608+
resolveBody({ method: "GET", data: "[1,2,3]" }, MOCK_STDIN, stderr)
1609+
).rejects.toThrow(/cannot.*query parameters/i);
1610+
});
1611+
1612+
test("POST --data still returns body (regression guard)", async () => {
1613+
const stderr = createMockWriter();
1614+
const result = await resolveBody(
1615+
{ method: "POST", data: '{"status":"resolved"}' },
1616+
MOCK_STDIN,
1617+
stderr
1618+
);
1619+
expect(result.body).toEqual({ status: "resolved" });
1620+
expect(result.params).toBeUndefined();
1621+
});
1622+
});
1623+
1624+
// -- dataToQueryParams: converts parsed --data to query params for GET --
1625+
1626+
describe("dataToQueryParams", () => {
1627+
test("parses URL-encoded string", () => {
1628+
expect(dataToQueryParams("stat=received&resolution=1d")).toEqual({
1629+
stat: "received",
1630+
resolution: "1d",
1631+
});
1632+
});
1633+
1634+
test("handles duplicate keys as arrays", () => {
1635+
expect(dataToQueryParams("tag=foo&tag=bar&tag=baz")).toEqual({
1636+
tag: ["foo", "bar", "baz"],
1637+
});
1638+
});
1639+
1640+
test("handles empty string", () => {
1641+
expect(dataToQueryParams("")).toEqual({});
1642+
});
1643+
1644+
test("converts JSON object with string values", () => {
1645+
expect(dataToQueryParams({ stat: "received", resolution: "1d" })).toEqual({
1646+
stat: "received",
1647+
resolution: "1d",
1648+
});
1649+
});
1650+
1651+
test("stringifies non-string JSON values", () => {
1652+
expect(dataToQueryParams({ count: 5, enabled: true })).toEqual({
1653+
count: "5",
1654+
enabled: "true",
1655+
});
1656+
});
1657+
1658+
test("throws on JSON array", () => {
1659+
expect(() => dataToQueryParams([1, 2, 3])).toThrow(ValidationError);
1660+
expect(() => dataToQueryParams([1, 2, 3])).toThrow(
1661+
/cannot.*JSON array.*query parameters/i
1662+
);
1663+
});
15781664
});

0 commit comments

Comments
 (0)