You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
fix(api): guard listOrganizations against non-array SDK responses (CLI-1CQ) (#854)
## Summary
Fixes [CLI-1CQ](https://sentry.sentry.io/issues/7429871167/):
`TypeError: l.map is not a function` in `listOrganizationsUncached` on
self-hosted Sentry instances.
## Root cause
The `@sentry/api` SDK returns `data = {}` (not `[]`) when:
- Response body is empty
- `Content-Length: 0`
- Status 204
It also returns a `ReadableStream` when `Content-Type` is missing.
`unwrapResult` returns `data` as-is, and the `as unknown as
SentryOrganization[]` cast in `listOrganizationsInRegion` silently lies.
The crash surfaces in the self-hosted fallback path of
`listOrganizationsUncached` when `.map()` is called on the non-array
result.
Self-hosted instances behind reverse proxies (nginx, Cloudflare, WAFs)
commonly trigger this by stripping bodies or wrapping responses.
## Fix
Add an `Array.isArray` guard in `listOrganizationsInRegion` after
`unwrapResult`. If the data is not an array, throw a descriptive
`ApiError` that names the region URL and points at the likely cause
(incompatible self-hosted version or proxy interference). This protects
both the self-hosted path and the multi-region fan-out path, since both
go through `listOrganizationsInRegion`.
## Tests
Three new tests in `test/lib/api/organizations.test.ts`:
- valid array response works as before
- empty object `{}` body throws `ApiError` with the right message +
region URL
- empty body throws `ApiError`
* **Sentry CLI resolve-target cascade has 5 priority levels with env var support**: resolve-target.ts cascade has 5 priority levels: (1) Explicit CLI flags, (2) SENTRY\_ORG/SENTRY\_PROJECT env vars, (3) SQLite config defaults, (4) DSN auto-detection, (5) Directory name inference. SENTRY\_PROJECT supports combo notation \`org/project\` — when used, SENTRY\_ORG is ignored. If combo parse fails (e.g. \`org/\`), the entire value is discarded. \`resolveFromEnvVars()\` helper is injected into all four resolution functions.
* **Bun bytecode: true crashes esbuild→compile ESM bundles (Bun 1.3.11)**: Bun build flags for compiled CLI (\`script/build.ts\`): (1) Do NOT enable \`bytecode: true\` with esbuild→\`Bun.build({ compile })\` pipeline (Bun 1.3.11). Crashes with \`TypeError: Expected CommonJS module to have a function wrapper\`, exit 0, no output. Upstream: oven-sh/bun#21097, #23490. (2) Pass \`autoloadDotenv: false\` and \`autoloadBunfig: false\` — otherwise a user's \`.env\`/\`bunfig.toml\` silently injects into \`process.env\` (e.g., Next.js \`.env.local\` could override stored OAuth token). Shell env vars still work; suggest direnv for dir-scoped vars.
* **Bun mock.module() leaks globally across test files in same process**: Bun's mock.module() replaces modules globally and leaks across test files in the same process. Solution: tests using mock.module() must run in a separate \`bun test\` invocation. In package.json, use \`bun run test:unit && bun run test:isolated\` instead of \`bun test\`. The \`test/isolated/\` directory exists for these tests. This was the root cause of ~100 test failures (getsentry/cli#258).
* **@sentry/api SDK can return non-array data for empty/edge responses**: \`@sentry/api\` SDK (in \`node\_modules/@sentry/api/dist/index.js\`) returns \`data = {}\` (not \`\[]\`) when response body is empty, has \`Content-Length: 0\`, or status 204; and returns a \`ReadableStream\` when \`Content-Type\` is missing. \`unwrapResult\` from \`src/lib/api/infrastructure.ts\` returns \`data\` as-is, and \`as unknown as SentryX\[]\` casts silently lie. Always guard array-typed SDK results with \`Array.isArray(data)\` before \`.map()\` — applied in \`listOrganizationsInRegion\` (CLI-1CQ). Self-hosted instances behind reverse proxies (nginx, Cloudflare, WAFs) commonly trigger this by stripping bodies or wrapping responses. Throw a descriptive \`ApiError\` on mismatch rather than letting \`TypeError: x.map is not a function\` bubble up minified.
* **Bun test files share globalThis.fetch — mock counters leak across file boundaries**: Bun runs all test files in one process with shared \`globalThis.fetch\`. A test that swaps\`globalThis.fetch = myCountingMock\`and asserts on \`callCount\` can see foreign calls from async work leaked by earlier test files (e.g. the CLI's own Sentry telemetry, or pending retries that outlive their test's \`afterEach\`). CI flake symptom: \`expect(callCount).toBe(2)\` fails with\`Received: 7\`, with debug logs showing stray URLs like \`/api/0/organizations/1/\` and \`/api/0/projects/1/4510776311808000/\`(the CLI's telemetry project ID) between your expected calls. Fix: scope every fetch mock to a per-test URL marker and delegate foreign URLs to the captured \`originalFetch\` (preload.ts blocker). Pattern \`scopedFetchMock(marker, handler)\`lives in \`test/lib/sentry-client.test.ts\`. Reference: PR #832, CI run 24835339085.
* **Bun bytecode: true crashes esbuild→compile ESM bundles (Bun 1.3.11)**: Bun build flags for compiled CLI (\`script/build.ts\`): (1) Do NOT enable\`bytecode: true\`with esbuild→\`Bun.build({ compile })\` pipeline. Still broken on Bun 1.3.13 — crashes \`TypeError: Expected CommonJS module to have a function wrapper\` at entry.instantiate (esbuild emits ESM; bytecode loader mis-caches as CJS). Exit 0, no output. Upstream: oven-sh/bun#21097, #23490. (2) Pass \`autoloadDotenv: false\` and\`autoloadBunfig: false\` — otherwise user's \`.env\`/\`bunfig.toml\`silently injects into \`process.env\` (e.g. Next.js \`.env.local\`could override stored OAuth token). Shell env vars still work; suggest direnv for dir-scoped vars.
* **dist/bin.cjs runtime Node version check must match engines.node**: Node 20 dropped; \`engines.node >=22.12\` matches Astro 6 floor. CI \`Build npm Package\` matrix \`\["22","24"]\`. Docs build jobs pin \`actions/setup-node@v6\` with \`node-version: "24"\` after \`setup-bun\` for astro's node shebang. The npm package's \`dist/bin.cjs\` (from \`script/bundle.ts\`) contains an inline Node guard that MUST match \`engines.node\`. Simple \`parseInt(process.versions.node) < 22\` misses 22.0.0–22.11.x — use explicit major+minor: \`let v=process.versions.node.split('.').map(Number);if(v\[0]<22||(v\[0]===22&\&v\[1]<12)){...}\`. When bumping, update BIN\_WRAPPER string AND error message in lockstep. Without \`engine-strict=true\`, npm only warns — the runtime guard is real enforcement.
* **script/generate-api-schema.ts regex is brittle against SDK bundler output changes**: \`script/generate-api-schema.ts\` parses \`node\_modules/@sentry/api/dist/index.js\` with a regex (\`/var (\w+) = \\(options\S\*\\) => \\(options\S\*client \\?\\? client\\)\\.(\w+)\\(/g\`) to map SDK function names to URL+method pairs, producing \`src/generated/api-schema.json\`. If the SDK changes its generator's bundle format (e.g., switches to \`const\`, arrow vs function, different client-selection pattern), schema generation silently produces empty \`fn\` fields. When bumping \`@sentry/api\`, verify \`sentry schema\` output still populates function names. \`src/generated/api-schema.json\` is gitignored — regenerates on every dev/build/typecheck via \`bun run generate:schema\`.
* **Sentry /auth/ endpoint returns 400 (not 401) for Bearer tokens pre-fix**: Sentry \`/auth/\` endpoint historically returned \*\*400 with empty body\*\* for valid Bearer tokens — \`AuthIndexEndpoint\` excluded \`UserAuthTokenAuthentication\`. Fixed server-side by getsentry/sentry#112853 (release 26.4.1, Apr 22 2026), now fully rolled out to SaaS. Fix adds \`UserAuthTokenAuthentication\` only — \`OrgAuthTokenAuthentication\` was NOT added, and \`UserAuthTokenAuthentication.accepts\_auth\` rejects \`sntrys\_\` prefix. So \`sntrys\_\` org-auth-tokens always fail on \`/auth/\` (and whoami is semantically meaningless for org tokens — no user). CLI short-circuits \`sntrys\_\` via \`classifySentryToken\` before the call, throwing \`ResolutionError\` with \`sentry auth status\` hint. The 400-translation path in \`translateWhoamiApiError\` was dropped post-rollout (PR #841) as vestigial — anomalous 400s now bubble as \`ApiError\` and are diagnosable via the \`api\_response\_headers\` Sentry context. \`sentry auth status\` verifies via \`listOrganizationsUncached()\` so it works with org tokens.
* **Source Map v3 spec allows null entries in sources array**: The Source Map v3 spec allows \`null\` entries in the \`sources\` array, and bundlers like esbuild actually produce them. Any code iterating over \`sources\` and calling string methods (e.g., \`.replaceAll()\`) must guard against null: \`map.sources.map((s) => typeof s === "string" ? s.replaceAll("\\\\", "/") : s)\`. Without the guard, \`null.replaceAll()\` throws \`TypeError\`. This applies to \`src/lib/sourcemap/debug-id.ts\` and any future sourcemap manipulation code.
* **Non-essential DB cache writes should be guarded with try-catch**: Non-essential DB cache writes (e.g., \`setUserInfo()\`, \`setInstallInfo()\`) must be wrapped in try-catch so a broken/read-only DB doesn't crash a command whose primary operation succeeded. Pattern: \`try { setInstallInfo(...) } catch { log.debug(...) }\`. In login.ts, \`getCurrentUser()\` failure after token save must not block auth — log warning, continue. In upgrade.ts, \`setInstallInfo\` after legacy detection is guarded same way. Exception: \`getUserRegions()\` failure should \`clearAuth()\` and fail hard (indicates invalid token). This is enforced by BugBot reviews — any \`setInstallInfo\`/\`setUserInfo\` call outside setup.ts's \`bestEffort()\` wrapper needs its own try-catch.
* **Regenerating @sentry/core + @sentry/node-core patches for SDK version bumps**: Bumping sentry-javascript exact-pinned version: (1) Delete old \`patches/@sentry%2F{core,node-core}@OLD.patch\` and remove \`patchedDependencies\` entries from \`package.json\`. (2) Bump \`@sentry/node-core\` in devDependencies, \`bun install\`. (3) \`bun patch @sentry/node-core\`, edit \`node\_modules/@sentry/node-core/build/{cjs,esm}/light/index.js\` to strip unused exports, \`bun patch --commit\`. (4) Repeat for \`@sentry/core\` editing \`build/{cjs,esm}/index.js\` — strip unused \`require()\`s AND their \`exports.X = Y;\` lines in CJS, strip names from single-line ESM export. (5) Verify with \`bun install && bun run typecheck && bun test\`. \*\*Critical\*\*: before stripping any \`core\` export, grep \`node-core/build/{cjs,esm}/light/sdk.js\` for runtime usage — e.g. 10.50.0+ calls \`spanStreamingIntegration()\` at runtime when \`traceLifecycle === 'stream'\`; stripping causes \`SyntaxError: Export named 'spanStreamingIntegration' not found\` on first \`Sentry.init()\`. Safe to strip from node-core/light re-export surface. \*\*Also\*\*: when running \`bun patch --commit\`, remove any \`.bun-tag-\<hash>\` hunks at the top of the generated patch file — they embed a stray empty marker file into every install (\`node\_modules/@sentry/core/.bun-tag-\*\`). Bun creates its own tag on install regardless; the one from the patch is redundant and noisy.
* **Sentry CLI command docs are auto-generated from Stricli route tree with CI freshness check**: Sentry CLI command docs are auto-generated from Stricli route tree: Docs in \`docs/src/content/docs/commands/\*.md\` and skill files in \`plugins/sentry-cli/skills/sentry-cli/references/\*.md\` are generated via \`bun run generate:docs\`. Content between \`\<!-- GENERATED:START/END -->\` markers is regenerated; hand-written examples go in \`docs/src/fragments/commands/\`. CI checks \`check:command-docs\` and \`check:skill\` fail if stale. Run generators after changing command parameters/flags/docs.
* **Stricli buildCommand output config injects json flag into func params**: Stricli command gotchas: (1) In \`func()\` handlers use \`this.stdout\`/\`this.stderr\` directly — NOT \`this.process.stdout\`. \`SentryContext\` has \`process\` and \`stdout\`/\`stderr\` as separate top-level properties; test mocks omit full \`process\` so \`this.process.stdout\` throws \`TypeError\` at runtime (TS doesn't catch). (2) \`output: { json: true, human: formatFn }\` auto-injects \`--json\`/\`--fields\` flags — type flags explicitly (\`flags: { json?: boolean }\`). Commands with interactive side effects (prompts, QR codes) should check \`flags.json\` and skip. (3) Route maps with \`defaultCommand\` blend the default command's flags into subcommand completions — completion tests must track \`hasDefaultCommand\` and skip strict subcommand-matching.
* **Token-type classification via literal prefix match (classifySentryToken)**: \`src/lib/token-type.ts\`\`classifySentryToken(token)\` returns \`'org-auth-token'\` (\`sntrys\_\` prefix), \`'user-auth-token'\` (\`sntryu\_\` prefix), or \`'oauth-or-legacy'\` (OAuth, legacy PATs, JWT-shaped). Case-sensitive literal \`startsWith\` — matches Sentry backend's \`SENTRY\_ORG\_AUTH\_TOKEN\_PREFIX\`. Use to short-circuit commands where a token type is semantically invalid (e.g. \`whoami\` with org token) before a confusing API failure. \`getAuthToken()\` from \`db/auth\` returns the effective token (env + DB fallback).
* **PR workflow: address review comments, resolve threads, wait for CI**: PR workflow: (1) wait for CI; (2) check unresolved review comments via \`gh api repos/.../pulls/N/comments\`; (3) fix in follow-up commits (not amends); (4) reply explaining fix; (5) resolve thread via \`gh api graphql resolveReviewThread\`; (6) push + re-check CI. BugBot/Seer/Warden/Cursor bots post new comments per-commit and frequently catch real bugs in fix commits themselves — always re-check after each push. \*\*Dispatch a subagent review before declaring merge-ready\*\* — self-assessment is unreliable; has caught real backwards-compat and error-path bugs. After applying review fixes, plan for ANOTHER critical review pass on the HEAD commit. Branches: \`fix/\*\` or \`feat/\*\`. Style: \`Array.from(set)\` over spreads; 'allowlist' not 'whitelist'; \`arr.at(-1)\` over index math. Reviewer questions may be inquiries — confirm intent before changing. Multi-fix PRs: split into independent PRs off \`main\`.
1075
+
* **PR workflow: address review comments, resolve threads, wait for CI**: PR workflow: (1) wait for CI; (2) check unresolved comments via \`gh api repos/.../pulls/N/comments\`; (3) fix in follow-up commits (NEVER amend a pushed commit without explicit user request + force push); (4) reply explaining fix; (5) resolve thread via \`gh api graphql resolveReviewThread\`; (6) push + re-check CI. BugBot/Seer/Warden/Cursor post new comments per-commit and often catch bugs in fix commits — re-check after each push. Dispatch a subagent review before declaring merge-ready; has caught real backwards-compat, error-path, and patch-hygiene bugs. Branches: \`fix/\*\` or \`feat/\*\`. Style: \`Array.from(set)\` over spreads; 'allowlist' not 'whitelist'; \`arr.at(-1)\` over index math. Reviewer questions may be inquiries — confirm intent before changing.
0 commit comments