Skip to content

sqlite: isolate applyChangeset filter errors - #64823

Open
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:sqlite-filter-exception-supression
Open

sqlite: isolate applyChangeset filter errors#64823
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:sqlite-filter-exception-supression

Conversation

@trivikr

@trivikr trivikr commented Jul 29, 2026

Copy link
Copy Markdown
Member

Fixes: #64822

When an applyChangeset() filter callback throws,
ignore_next_sqlite_error_ is set. Returning false from xFilter is not
itself a SQLite error, so sqlite3changeset_apply() can return SQLITE_OK
without consuming the flag. The next unrelated SQLite error is then
silently suppressed.

This change keeps filter callback failure state local to the
applyChangeset() invocation.


Assisted-by: codex:gpt-5.6-sol

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/sqlite

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. sqlite Issues and PRs related to the SQLite subsystem. labels Jul 29, 2026
Track filter callback failures within each applyChangeset() invocation.
Returning false from xFilter is not a SQLite error. This previously left
the database-wide suppression flag set, which could hide the next
unrelated SQLite error.

Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: codex:gpt-5.6-sol
@trivikr
trivikr force-pushed the sqlite-filter-exception-supression branch from 58e98fa to 927eaf5 Compare July 29, 2026 16:18
@trivikr trivikr added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 29, 2026
@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.15%. Comparing base (f8f8aa2) to head (927eaf5).
⚠️ Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
src/node_sqlite.cc 80.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #64823      +/-   ##
==========================================
+ Coverage   90.14%   90.15%   +0.01%     
==========================================
  Files         746      746              
  Lines      242818   242818              
  Branches    45765    45765              
==========================================
+ Hits       218884   218920      +36     
+ Misses      15417    15384      -33     
+ Partials     8517     8514       -3     
Files with missing lines Coverage Δ
src/node_sqlite.cc 80.67% <80.00%> (ø)

... and 30 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. request-ci Add this label to start a Jenkins CI on a PR. sqlite Issues and PRs related to the SQLite subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sqlite: filter exception in applyChangeset() suppresses next SQLite error

3 participants