Skip to content

fix(node): handle sqlite conversion failures#36024

Merged
bartlomieju merged 1 commit into
denoland:mainfrom
nathanwhit:fix/sqlite-conversion-failures
Jul 14, 2026
Merged

fix(node): handle sqlite conversion failures#36024
bartlomieju merged 1 commit into
denoland:mainfrom
nathanwhit:fix/sqlite-conversion-failures

Conversation

@nathanwhit

Copy link
Copy Markdown
Member

This makes node:sqlite treat JavaScript conversion failures as normal, catchable errors.

V8 property and index access can execute JavaScript and fail, while C string conversion rejects embedded null bytes. The affected statement binding, tag store, function and aggregate option, iterator setup, and session option paths now propagate those failures instead of assuming conversion always succeeds. Result and error properties are also defined directly to avoid invoking inherited setters.

After a failed conversion, the database remains usable by subsequent operations.

Validation:

  • ./tools/format.js
  • cargo fmt --check --package deno_node_sqlite
  • cargo build --bin deno
  • full tests/unit_node/sqlite_test.ts suite (82 tests)

@nathanwhit
nathanwhit marked this pull request as ready for review July 13, 2026 23:14
@bartlomieju
bartlomieju merged commit 6a9d607 into denoland:main Jul 14, 2026
304 of 306 checks passed
bartlomieju pushed a commit that referenced this pull request Jul 15, 2026
This makes `node:sqlite` treat JavaScript conversion failures as normal,
catchable errors.

V8 property and index access can execute JavaScript and fail, while C
string conversion rejects embedded null bytes. The affected statement
binding, tag store, function and aggregate option, iterator setup, and
session option paths now propagate those failures instead of assuming
conversion always succeeds. Result and error properties are also defined
directly to avoid invoking inherited setters.

After a failed conversion, the database remains usable by subsequent
operations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants