Skip to content

Commit 2e4435e

Browse files
fix: address review comments from Bun→Node migration PRs (#989)
## Summary Consolidates fixes for review comments across PRs #967, #970, #984, #986, superseding #988. ### Critical - **Bump `engines.node` to `>=22.15`** — `node:zlib` zstd APIs require 22.15+. Node 20 is EOL. Updated `dist/bin.cjs` runtime version check to match. - **Simplify zstd imports** — replaced feature-detection dance with direct `import { zstdCompress } from "node:zlib"` now that >=22.15 is guaranteed. ### High - **Fix spawn error handling in `upgrade.ts`** — `proc.on("error", () => resolve(1))` discarded the error object, making ENOENT/EBUSY detection dead code. Now properly rejects with the error. ### Medium - **Fix `sqlite.ts` ROLLBACK** — if ROLLBACK throws, the original transaction error was lost. Now wrapped in try/catch. - **Guard `semver.compare`** in `delta-upgrade.ts` with `semver.valid()` to avoid throwing on invalid version strings. - **Narrow catch in `shell.ts`** `addToGitHubPath` — only catch ENOENT, re-throw EACCES/EPERM. - **Add WriteStream backpressure** in `upgrade.ts` `streamDecompressToFile` — check `write()` return value, await `'drain'`. - **Add `setup-node@v6`** with Node 22 to E2E CI job. ### Low - **Fix `whichSync` Windows CRLF** — split on `/\r?\n/` instead of `\n` to strip trailing `\r` from `where.exe` output. ### Test results All 7014 tests pass, 0 failures. --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
1 parent 862ba00 commit 2e4435e

13 files changed

Lines changed: 123 additions & 61 deletions

File tree

.github/workflows/ci.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -633,6 +633,9 @@ jobs:
633633
with:
634634
bun-version: "1.3.13"
635635
- uses: pnpm/action-setup@v4
636+
- uses: actions/setup-node@v6
637+
with:
638+
node-version: "22"
636639
- uses: actions/cache@v5
637640
id: cache
638641
with:

.lore.md

Lines changed: 50 additions & 9 deletions
Large diffs are not rendered by default.

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ Credentials are stored in `~/.sentry/` with restricted permissions (mode 600).
7777
## Library Usage
7878

7979
<!-- GENERATED:START library-prereq -->
80-
Use Sentry CLI programmatically in Node.js (≥22.12) or Bun without spawning a subprocess:
80+
Use Sentry CLI programmatically in Node.js (≥22.15) or Bun without spawning a subprocess:
8181
<!-- GENERATED:END library-prereq -->
8282

8383
```typescript

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@
6363
},
6464
"description": "Sentry CLI - A command-line interface for using Sentry built by robots and humans for robots and humans",
6565
"engines": {
66-
"node": ">=22.12"
66+
"node": ">=22.15"
6767
},
6868
"files": [
6969
"dist/bin.cjs",

script/bundle.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ const result = await build({
242242
// Write the CLI bin wrapper (tiny — shebang + version check + dispatch).
243243
// Version floor must track `engines.node` in package.json.
244244
const BIN_WRAPPER = `#!/usr/bin/env node
245-
{let v=process.versions.node.split(".").map(Number);if(v[0]<22||(v[0]===22&&v[1]<12)){console.error("Error: sentry requires Node.js 22.12 or later (found "+process.version+").\\n\\nEither upgrade Node.js, or install the standalone binary instead:\\n curl -fsSL https://cli.sentry.dev/install | bash\\n");process.exit(1)}}
245+
{let v=process.versions.node.split(".").map(Number);if(v[0]<22||(v[0]===22&&v[1]<15)){console.error("Error: sentry requires Node.js 22.15 or later (found "+process.version+").\\n\\nEither upgrade Node.js, or install the standalone binary instead:\\n curl -fsSL https://cli.sentry.dev/install | bash\\n");process.exit(1)}}
246246
{let e=process.emit;process.emit=function(n,...a){return n==="warning"?!1:e.apply(this,[n,...a])}}
247247
require('./index.cjs')._cli().catch(()=>{process.exitCode=1});
248248
`;

src/commands/cli/upgrade.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -431,9 +431,9 @@ async function spawnWithRetry(
431431
stdio: ["ignore", "inherit", "inherit"],
432432
env,
433433
});
434-
return await new Promise<number>((resolve) => {
434+
return await new Promise<number>((resolve, reject) => {
435435
proc.on("close", (code) => resolve(code ?? 1));
436-
proc.on("error", () => resolve(1));
436+
proc.on("error", (err) => reject(err));
437437
});
438438
} catch (error) {
439439
// Translate the opaque Bun "Executable not found" error into an

src/lib/api/sourcemaps.ts

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,11 @@ import { open, readFile, stat, unlink } from "node:fs/promises";
2424
import { tmpdir } from "node:os";
2525
import { join } from "node:path";
2626
import { promisify } from "node:util";
27-
// biome-ignore lint/performance/noNamespaceImport: needed for feature-detected zstd access
28-
import * as zlib from "node:zlib";
27+
import {
28+
gzip as gzipCb,
29+
constants as zlibConstants,
30+
zstdCompress as zstdCompressCb,
31+
} from "node:zlib";
2932
import pLimit from "p-limit";
3033
import { z } from "zod";
3134
import { ApiError } from "../errors.js";
@@ -35,22 +38,8 @@ import { getSdkConfig } from "../sentry-client.js";
3538
import { type ZipCompression, ZipWriter } from "../sourcemap/zip.js";
3639
import { apiRequestToRegion } from "./infrastructure.js";
3740

38-
const gzipAsync = promisify(zlib.gzip);
39-
// zstdCompress is available in Node 22.15+. Feature-detect to avoid crashing
40-
// the npm bundle on older Node versions (e.g., CI runners with Node 20).
41-
// zstdCompress is available in Node 22.15+. Feature-detect to avoid crashing
42-
// the npm bundle on older Node versions (e.g., CI runners with Node 20).
43-
// biome-ignore lint/suspicious/noExplicitAny: zstd types unavailable on older @types/node
44-
const zstdCompressFn = (zlib as any).zstdCompress as
45-
| ((...args: unknown[]) => unknown)
46-
| undefined;
47-
const zstdCompressAsync =
48-
typeof zstdCompressFn === "function"
49-
? (promisify(zstdCompressFn) as (
50-
buf: Buffer,
51-
opts?: unknown
52-
) => Promise<Buffer>)
53-
: undefined;
41+
const gzipAsync = promisify(gzipCb);
42+
const zstdCompressAsync = promisify(zstdCompressCb);
5443
const log = logger.withTag("api.sourcemaps");
5544

5645
// ── Schemas ─────────────────────────────────────────────────────────
@@ -219,13 +208,13 @@ export async function encodeChunk(
219208
buf: Buffer,
220209
encoding: UploadEncoding | undefined
221210
): Promise<Uint8Array> {
222-
if (encoding === "zstd" && zstdCompressAsync) {
211+
if (encoding === "zstd") {
223212
// L3 is libzstd's default; passed explicitly for self-documenting
224213
// code. L9+ trades ~14% size for 4x compress time and forces the
225214
// server's decoder to allocate 15-30 MiB of window state -- not
226215
// worth it once decode cost is counted.
227216
return await zstdCompressAsync(buf, {
228-
params: { [zlib.constants.ZSTD_c_compressionLevel]: 3 },
217+
params: { [zlibConstants.ZSTD_c_compressionLevel]: 3 },
229218
});
230219
}
231220
if (encoding === "gzip") {

src/lib/db/sqlite.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,11 @@ export class Database {
139139
this.db.exec("COMMIT");
140140
return result;
141141
} catch (error) {
142-
this.db.exec("ROLLBACK");
142+
try {
143+
this.db.exec("ROLLBACK");
144+
} catch (rollbackError) {
145+
log.debug("ROLLBACK failed after transaction error", rollbackError);
146+
}
143147
throw error;
144148
}
145149
};

src/lib/delta-upgrade.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import { unlinkSync } from "node:fs";
2121

2222
// biome-ignore lint/performance/noNamespaceImport: Sentry SDK recommends namespace import
2323
import * as Sentry from "@sentry/node-core/light";
24-
import { compare as semverCompare } from "semver";
24+
import { compare as semverCompare, valid as semverValid } from "semver";
2525

2626
import {
2727
GITHUB_RELEASES_URL,
@@ -460,10 +460,17 @@ export function filterAndSortChainTags(
460460
currentVersion: string,
461461
targetVersion: string
462462
): string[] {
463+
if (!(semverValid(currentVersion) && semverValid(targetVersion))) {
464+
return [];
465+
}
466+
463467
const chainTags: { tag: string; version: string }[] = [];
464468

465469
for (const tag of allTags) {
466470
const version = tag.slice(PATCH_TAG_PREFIX.length);
471+
if (!semverValid(version)) {
472+
continue;
473+
}
467474
// Include tags where: currentVersion < version <= targetVersion
468475
if (
469476
semverCompare(version, currentVersion) === 1 &&

src/lib/shell.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,11 @@
88
import { existsSync } from "node:fs";
99
import { access, readFile, writeFile } from "node:fs/promises";
1010
import { basename, delimiter, join } from "node:path";
11+
import { logger } from "./logger.js";
1112
import { whichSync } from "./which.js";
1213

14+
const log = logger.withTag("shell");
15+
1316
/** Supported shell types */
1417
export type ShellType = "bash" | "zsh" | "fish" | "sh" | "ash" | "unknown";
1518

@@ -299,7 +302,12 @@ export async function addToGitHubPath(
299302
let content = "";
300303
try {
301304
content = await readFile(env.GITHUB_PATH, "utf-8");
302-
} catch {
305+
} catch (error) {
306+
const code = (error as NodeJS.ErrnoException).code;
307+
if (code !== "ENOENT") {
308+
log.debug(`Failed to read GITHUB_PATH (${code}):`, error);
309+
return false;
310+
}
303311
// File doesn't exist yet — start with empty content
304312
}
305313

@@ -310,7 +318,8 @@ export async function addToGitHubPath(
310318
await writeFile(env.GITHUB_PATH, newContent, "utf-8");
311319
}
312320
return true;
313-
} catch {
321+
} catch (error) {
322+
log.debug("Failed to update GITHUB_PATH:", error);
314323
return false;
315324
}
316325
}

0 commit comments

Comments
 (0)