From ced9182ed48abad7fad3092d17f48c292f12d6c5 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Wed, 20 May 2026 17:02:16 +0000 Subject: [PATCH] fix: use Node 22 in E2E CI and simplify zstd imports - Add setup-node@v6 with node-version 22 to E2E test job (Node 20 is EOL) - Replace feature-detected zstd access with direct imports from node:zlib (zstdCompress/zstdDecompress are available in Node 22.15+) - Remove unnecessary runtime fallback code and type gymnastics - Update stale Bun reference in zstd-transport comment --- .github/workflows/ci.yml | 5 ++- .lore.md | 62 +++++++++++++++++++++++++++++ src/lib/api/sourcemaps.ts | 29 +++++--------- src/lib/telemetry/zstd-transport.ts | 33 +++++---------- 4 files changed, 86 insertions(+), 43 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 490970d3c..6fc9cf794 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -629,10 +629,13 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v6 - - uses: oven-sh/setup-bun@v2 + - uses: oven-sh/setup-bun@v2 with: bun-version: "1.3.13" - uses: pnpm/action-setup@v4 + - uses: actions/setup-node@v6 + with: + node-version: "22" - uses: actions/cache@v5 id: cache with: diff --git a/.lore.md b/.lore.md index 93d44ae15..a4bc6dc43 100644 --- a/.lore.md +++ b/.lore.md @@ -4,9 +4,18 @@ ### Architecture + +* **@sentry/api SDK integration: type wrapping pattern and pagination helpers**: @sentry/api SDK integration: wrap SDK types at \`src/lib/api/\*.ts\` with \`as unknown as SentryX\` casts; never leak to commands. \`src/types/sentry.ts\` uses \`Partial\ & RequiredCore\`. Avoid circular deps by importing SDK functions directly in \`src/lib/region.ts\`. \`unwrapResult\`/\`unwrapPaginatedResult\` must stay CLI-owned (SDK versions throw plain \`Error\`). Body-shape casts use \`Parameters\\[0]\["body"]\`. + + +* **apiRequestToRegion/rawApiRequest options shape — no timeout/signal/headers on the typed API**: apiRequestToRegion/rawApiRequest expose only \`{ method, body, params, schema }\` (typed API). \`rawApiRequest\` adds \`headers?\`. Neither exposes \`timeout\`/\`signal\`—callers pass \`(url, init: RequestInit)\` to authenticated fetch. \`apiRequestToRegion\` auto-sets JSON Content-Type; \`rawApiRequest\` preserves strings. 204/205 throw \`ApiError\` not \`TypeError\`. + * **Auth token env var override pattern: SENTRY\_AUTH\_TOKEN > SENTRY\_TOKEN > SQLite**: Auth in \`src/lib/db/auth.ts\` follows layered precedence: \`SENTRY\_AUTH\_TOKEN\` > \`SENTRY\_TOKEN\` > SQLite OAuth token. \`getEnvToken()\` trims env vars (empty/whitespace = unset). \`AuthSource\` tracks provenance. \`ENV\_SOURCE\_PREFIX = "env:"\` — use \`.length\` not hardcoded 4. Env tokens bypass refresh/expiry. \`isEnvTokenActive()\` guards auth commands. Logout must NOT clear stored auth when env token active. These functions stay in \`db/auth.ts\` despite not touching DB because they're tightly coupled with token retrieval. + +* **Completion fast-path skips Sentry SDK via SENTRY\_CLI\_NO\_TELEMETRY and SQLite telemetry queue**: Completion fast-path: Shell completions set \`SENTRY\_CLI\_NO\_TELEMETRY=1\` in \`bin.ts\` before imports, skipping \`createTracedDatabase\` and \`@sentry/node-core/light\` load (~85ms). Timing queued to \`completion\_telemetry\_queue\` SQLite table; normal runs drain via \`DELETE ... RETURNING\`. Achieves ~60ms dev / ~140ms CI within 200ms budget. + * **Consola chosen as CLI logger with Sentry createConsolaReporter integration**: Consola is the CLI logger with Sentry \`createConsolaReporter\` integration. Two reporters: FancyReporter (stderr) + Sentry structured logs. Level via \`SENTRY\_LOG\_LEVEL\`. \`buildCommand\` injects hidden \`--log-level\`/\`--verbose\` flags. \`withTag()\` creates independent instances; \`setLogLevel()\` propagates via registry. All user-facing output must use consola, not raw stderr. \`HandlerContext\` intentionally omits stderr. @@ -28,12 +37,18 @@ * **Magic @ selectors resolve issues dynamically via sort-based list API queries**: Magic @ selectors resolve issues dynamically: \`@latest\`, \`@most\_frequent\` in \`parseIssueArg\` detected before \`validateResourceId\` (@ not in forbidden charset). \`SELECTOR\_MAP\` provides case-insensitive matching. \`resolveSelector\` maps to \`IssueSort\` values, calls \`listIssuesPaginated\` with \`perPage: 1\`, \`query: 'is:unresolved'\`. Supports org-prefixed: \`sentry/@latest\`. Unrecognized \`@\`-prefixed strings fall through. \`ParsedIssueArg\` union includes \`{ type: 'selector' }\`. + +* **Project cache is org-scoped with three key formats and three population paths**: Project cache (\`project\_cache\` table) uses three key shapes: \`{orgId}:{projectId}\` (DSN), \`dsn:{publicKey}\` (DSN-only), \`list:{orgSlug}/{projectSlug}\` (batch). \`getCachedProjectBySlug\` queries all three for hot-path lookups. Population: DSN resolution, \`listProjects()\` batch, \`fetchProjectId\` seed. Resolution errors use live API via \`findSimilarProjectsAcrossOrgs\` — no cross-org cache search. + * **safe-read.ts wraps isRegularFile + Bun.file().text() for FIFO-safe user-path reads**: \`src/lib/safe-read.ts\` \`safeReadFile(path, operation): Promise\\` combines \`isRegularFile()\` + \`Bun.file().text()\` + broad error swallow (FIFO/ENOENT/EACCES/EPERM/EISDIR/ENOTDIR). Sole caller: \`apply-patchset.ts\`. \*\*Do NOT use for committed config loads\*\* — swallows EPERM/EISDIR, making \`chmod 000 .sentryclirc\` manifest as confusing 'no auth token'. For loud permission surfacing (\`tryReadSentryCliRc\`), call \`fs.promises.stat\` directly, gate on \`isFile()\`, catch only ENOENT/EACCES. \`read-files.ts\`/\`workflow-inputs.ts\` use direct stat to reuse one stat for size-gating. Test with real \`mkfifo\` + short timeout as hang detector. * **Seer trial prompt uses middleware layering in bin.ts error handling chain**: Seer trial prompt via error middleware layering: \`bin.ts\` chain is \`main() → executeWithAutoAuth() → executeWithSeerTrialPrompt() → runCommand()\`. Seer trial prompts (\`no\_budget\`/\`not\_enabled\`) caught by inner wrapper; auth errors bubble to outer. Trial API: \`GET /api/0/customers/{org}/\` → \`productTrials\[]\` (prefer \`seerUsers\`, fallback \`seerAutofix\`). Start: \`PUT /api/0/customers/{org}/product-trial/\`. SaaS-only; self-hosted 404s gracefully. \`ai\_disabled\` excluded. \`startSeerTrial\` accepts \`category\` from trial object — don't hardcode. + +* **Sentry API: events require org+project, issues have legacy global endpoint**: Sentry API quirks: (1) Events need org+project (\`/projects/{org}/{project}/events/{id}/\`); issues use legacy global \`/api/0/issues/{id}/\`; traces need org only. (2) \`/users/me/\` returns 403 for OAuth — use \`/auth/\` instead via \`getControlSiloUrl()\`. (3) Chunk upload endpoint returns camelCase (\`chunkSize\`, etc.) — exception to snake\_case convention. + * **Sentry CLI authenticated fetch architecture with response caching**: (architecture) Authenticated fetch + response cache: \`createAuthenticatedFetch\`: auth headers, 30s timeout, max 2 retries, 401 refresh, span tracing. \`buildAttemptFactory\` clones \`Request\`; do NOT materialize FormData (strips boundary). Per-endpoint timeout overrides (e.g. \`/autofix/\` 120s). Response cache RFC 7234 at \`~/.sentry/cache/responses/\`, GET 2xx only. TTL tiers: stable=5min, volatile=60s, immutable=24h. \`@sentry/api\` SDK passes Request with no init — undefined init → empty headers stripping Content-Type (HTTP 415); fall back to \`input.headers\` when init undefined. Guard \`Array.isArray(data)\` before \`.map()\` (SDK returns \`{}\` for 204/empty). GET response cache checked BEFORE fetch — tests asserting call counts see 0 calls if prior test cached same URL. @@ -57,11 +72,32 @@ * **All view subcommands should use \ \ positional pattern**: All \`\* view\` subcommands use \`\ \\` positional pattern (Intent-First Correction UX): target is optional \`org/project\`. Use opportunistic arg swapping with \`log.warn()\` when args are wrong order — when intent is unambiguous, do what they meant. Normalize at command level, keep parsers pure. Model after \`gh\` CLI. Exception: \`auth\` uses \`defaultCommand: "status"\` (no viewable entity). Routes without defaults: \`cli\`, \`sourcemap\`, \`repo\`, \`team\`, \`trial\`, \`release\`, \`dashboard/widget\`. + +* **Issue list global limit with fair per-project distribution and representation guarantees**: \`issue list --limit\` is a global total across all detected projects. \`fetchWithBudget\` Phase 1 divides evenly, Phase 2 redistributes surplus via cursor resume. \`trimWithProjectGuarantee\` ensures ≥1 issue per project before filling remaining slots. JSON output wraps in \`{ data, hasMore }\` with optional \`errors\` array. Compound cursor (pipe-separated) enables \`-c last\` for multi-target pagination, keyed by sorted target fingerprint. + + +* **Prefer dedicated SQLite tables + migrations over metadata KV for non-trivial caches**: Prefer dedicated SQLite tables + migrations over \`metadata\` KV for non-trivial caches. Schema migrations are cheap; don't shoehorn structured data into KV with dotted-prefix keys. Dedicated tables give clearer schema, proper indexes, simpler bulk-clear. \`metadata\` KV fine for small scalars (defaults.\*, install.\*). Example: \`issue\_org\_cache\` (v15) replaced \`metadata\` keys. + + +* **Raw markdown output for non-interactive terminals, rendered for TTY**: Markdown-first output pipeline: custom renderer in \`src/lib/formatters/markdown.ts\` walks \`marked\` tokens to produce ANSI-styled output. Commands build CommonMark using helpers (\`mdKvTable()\`, \`mdRow()\`, \`colorTag()\`, \`escapeMarkdownCell()\`, \`safeCodeSpan()\`) and pass through \`renderMarkdown()\`. \`isPlainOutput()\` precedence: \`SENTRY\_PLAIN\_OUTPUT\` > \`NO\_COLOR\` > \`FORCE\_COLOR\` > \`!isTTY\`. \`--json\` always outputs JSON. Colors defined in \`COLORS\` object in \`colors.ts\`. Tests run non-TTY so assertions match raw CommonMark; use \`stripAnsi()\` helper for rendered-mode assertions. + ### Gotcha + +* **@sentry/api SDK can return non-array data for empty/edge responses**: @sentry/api SDK returns \`{}\` (not \`\[]\`) for empty/204 responses and \`ReadableStream\` when \`Content-Type\` missing. \`as unknown as SentryX\[]\` casts silently lie. Always guard with \`Array.isArray(data)\` before \`.map()\`. Self-hosted behind proxies (nginx, Cloudflare) trigger this. Throw descriptive \`ApiError\` rather than \`TypeError: x.map\`. + + +* **API tests must use useTestConfigDir to isolate disk response cache**: API tests that mock \`globalThis.fetch\` MUST call \`useTestConfigDir()\` from \`test/helpers.ts\` + \`setAuthToken()\`. The \`authenticatedFetch\` singleton in \`src/lib/sentry-client.ts\` checks a filesystem-based response cache (\`~/.sentry/cache/responses/\`, see \`response-cache.ts\`) BEFORE calling fetch. Without per-test config dirs, test N's API response gets cached to disk and served to test N+1 — fetch mock never fires, assertion sees stale data. TTL tiers in \`classifyUrl()\`: stable=5min (default), volatile=60s (issues, logs), immutable=24h (events/traces by ID). Symptom: test expects fresh mock value, receives prior test's value. Reference: \`test/lib/api/issues.test.ts\` (correct pattern), \`test/lib/api/repositories.test.ts\` regression fixed by adding \`useTestConfigDir("repo-cache-")\` + \`setAuthToken("test-token", 3600, "test-refresh")\` in beforeEach. + * **Biome noUselessUndefined also rejects () => {} empty arrow callbacks**: (gotcha) Biome lint traps (run \`bun run lint\` not \`lint:fix\` before pushing): (1) \`noUselessUndefined\`+\`noEmptyBlockStatements\` reject \`()=>undefined\` and \`()=>{}\` — use \`function noop():void{}\`. (2) \`noExcessiveCognitiveComplexity\` caps at 15 — extract helpers. (3) \`noPrecisionLoss\` on int >2^53 — use \`Number(string)\`. (4) \`noIncrementDecrement\` — use \`i+=1\`. (5) \`useYield\` on \`async \*func()\` needs \`biome-ignore\`. (6) Plugin forbids raw \`metadata\` table queries — use \`getMetadata\`/\`setMetadata\`/\`clearMetadata\`. (7) \`noMisplacedAssertion\` fires on helper functions — use inline \`biome-ignore\` above each \`expect()\`, NOT file-level. (8) \`AuthError(reason, message?)\` — easy to swap args; correct: \`new AuthError("expired", "Token expired")\`. Tests aren't type-checked but ARE lint-checked. Node polyfill (\`script/node-polyfills.ts\`) is INCOMPLETE — prefer \`node:fs/promises\` for file ops; \`execSync\` for shell. + +* **dist/bin.cjs runtime Node version check must match engines.node**: dist/bin.cjs runtime Node check: engines.node >=22.12 matches Astro 6 floor. CI builds \`\["22","24"]\`; npm package's bin.cjs must match. Don't use \`parseInt(node\_version) < 22\` — it misses 22.0.0–22.11.x. Use: \`let v=process.versions.node.split('.').map(Number);if(v\[0]<22||(v\[0]===22&\&v\[1]<12))\`. Update BIN\_WRAPPER in lockstep. + + +* **MastraClient has no dispose API — use AbortController for cleanup**: MastraClient has no \`close()\`/\`dispose()\` API — cleanup via \`ClientOptions.abortSignal\` (constructor) or per-prompt \`signal\`. Without explicit abort, Bun's fetch dispatcher keep-alive sockets hold the event loop alive past natural exit. Pattern in \`src/lib/init/wizard-runner.ts\`: create \`AbortController\` per \`runWizard\`, pass \`abortSignal: controller.signal\` to \`new MastraClient(...)\`, abort via \`using \_ = { \[Symbol.dispose]: () => controller.abort() }\`. Custom \`fetch\` wrapper must preserve \`init.signal\` via spread. Tests capture \`ClientOptions\` via \`spyOn(MastraClient.prototype, 'getWorkflow').mockImplementation(function() { capturedOpts.push(this.options); ... })\`. + * **process.stdin.isTTY unreliable in Bun — use isatty(0) and backfill for clack**: \`process.stdin.isTTY\` unreliable in Bun — use \`isatty(0)\` from \`node:tty\`. Bun's single-file binary can leave \`process.stdin.isTTY === undefined\` on TTY fds inherited via redirects like \`exec … \ * **Grouped widget --limit auto-default via applyGroupLimitAutoDefault helper**: Dashboard widget flag normalization: (1) Dataset aliases (errors→error-events) normalize ONCE at top of \`func()\` via \`normalizeDataset()\` in \`src/commands/dashboard/resolve.ts\`. In \`edit.ts\`, pass \`normalizedFlags\` to \`buildReplacement\` — \`validateAggregateNames\` reads \`flags.dataset\` and rejects valid aggregates like \`failure\_rate\` if it sees raw alias. (2) Grouped widgets need \`limit\` (API rejects). \`applyGroupLimitAutoDefault\` defaults to \`DEFAULT\_GROUP\_BY\_LIMIT=5\` only when user passed \`--group-by\` without \`--limit\`; skip for auto-defaulted columns like \`\["issue"]\`. (3) Tests asserting \`--limit\` >10 survives into PUT body must use \`display: "line"\` — \`prepareWidgetQueries\` clamps bar/table to max=10. + +* **Hidden --org/--project compat flags via mergeGlobalFlags**: Hidden global \`--org\`/\`--project\` flags accept old \`sentry-cli\` syntax. Defined in \`GLOBAL\_FLAGS\` (global-flags.ts) so argv-hoist relocates them. \`mergeGlobalFlags()\` in command.ts injects hidden flag shapes (skip if command owns the flag — e.g. \`release create --project -p\`) and returns \`stripKeys\` set used by \`cleanRawFlags\`. \`applyOrgProjectFlags()\` writes values to \`SENTRY\_ORG\`/\`SENTRY\_PROJECT\` via \`getEnv()\` before auth guard, overwriting existing env vars (explicit CLI > env var). Resolution chain in resolve-target.ts picks them up at priority #2. No short aliases (\`-p\` conflicts). The helper extraction was needed to keep \`buildCommand\` under Biome's cognitive complexity limit of 15. + * **Merging mock.module() test files with static-import counterparts**: (pattern) Bun test mocking traps: (1) \`mock.module()\` for CJS built-ins needs \`default\` re-export + named exports, declared top-level BEFORE \`await import()\`. (2) Convert code-under-test to \`await import()\` when merging mocks — static imports won't re-bind. (3) \`Bun.mmap()\` always PROT\_WRITE — use \`new Uint8Array(await Bun.file(path).arrayBuffer())\` for read-only. (4) \`mock.module()\` pollutes registry — use \`test/isolated/\`. (5) \`buildCommand\` wrapper: \`cmd.loader()\` returns wrapped async fn; call \`func.call(ctx, flags, ...args)\` as a promise. Auth guard runs first; \`test/preload.ts\` sets fake \`SENTRY\_AUTH\_TOKEN\`. (6) Test glob \`test:unit\` only picks up \`test/lib\`, \`test/commands\`, \`test/types\` — tests under \`test/fixtures/\`, \`test/scripts/\`, \`test/script/\` NOT run by CI. (7) Tests mocking fetch MUST call \`useTestConfigDir()\` + \`setAuthToken()\` + \`resetCacheState()\` + \`disableResponseCache()\` + \`resetAuthenticatedFetch()\` in beforeEach — filesystem cache will serve prior test responses otherwise. + +* **Non-essential DB cache writes should be guarded with try-catch**: Non-essential DB cache writes (e.g., \`setUserInfo()\`, \`setInstallInfo()\`) must wrap in try-catch so broken/read-only DB doesn't crash a successful command. Pattern: \`try { setInstallInfo(...) } catch { log.debug(...) }\`. Exception: \`getUserRegions()\` failure should \`clearAuth()\` and fail hard (invalid token). BugBot enforces any \`setInstallInfo\`/\`setUserInfo\` outside \`setup.ts\`'s wrapper needs try-catch. + * **Shared pagination infrastructure: buildPaginationContextKey and parseCursorFlag**: (pattern) Pagination infrastructure + org flag injection: Bidirectional pagination via cursor stack in \`src/lib/db/pagination.ts\`. \`resolveCursor(flag, key, contextKey)\` maps keywords (next/prev/first/last) to \`{cursor, direction}\`. \`advancePaginationState\` manages stack — back-then-forward truncates stale entries. \`paginationHint()\` builds nav strings. Critical: \`resolveCursor()\` must be called INSIDE \`org-all\` override closures, not before \`dispatchOrgScopedList\`. Hidden global \`--org\`/\`--project\` flags accept old \`sentry-cli\` syntax via \`mergeGlobalFlags()\` in command.ts; \`applyOrgProjectFlags()\` writes to \`SENTRY\_ORG\`/\`SENTRY\_PROJECT\` before auth guard. No short aliases (\`-p\` conflicts). Issue list \`--limit\` is global total: \`fetchWithBudget\` Phase 1 divides evenly, Phase 2 redistributes surplus. \`trimWithProjectGuarantee\` ensures ≥1 issue per project. Compound cursor (pipe-separated) enables \`-c last\` for multi-target pagination. @@ -90,3 +132,23 @@ * **Test helpers for host-scoping security tests**: (pattern) Test helpers for host-scoping security tests: \`test/helpers.ts\` provides shared utilities. \`useEnvSandbox(keys)\` saves+clears+restores env keys (do NOT use in tests depending on preload's \`SENTRY\_AUTH\_TOKEN\`). \`resetHostScopingState()\` bundles \`resetEnvTokenHostForTesting\` + \`resetLoginTrustAnchorForTesting\` + \`resetTrustedRegionUrlsForTesting\` (always reset together). \`mintSntrysToken(payload)\` produces \`sntrys\_\\_\\` test tokens (rstrip \`=\`). \`extractFetchUrl(input)\` for fetch-mock assertions. \`useTestConfigDir\` handles config-dir isolation. Tests mocking fetch with non-SaaS URLs must pass \`{host}\` to \`setAuthToken\` — token defaults to SaaS via \`captureEnvTokenHost\`. For \`assertRcUrlTrusted\`: sequence is \`resetEnvTokenHostForTesting()\` → delete env vars → \`captureEnvTokenHost()\` → \`applySentryCliRcEnvShim()\` → \`assertRcUrlTrusted()\`. E2E: \`createE2EContext\` parent must \`setAuthToken(token, ttl, {host: serverUrl})\`; multi-region tests need \`registerTrustedRegionUrls\`. + + +* **Token-type classification via literal prefix match (classifySentryToken)**: Token-type classification via literal prefix match: \`src/lib/token-type.ts\` \`classifySentryToken(token)\` returns \`'org-auth-token'\` (\`sntrys\_\` prefix), \`'user-auth-token'\` (\`sntryu\_\` prefix), or \`'oauth-or-legacy'\`. Case-sensitive \`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 — \`/auth/\` rejects \`sntrys\_\`) before a confusing API failure. \`getAuthToken()\` from \`db/auth\` returns the effective token (env + DB fallback). + +### Preference + + +* **Always investigate multiple related files thoroughly before proposing or implementing changes**: When tackling a bug or feature, the user consistently requests deep, multi-file investigation first — reading complete files, checking SDK/type definitions, grepping for existing usage, and mapping all relevant call sites — before any implementation begins. This applies especially when the scope of a problem may be wider than it first appears (e.g., TLS gaps across 13 fetch sites, or parameter interactions across API layer, types, formatters, and commands). The user expects the assistant to surface all findings (including what is NOT present, like missing \`expand\` usage or uncovered TLS sites) before moving to a fix. Do not jump to implementation; complete the audit/investigation phase first. + + +* **Always provide precise file locations and line numbers when describing code issues**: When reporting bugs or requesting design/fixes, the user consistently provides exact file paths, function names, and line number ranges (e.g., \`src/lib/api/issues.ts:70-78\`, \`buildIssueListCollapse()\`). They also specify the exact current behavior vs. expected behavior, identify misleading comments, and call out SDK type constraints. When working on a fix, expect the user to reference specific line numbers and function signatures. Always anchor responses to the same level of precision — cite exact file paths, line numbers, and function names rather than speaking in generalities. + + +* **Always request a thorough self-review before merging a PR**: Before merging any PR, the user expects a critical self-review — often delegated to a subagent for objectivity — that categorizes findings by severity (CRITICAL, MEDIUM, LOW). All CRITICAL and MEDIUM issues must be fixed and verified (tests passing, lint clean) before the PR is pushed and merged. This pattern applies consistently across PRs in the Bun→Node.js migration project. The assistant should proactively perform or suggest this review step rather than waiting to be asked, and should not merge until the full test suite passes and lint is clean. + + +* **Always write migration plans to the .opencode/plans/ directory**: When the user requests a migration plan or similar structured planning output, the assistant should write the completed plan as a markdown file to \`.opencode/plans/\` in the project root. The user expects a full codebase exploration phase before writing the plan, followed by writing the plan file to that specific directory. Plan files use timestamp-based or descriptive slugs as filenames (e.g., \`1779289703678-quiet-meadow.md\`). After completing the plan, the assistant must call \`plan\_exit\` to signal completion to the user. + + +* **Bot review triage: distinguish real bugs from SDK-mirroring false positives**: When Sentry Seer or Cursor Bugbot flags 'unusual' code that intentionally mirrors upstream SDK behavior (e.g., \`http\_proxy\` as last-resort fallback for HTTPS URLs — deliberate in \`@sentry/node-core\` \`applyNoProxyOption\`), decline with a written rationale referencing the SDK source rather than silently changing behavior. Removing the mirror creates a divergence where users get different proxy semantics from our transport vs. the SDK default. BYK's pattern: verify against \`node\_modules/@sentry/node-core/build/esm/transports/http.js\`, post a reply explaining the precedent, and resolve the thread. Real bugs (uppercase env var support, whitespace trimming, wildcard handling) get fixed; SDK-mirroring 'bugs' get explained and dismissed. diff --git a/src/lib/api/sourcemaps.ts b/src/lib/api/sourcemaps.ts index 78ac361de..41768a107 100644 --- a/src/lib/api/sourcemaps.ts +++ b/src/lib/api/sourcemaps.ts @@ -24,8 +24,11 @@ import { open, readFile, stat, unlink } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { promisify } from "node:util"; -// biome-ignore lint/performance/noNamespaceImport: needed for feature-detected zstd access -import * as zlib from "node:zlib"; +import { + gzip as gzipCb, + constants as zlibConstants, + zstdCompress as zstdCompressCb, +} from "node:zlib"; import pLimit from "p-limit"; import { z } from "zod"; import { ApiError } from "../errors.js"; @@ -35,22 +38,8 @@ import { getSdkConfig } from "../sentry-client.js"; import { type ZipCompression, ZipWriter } from "../sourcemap/zip.js"; import { apiRequestToRegion } from "./infrastructure.js"; -const gzipAsync = promisify(zlib.gzip); -// zstdCompress is available in Node 22.15+. Feature-detect to avoid crashing -// the npm bundle on older Node versions (e.g., CI runners with Node 20). -// zstdCompress is available in Node 22.15+. Feature-detect to avoid crashing -// the npm bundle on older Node versions (e.g., CI runners with Node 20). -// biome-ignore lint/suspicious/noExplicitAny: zstd types unavailable on older @types/node -const zstdCompressFn = (zlib as any).zstdCompress as - | ((...args: unknown[]) => unknown) - | undefined; -const zstdCompressAsync = - typeof zstdCompressFn === "function" - ? (promisify(zstdCompressFn) as ( - buf: Buffer, - opts?: unknown - ) => Promise) - : undefined; +const gzipAsync = promisify(gzipCb); +const zstdCompressAsync = promisify(zstdCompressCb); const log = logger.withTag("api.sourcemaps"); // ── Schemas ───────────────────────────────────────────────────────── @@ -219,13 +208,13 @@ export async function encodeChunk( buf: Buffer, encoding: UploadEncoding | undefined ): Promise { - if (encoding === "zstd" && zstdCompressAsync) { + if (encoding === "zstd") { // L3 is libzstd's default; passed explicitly for self-documenting // code. L9+ trades ~14% size for 4x compress time and forces the // server's decoder to allocate 15-30 MiB of window state -- not // worth it once decode cost is counted. return await zstdCompressAsync(buf, { - params: { [zlib.constants.ZSTD_c_compressionLevel]: 3 }, + params: { [zlibConstants.ZSTD_c_compressionLevel]: 3 }, }); } if (encoding === "gzip") { diff --git a/src/lib/telemetry/zstd-transport.ts b/src/lib/telemetry/zstd-transport.ts index ce3fb1b2e..890cae6b7 100644 --- a/src/lib/telemetry/zstd-transport.ts +++ b/src/lib/telemetry/zstd-transport.ts @@ -32,8 +32,11 @@ import * as http from "node:http"; import * as https from "node:https"; import { Readable } from "node:stream"; import { promisify } from "node:util"; -// biome-ignore lint/performance/noNamespaceImport: needed for feature-detected zstd access -import * as zlib from "node:zlib"; +import { + gzip as gzipCb, + constants as zlibConstants, + zstdCompress as zstdCompressCb, +} from "node:zlib"; import { createTransport, suppressTracing, @@ -66,7 +69,7 @@ const ZSTD_LEVEL = 3; * Minimum body length above which we attempt compression. * * For zstd we lower this from the SDK's 32 KiB gzip threshold to 1 KiB - * — Bun's zstd worker is cheap to dispatch and most error envelopes + * — zstd compression via libuv is cheap to dispatch and most error envelopes * (5–15 KiB) would miss the 32 KiB cutoff and ship uncompressed * otherwise. */ @@ -78,22 +81,8 @@ const ZSTD_THRESHOLD = 1024; */ const GZIP_THRESHOLD = 1024 * 32; -const gzipAsync = promisify(zlib.gzip); -// zstdCompress is available in Node 22.15+. Feature-detect to avoid crashing -// the npm bundle on older Node versions (e.g., CI runners with Node 20). -// zstdCompress is available in Node 22.15+. Feature-detect to avoid crashing -// the npm bundle on older Node versions (e.g., CI runners with Node 20). -// biome-ignore lint/suspicious/noExplicitAny: zstd types unavailable on older @types/node -const zstdCompressFn = (zlib as any).zstdCompress as - | ((...args: unknown[]) => unknown) - | undefined; -const zstdCompressAsync = - typeof zstdCompressFn === "function" - ? (promisify(zstdCompressFn) as ( - buf: Buffer, - opts?: unknown - ) => Promise) - : undefined; +const gzipAsync = promisify(gzipCb); +const zstdCompressAsync = promisify(zstdCompressCb); /** * Factory for the SDK's `Sentry.init({ transport })` option. @@ -288,9 +277,9 @@ export async function maybeCompress( return { payload: buf, encodingApplied: "none" }; } - if (encoding === "zstd" && zstdCompressAsync) { + if (encoding === "zstd") { const out = await zstdCompressAsync(buf, { - params: { [zlib.constants.ZSTD_c_compressionLevel]: ZSTD_LEVEL }, + params: { [zlibConstants.ZSTD_c_compressionLevel]: ZSTD_LEVEL }, }); return { payload: Buffer.from(out.buffer, out.byteOffset, out.byteLength), @@ -304,7 +293,7 @@ export async function maybeCompress( /** Feature-detect zstd support on the current runtime. */ export function hasZstdSupport(): boolean { - return zstdCompressAsync !== undefined; + return typeof zstdCompressCb === "function"; } /**