feat(db): support Node.js 18+ for the npm package via WASM SQLite fallback by BYK · Pull Request #1260 · getsentry/cli

BYK · 2026-07-16T12:32:54Z

Why

The npm package required Node.js 22.15+ purely because our config/cache/auth layer depends on the built-in node:sqlite module, which older runtimes don't ship. That excluded users still on Node 18/20 for no fundamental reason — the CLI's actual SQLite footprint is tiny (prepare → get/all/run, exec, manual transactions).

A user asked for Node 20 support and proposed better-sqlite3. We pushed back on that: a native addon breaks the single-file bundle, reintroduces per-platform/per-ABI install flakiness, and violates our zero-runtime-dependencies rule.

Approach

Fall back to a pure-WASM driver (node-sqlite3-wasm) on Node < 22.15, selected at runtime behind the existing single-file adapter:

22.15+ → native node:sqlite (unchanged fast path)
18–22.14 → bundled WASM driver

It's a bundled devDependency, so the single-file bundle and no-runtime-deps guarantees both hold.

Critically, the driver is require()d lazily so the standalone binary build externalizes it: the SEA binary always embeds a modern LTS Node and uses native node:sqlite, making the WASM path dead code there that adds zero bytes to the binary. This was the hard constraint for taking this route over better-sqlite3.

Tradeoff

The WASM fallback can't use SQLite WAL mode, so its local cache is slower and less concurrency-friendly. Acceptable for a single-process CLI, and the docs now steer people toward the standalone binary or Node 22.15+.

Version floors

Dev tooling still needs 22.15+ (it runs the sources against node:sqlite via tsx). So engines.node advertises the consumer floor (>=18) while a new devEngines field records the development floor — the doc generator was decoupled to keep the two independent.

CI now runs the npm-package job on a Node 20 matrix entry so the WASM fallback is exercised, not just shipped.

…lback The npm package required Node.js 22.15+ purely because the config/cache/auth layer depends on the built-in node:sqlite module, which doesn't exist on older runtimes. That shut out users still on Node 18/20 for no fundamental reason — the CLI's actual SQLite needs are tiny. Rather than take on a native addon (better-sqlite3), which would break our single-file bundle, reintroduce per-platform/per-ABI install flakiness, and violate the zero-runtime-dependencies rule, we fall back to a pure-WASM driver (node-sqlite3-wasm) on Node < 22.15. It's a bundled devDependency like everything else, so the single-file bundle and no-runtime-deps guarantees hold. The driver is loaded lazily so the standalone binary build can externalize it: the SEA binary always embeds a modern LTS Node and uses native node:sqlite, so the WASM path is dead code there and adds zero bytes — the whole point of the "only if it stays out of the Node 24 binary" constraint. The WASM fallback can't use SQLite WAL mode, so its local cache is slower and less concurrency-friendly. This is acceptable for a single-process CLI, but we steer people toward the standalone binary or Node 22.15+ in the docs. Dev tooling still needs 22.15+ (it runs the sources against node:sqlite via tsx), so engines.node advertises the consumer floor (>=18) while a new devEngines field records the development floor, keeping the two independent in generated docs.

github-actions · 2026-07-16T12:33:59Z

PR Preview Action v1.8.1
🚀 View preview at https://cli.sentry.dev/_preview/pr-1260/
Built to branch `gh-pages` at 2026-07-17 08:40 UTC. Preview will be ready when the GitHub Pages deployment is complete.

github-actions · 2026-07-16T12:36:56Z

Codecov Results 📊

✅ Patch coverage is 87.95%. Project has 5475 uncovered lines.
✅ Project coverage is 81.67%. Comparing base (base) to head (head).

Files with missing lines (4)

File	Patch %	Lines
src/lib/db/sqlite.ts	92.19%	⚠️ 5 Missing and 3 partials
src/lib/api/chunk-upload.ts	60.00%	⚠️ 2 Missing and 3 partials
src/lib/db/index.ts	75.00%	⚠️ 3 Missing and 1 partials
src/lib/telemetry/zstd-transport.ts	100.00%	⚠️ 1 partials

Coverage diff

@@            Coverage Diff             @@
##          main       #PR       +/-##
==========================================
+ Coverage    81.64%    81.67%    +0.03%
==========================================
  Files          425       425         —
  Lines        29795     29868       +73
  Branches     19339     19398       +59
==========================================
+ Hits         24325     24393       +68
- Misses        5470      5475        +5
- Partials      2025      2033        +8

Generated by Codecov Action

…rix Node The Node 20 matrix entry failed because it tried to *build* under Node 20, but our build tooling (the tsx loader hook in require-shim.mjs) uses node:module's registerHooks, which only exists on 22.15+. That's a development-toolchain requirement, not a constraint on the shipped artifact — we always publish a bundle built on modern Node. So build (and pack) under Node 22 unconditionally, then switch to the matrix Node purely to run the produced artifact. This models reality: Node 20 now genuinely exercises the WASM SQLite fallback at runtime, which was the point of adding the entry, without falsely requiring the toolchain to run there.

Two modules called promisify(zlib.zstdCompress) at top level. On Node < 22.15 (the WASM-SQLite fallback range we now support) zlib.zstdCompress is undefined, so promisify(undefined) threw ERR_INVALID_ARG_TYPE at import time — taking down the entire CLI before any command ran. This is what broke the new Node 20 CI smoke test. Guard both the same way infrastructure.ts already does, and teach the chunk-upload codec picker to not advertise zstd when the local runtime can't produce it — so a zstd-capable server never steers us into a codec we can't emit. zstd stays the path on 22.15+; older Node cleanly uses gzip.

node-sqlite3-wasm locks the database by mkdir("<db>.lock") and only rmdir's it when SQLite lowers the lock to NONE. A CLI process that exits while still holding a lock — the normal case, since we never explicitly downgrade before exit — leaves the empty directory behind, and the very next invocation's mkdir fails with EEXIST, surfacing as "database is locked" permanently. This broke every command after the first on Node < 22.15. Since CLI runs are short-lived, effectively single-writer invocations, a leftover lock dir is always stale (its owner is gone). Clear it before opening on the WASM path; live contention is still handled by busy_timeout. Also close the connection on process exit to release the lock proactively and shrink the window.

The driver was loaded via _require("node-sqlite3-wasm"), but the require-alias plugin (which rewrites _require→require so esbuild inlines it) didn't cover db/sqlite.ts. So the bundle kept a runtime require() for a package that isn't shipped (it's a devDependency), meaning a real npm install on Node < 22.15 would fail with MODULE_NOT_FOUND — the fallback only "worked" in the repo where node_modules happens to contain it. Add db/sqlite to the filter so the driver is genuinely inlined. (Caught by sentry-warden.) Also only clear the WASM lock directory when it's demonstrably stale (older than a generous window), instead of unconditionally. A freshly-created lock may belong to a concurrently-running CLI; deleting it could let two processes hold the DB at once. Recent locks are now left to busy_timeout. (Caught by Cursor.)

BYK · 2026-07-16T13:33:52Z

Thanks — both findings were valid and are fixed in 174edeb:

@sentry-warden (driver never bundled): Correct, and worse than it looked — the fallback only appeared to work because node_modules was present in the dev/CI checkout. A real npm i -g sentry on Node < 22.15 would've hit MODULE_NOT_FOUND since node-sqlite3-wasm is a devDependency. Fixed by adding db/sqlite to the require-alias filter so esbuild actually inlines the driver. Verified against an isolated dir with no node_modules: import succeeds, the sibling .wasm loads via __dirname, schema initializes, and auth status returns exit 10. The SEA binary still externalizes the driver (0 driver symbols embedded).

@cursor (live lock removal): Valid race. clearStaleWasmLock now only removes the <db>.lock directory when its mtime is older than a generous staleness window (60s), so a lock a concurrent process just acquired is left to busy_timeout instead of being deleted out from under it. Added a regression test covering both stale-cleared and recent-preserved cases.

…blocked The 60s staleness guard, while correct for avoiding races, reintroduced the original failure for the common case: CI runs `--help` then `auth status` seconds apart, so the lock left by the (now-dead) first process is too "recent" to be cleared on the second's open — and the second fails with "database is locked". The real fix is to release the lock when we close our own connection. A plain node-sqlite3-wasm close() only rmdir's <db>.lock when SQLite happens to be at lock level NONE, which it usually isn't, so the empty dir survives. Since close() is releasing our own lock (no race), remove it unconditionally there. The age-guarded cleanup on open now only backstops the crash/SIGKILL case where close() never ran — and stays conservative to avoid deleting a live peer's lock.

…shared mutex An adversarial review flagged that the previous approach — having close() unconditionally rmdir the <db>.lock directory — is a corruption hazard. That directory is a path-keyed cross-process mutex, not a per-connection handle, so a second CLI's close() could delete a concurrently-running writer's live lock and let two processes write at once. Verified reproducible. Fix the root cause instead: node-sqlite3-wasm only leaked the lock because a `.get()` leaves its statement cursor open, which blocks the driver's own close-time unlock. Finalizing the statement after each single-row read lets the driver release exactly our lock during close() — ownership-correct, no manual directory removal. (`.all()`/`.run()` already read to completion, so they don't leak.) close() no longer touches the lock dir at all; the age-guarded open-time cleanup remains solely as a crash-recovery backstop. Also from the review: coerce boolean binds to 0/1 for both drivers (node:sqlite rejects raw booleans) so the exported binding type is actually portable; fail loudly if zstd encoding is ever requested without runtime support instead of silently shipping mislabeled bytes; and correct stale bun:sqlite comments to reference the real second driver.

sentry · 2026-07-17T01:08:18Z

  // The library bundle must not suppress the host application's warnings.
  sourcemap: true,


Bug: The requireAliasPlugin filter in script/bundle.ts is missing db/sqlite.ts, preventing _require("node-sqlite3-wasm") from being correctly transformed, which will cause a runtime crash.
_{Severity: CRITICAL}

Suggested Fix

Update the REQUIRE_ALIAS_FILTER regex in script/bundle.ts to include db/sqlite.ts. The pattern should be modified to something like /(?:db[\\/](?:index|schema|sqlite)|list-command|telemetry)\.ts$/ to ensure the requireAliasPlugin correctly processes the _require call for the WASM driver.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not valid. Location: script/bundle.ts#L194-L195 Potential issue: The `REQUIRE_ALIAS_FILTER` regex in `script/bundle.ts` does not include `db/sqlite.ts`. Consequently, the `requireAliasPlugin` fails to transform the `_require("node-sqlite3-wasm")` call in `src/lib/db/sqlite.ts` during the build. When the CLI is installed from npm on Node.js versions below 22.15, it will attempt to use the WASM-based SQLite driver as a fallback. This triggers the untransformed `_require` call, which fails with a `MODULE_NOT_FOUND` error because `node-sqlite3-wasm` is a dev dependency and not present in the final package. This will cause any CLI command that accesses the database to crash.

Also affects:

src/lib/db/sqlite.ts:209~209

_{Did we get this right? 👍 / 👎 to inform future reviews.}

sentry · 2026-07-17T02:17:40Z

+    }
+    this.db = new Ctor(path);
+    this.kind = kind;
  }

  /** Execute raw SQL (DDL statements, multi-statement strings). */


Bug: A race condition in Database.close() between releasing the lock and removing the lock directory could cause another process's lock to be deleted, risking data corruption.
_{Severity: MEDIUM}

Suggested Fix

Instead of manually calling rmdirSync(), ensure all cursors are finalized before closing the connection. This should allow the underlying SQLite WASM driver to handle the lock release and directory removal atomically, avoiding the race condition. This approach was noted in a previous commit message.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not valid. Location: src/lib/db/sqlite.ts#L329-L338 Potential issue: The `Database.close()` function first calls `this.db.close()` to release the database lock and then unconditionally calls `rmdirSync()` to remove the lock directory. This creates a small time window between the lock being released and its directory being deleted. If another process acquires the lock in this window, the original process's `rmdirSync()` call will delete the lock directory of the new process. This could lead to database corruption if the second process was actively using the database. While the window is small, rapid consecutive CLI invocations could trigger this condition.

…river failure Two issues surfaced by Seer: - `PRAGMA journal_mode = WAL` was issued unconditionally. The WASM driver silently ignores it and stays in rollback-journal mode, so the pragma was a no-op that misrepresented the actual mode. Expose `Database.driverKind` and only set WAL on the native `node:sqlite` path (WASM CLI cache is single- process, so rollback journal is fine). - `resolveDriver()` caught a `node:sqlite` load failure and fell through to the WASM driver even when a test had *explicitly* forced `"node"`, which would run the suite against the wrong backend and hide failures. Re-throw when the native driver was explicitly requested; keep the fallback only for the automatic (version-based) path.

BYK · 2026-07-17T08:39:53Z

Triaged the latest Seer (sentry[bot]) findings — all four reviewed the cumulative diff and two were already resolved by later commits; the other two are now fixed in 7be144c:

bundle.ts missing db/sqlite (CRITICAL): Already fixed in 174edeb — REQUIRE_ALIAS_FILTER is /(?:db[\\/](?:index|schema|sqlite)|list-command|telemetry)\.ts$/ and the driver is verifiably inlined (confirmed against a no-node_modules install). False positive on current HEAD.
Race in close() rmdir (MEDIUM): Already fixed in 90173ec — close() no longer touches the lock dir; the driver releases our own lock because the .get() wrapper finalizes cursors. False positive on current HEAD.
Unconditional PRAGMA journal_mode = WAL (MEDIUM): Valid. Now gated on driverKind === 'node'; the WASM path stays in rollback-journal mode (fine for a single-process CLI cache) instead of issuing a silent no-op. Verified native still persists journal_mode=wal.
__setDriverForTests('node') silent WASM fallback (MEDIUM): Valid. resolveDriver() now re-throws when the native driver was explicitly forced, so tests can't silently run against the wrong backend; the automatic version-based fallback is unchanged.

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 7be144c. Configure here.}

cursor · 2026-07-17T08:41:43Z

+      // must never mask the real exit code.
+      log.debug("Failed to close database on exit", error);
+    }
+  });


SIGINT skips database close

Medium Severity

On the WASM SQLite path, lock cleanup relies on close() releasing the driver's <db>.lock mutex. The new handler only registers on process 'exit', but the same module notes that 'exit' does not run on SIGINT. Interrupting the CLI mid-session can leave the lock directory in place and block the next invocation until stale-lock recovery (up to about 60 seconds).

^{Reviewed by Cursor Bugbot for commit 7be144c. Configure here.}

…ndow (#1261) ## Summary Follow-up to #1260 addressing a post-merge finding from **Cursor Bugbot** and **sentry-warden**. On the WASM SQLite fallback (npm package on Node < 22.15), `node-sqlite3-wasm` locks the DB with a `<db>.lock` directory (mkdir mutex) and releases it via `rmdir` on close. A CLI process killed by **SIGINT (Ctrl+C) / SIGTERM** mid-write bypasses the `process.on('exit')` handler, so the lock dir leaks. The previous recovery only cleared a leftover lock once it aged past a **60-second** window — so a quick re-run after Ctrl+C failed with `database is locked` for up to a minute. ## Fix PID-owner sentinel for **precise** liveness detection instead of a time guess: - On open (WASM), write `<db>.lock.owner` = our PID. - On the next open, `clearStaleWasmLock` reads it: - owner **dead** → lock is provably orphaned → cleared **immediately** (no wait), - owner **alive** → genuine live lock → left to `busy_timeout` (no stealing), - **no sentinel** → fall back to the age window (older CLI / races). - On clean close, remove the sentinel. The sentinel lives **outside** the lock dir — a file *inside* it would make the driver's `rmdir` unlock fail (verified), defeating the driver's own cleanup. ### Why not a SIGINT/SIGTERM handler? A DB-layer signal handler would risk interfering with commands that legitimately own SIGINT for graceful streaming shutdown (`log list --follow`, `dashboard view`), which keep the process alive after the signal. The recovery-side fix has zero interference risk. ## Verification - New tests: dead-owner lock cleared immediately despite fresh mtime; live-owner (current PID) lock preserved despite old mtime. 28 sqlite-adapter tests, 296 db tests pass. - E2E reproduction: a fresh lock dir with a dead-owner sentinel is cleared on the next run in ~startup time (no 60s stall); previously this exact case blocked for 60s. - SEA binary still excludes the WASM driver (0 driver symbols); native path unchanged. - npm real-install (no `node_modules`) smoke passes. Also corrected the now-inaccurate JSDoc that described the (previously wrong) 'live lock is recently re-touched' premise.

BYK added 2 commits July 15, 2026 16:13

chore: update .lore.md

ebdddf6

github-actions Bot added the risk: high PR risk score: high label Jul 16, 2026

sentry-warden Bot reviewed Jul 16, 2026

View reviewed changes

Comment thread script/build.ts

BYK added 2 commits July 16, 2026 12:54

cursor Bot reviewed Jul 16, 2026

View reviewed changes

Comment thread src/lib/db/sqlite.ts

sentry-warden Bot reviewed Jul 16, 2026

View reviewed changes

Comment thread src/lib/db/index.ts

Comment thread src/lib/db/sqlite.ts Outdated

sentry Bot reviewed Jul 17, 2026

View reviewed changes

Comment thread src/lib/db/index.ts

sentry Bot reviewed Jul 17, 2026

View reviewed changes

Comment thread src/lib/db/index.ts

cursor Bot reviewed Jul 17, 2026

View reviewed changes

BYK merged commit d2d3ca9 into main Jul 17, 2026
34 checks passed

BYK deleted the feat/node18-wasm-sqlite branch July 17, 2026 08:49

BYK mentioned this pull request Jul 17, 2026

fix(db): recover WASM SQLite locks via PID sentinel, not a 60s age window #1261

Merged

		// The library bundle must not suppress the host application's warnings.
		sourcemap: true,

Uh oh!

Uh oh!

Conversation

BYK commented Jul 16, 2026

Why

Approach

Tradeoff

Version floors

Uh oh!

github-actions Bot commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Built to branch gh-pages at 2026-07-17 08:40 UTC. Preview will be ready when the GitHub Pages deployment is complete.

Uh oh!

github-actions Bot commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Results 📊

Uh oh!

Uh oh!

Uh oh!

BYK commented Jul 16, 2026

Uh oh!

Uh oh!

Uh oh!

sentry Bot Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sentry Bot Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

BYK commented Jul 17, 2026

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor Bot Jul 17, 2026

Choose a reason for hiding this comment

SIGINT skips database close

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

github-actions Bot commented Jul 16, 2026 •

edited

Loading

Built to branch `gh-pages` at 2026-07-17 08:40 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

github-actions Bot commented Jul 16, 2026 •

edited

Loading