sqlite: reject deserialize() while in a callback

[trivikr]

Refs: #64795

deserialize() could be called from a user-defined function invoked during statement execution, tearing down the database connection while sqlite3_step() was still using it. Reuse the existing callback depth check to throw ERR_INVALID_STATE instead, matching the guard already in place for close().

---

Assisted-by: codex:gpt-5.6-sol

[nodejs-github-bot]

Review requested:

• @nodejs/sqlite

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.33%. Comparing base (bf2f995) to head (b2b7405).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #64796 +/- ##
==========================================
- Coverage 90.33% 90.33% -0.01% 
==========================================
Files 760 760 Lines 248522 248523 +1 Branches 46904 46906 +2 ==========================================
- Hits 224513 224511 -2 - Misses 15444 15451 +7 + Partials 8565 8561 -4 

Files with missing lines	Coverage Δ
src/node_sqlite.cc	81.23% <100.00%> (+<0.01%)	⬆️

... and 29 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.

[TrevorBurnham]

The guard itself looks right to me: IsInCallback() is the correct predicate, since the crash needs a live sqlite3_step() frame, which is only reachable from a callback, and every step-reentrant entry point (xFunc, xStepBase/xValueBase, the authorizer, sqlite3changeset_apply) already takes CallbackDepthGuard. Placing it before FinalizeStatements() also means a rejected call leaves no partial state. Three non-blocking notes below, plus one on the docs.

doc/api/sqlite.md: the new throwing condition isn't documented. The database.deserialize() section only mentions that existing statements are finalized first; it'd help to state that the method throws ERR_INVALID_STATE when called from a user-defined function, aggregate, authorizer, or changeset filter/conflict callback. The same is missing for database.close() from #64743 — might be worth adding both here.

[TrevorBurnham]

Thanks for the quick turnaround on the commit message and the test rename — both look good, and keeping the test with its deserialize siblings is a fair call.

The C++ guard is unchanged and still correct. I built this branch and confirmed the #64795 repro now throws instead of crashing, that coverage extends past the tested UDF case to aggregate and authorizer callbacks, and that there are no false positives across connections (deserializing an unrelated database from another database's callback still works). The remaining comments are all on the doc commit, plus one on the Fixes: line.

One more docs point that isn't in the diff: database.close() (line 291) has carried this same callback restriction since 5cef767, but its docs still say only "An exception is thrown if the database is not open." Documenting the restriction for deserialize() while leaving close() undocumented is an odd asymmetry — probably worth a one-line addition there while you're in this section.

[TrevorBurnham]

Looks ready to 🚢 to me.

One non-blocking note: database.close() is guarded by the same IsInCallback() check as deserialize(), and throws ERR_INVALID_STATE under the same conditions, but its docs don't reflect that. I'd suggest copying the sentence you added to the docs for deserialize() over to the docs for database.close().

[trivikr]

The database.close() doc update us posted at #65090

I'll mark it ready for review post rebase after this PR is merged, as they share a reference link.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/75715/

[trivikr]

Landed in b2b7405

[trivikr]

Refs: #64795

deserialize() could be called from a user-defined function invoked during statement execution, tearing down the database connection while sqlite3_step() was still using it. Reuse the existing callback depth check to throw ERR_INVALID_STATE instead, matching the guard already in place for close().

---

Assisted-by: codex:gpt-5.6-sol

[nodejs-github-bot]

Review requested:

• @nodejs/sqlite

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.33%. Comparing base (bf2f995) to head (b2b7405).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #64796 +/- ##
==========================================
- Coverage 90.33% 90.33% -0.01% 
==========================================
Files 760 760 Lines 248522 248523 +1 Branches 46904 46906 +2 ==========================================
- Hits 224513 224511 -2 - Misses 15444 15451 +7 + Partials 8565 8561 -4 

Files with missing lines	Coverage Δ
src/node_sqlite.cc	81.23% <100.00%> (+<0.01%)	⬆️

... and 29 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.

[TrevorBurnham]

The guard itself looks right to me: IsInCallback() is the correct predicate, since the crash needs a live sqlite3_step() frame, which is only reachable from a callback, and every step-reentrant entry point (xFunc, xStepBase/xValueBase, the authorizer, sqlite3changeset_apply) already takes CallbackDepthGuard. Placing it before FinalizeStatements() also means a rejected call leaves no partial state. Three non-blocking notes below, plus one on the docs.

doc/api/sqlite.md: the new throwing condition isn't documented. The database.deserialize() section only mentions that existing statements are finalized first; it'd help to state that the method throws ERR_INVALID_STATE when called from a user-defined function, aggregate, authorizer, or changeset filter/conflict callback. The same is missing for database.close() from #64743 — might be worth adding both here.

[TrevorBurnham]

Thanks for the quick turnaround on the commit message and the test rename — both look good, and keeping the test with its deserialize siblings is a fair call.

The C++ guard is unchanged and still correct. I built this branch and confirmed the #64795 repro now throws instead of crashing, that coverage extends past the tested UDF case to aggregate and authorizer callbacks, and that there are no false positives across connections (deserializing an unrelated database from another database's callback still works). The remaining comments are all on the doc commit, plus one on the Fixes: line.

One more docs point that isn't in the diff: database.close() (line 291) has carried this same callback restriction since 5cef767, but its docs still say only "An exception is thrown if the database is not open." Documenting the restriction for deserialize() while leaving close() undocumented is an odd asymmetry — probably worth a one-line addition there while you're in this section.

[TrevorBurnham]

Looks ready to 🚢 to me.

One non-blocking note: database.close() is guarded by the same IsInCallback() check as deserialize(), and throws ERR_INVALID_STATE under the same conditions, but its docs don't reflect that. I'd suggest copying the sentence you added to the docs for deserialize() over to the docs for database.close().

[trivikr]

The database.close() doc update us posted at #65090

I'll mark it ready for review post rebase after this PR is merged, as they share a reference link.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/75715/

[trivikr]

Landed in b2b7405

[trivikr]

Refs: #64795

deserialize() could be called from a user-defined function invoked during statement execution, tearing down the database connection while sqlite3_step() was still using it. Reuse the existing callback depth check to throw ERR_INVALID_STATE instead, matching the guard already in place for close().

---

Assisted-by: codex:gpt-5.6-sol

[nodejs-github-bot]

Review requested:

• @nodejs/sqlite

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.33%. Comparing base (bf2f995) to head (b2b7405).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #64796 +/- ##
==========================================
- Coverage 90.33% 90.33% -0.01% 
==========================================
Files 760 760 Lines 248522 248523 +1 Branches 46904 46906 +2 ==========================================
- Hits 224513 224511 -2 - Misses 15444 15451 +7 + Partials 8565 8561 -4 

Files with missing lines	Coverage Δ
src/node_sqlite.cc	81.23% <100.00%> (+<0.01%)	⬆️

... and 29 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.

[TrevorBurnham]

The guard itself looks right to me: IsInCallback() is the correct predicate, since the crash needs a live sqlite3_step() frame, which is only reachable from a callback, and every step-reentrant entry point (xFunc, xStepBase/xValueBase, the authorizer, sqlite3changeset_apply) already takes CallbackDepthGuard. Placing it before FinalizeStatements() also means a rejected call leaves no partial state. Three non-blocking notes below, plus one on the docs.

doc/api/sqlite.md: the new throwing condition isn't documented. The database.deserialize() section only mentions that existing statements are finalized first; it'd help to state that the method throws ERR_INVALID_STATE when called from a user-defined function, aggregate, authorizer, or changeset filter/conflict callback. The same is missing for database.close() from #64743 — might be worth adding both here.

[TrevorBurnham]

Thanks for the quick turnaround on the commit message and the test rename — both look good, and keeping the test with its deserialize siblings is a fair call.

The C++ guard is unchanged and still correct. I built this branch and confirmed the #64795 repro now throws instead of crashing, that coverage extends past the tested UDF case to aggregate and authorizer callbacks, and that there are no false positives across connections (deserializing an unrelated database from another database's callback still works). The remaining comments are all on the doc commit, plus one on the Fixes: line.

One more docs point that isn't in the diff: database.close() (line 291) has carried this same callback restriction since 5cef767, but its docs still say only "An exception is thrown if the database is not open." Documenting the restriction for deserialize() while leaving close() undocumented is an odd asymmetry — probably worth a one-line addition there while you're in this section.

[TrevorBurnham]

Looks ready to 🚢 to me.

One non-blocking note: database.close() is guarded by the same IsInCallback() check as deserialize(), and throws ERR_INVALID_STATE under the same conditions, but its docs don't reflect that. I'd suggest copying the sentence you added to the docs for deserialize() over to the docs for database.close().

[trivikr]

The database.close() doc update us posted at #65090

I'll mark it ready for review post rebase after this PR is merged, as they share a reference link.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/75715/

[trivikr]

Landed in b2b7405

[trivikr]

Refs: #64795

deserialize() could be called from a user-defined function invoked during statement execution, tearing down the database connection while sqlite3_step() was still using it. Reuse the existing callback depth check to throw ERR_INVALID_STATE instead, matching the guard already in place for close().

---

Assisted-by: codex:gpt-5.6-sol

[nodejs-github-bot]

Review requested:

• @nodejs/sqlite

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.33%. Comparing base (bf2f995) to head (b2b7405).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #64796 +/- ##
==========================================
- Coverage 90.33% 90.33% -0.01% 
==========================================
Files 760 760 Lines 248522 248523 +1 Branches 46904 46906 +2 ==========================================
- Hits 224513 224511 -2 - Misses 15444 15451 +7 + Partials 8565 8561 -4 

Files with missing lines	Coverage Δ
src/node_sqlite.cc	81.23% <100.00%> (+<0.01%)	⬆️

... and 29 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.

[TrevorBurnham]

The guard itself looks right to me: IsInCallback() is the correct predicate, since the crash needs a live sqlite3_step() frame, which is only reachable from a callback, and every step-reentrant entry point (xFunc, xStepBase/xValueBase, the authorizer, sqlite3changeset_apply) already takes CallbackDepthGuard. Placing it before FinalizeStatements() also means a rejected call leaves no partial state. Three non-blocking notes below, plus one on the docs.

doc/api/sqlite.md: the new throwing condition isn't documented. The database.deserialize() section only mentions that existing statements are finalized first; it'd help to state that the method throws ERR_INVALID_STATE when called from a user-defined function, aggregate, authorizer, or changeset filter/conflict callback. The same is missing for database.close() from #64743 — might be worth adding both here.

[TrevorBurnham]

Thanks for the quick turnaround on the commit message and the test rename — both look good, and keeping the test with its deserialize siblings is a fair call.

The C++ guard is unchanged and still correct. I built this branch and confirmed the #64795 repro now throws instead of crashing, that coverage extends past the tested UDF case to aggregate and authorizer callbacks, and that there are no false positives across connections (deserializing an unrelated database from another database's callback still works). The remaining comments are all on the doc commit, plus one on the Fixes: line.

One more docs point that isn't in the diff: database.close() (line 291) has carried this same callback restriction since 5cef767, but its docs still say only "An exception is thrown if the database is not open." Documenting the restriction for deserialize() while leaving close() undocumented is an odd asymmetry — probably worth a one-line addition there while you're in this section.

[TrevorBurnham]

Looks ready to 🚢 to me.

One non-blocking note: database.close() is guarded by the same IsInCallback() check as deserialize(), and throws ERR_INVALID_STATE under the same conditions, but its docs don't reflect that. I'd suggest copying the sentence you added to the docs for deserialize() over to the docs for database.close().

[trivikr]

The database.close() doc update us posted at #65090

I'll mark it ready for review post rebase after this PR is merged, as they share a reference link.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/75715/

[trivikr]

Landed in b2b7405

[trivikr]

Refs: #64795

deserialize() could be called from a user-defined function invoked during statement execution, tearing down the database connection while sqlite3_step() was still using it. Reuse the existing callback depth check to throw ERR_INVALID_STATE instead, matching the guard already in place for close().

---

Assisted-by: codex:gpt-5.6-sol

[nodejs-github-bot]

Review requested:

• @nodejs/sqlite

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.33%. Comparing base (bf2f995) to head (b2b7405).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #64796 +/- ##
==========================================
- Coverage 90.33% 90.33% -0.01% 
==========================================
Files 760 760 Lines 248522 248523 +1 Branches 46904 46906 +2 ==========================================
- Hits 224513 224511 -2 - Misses 15444 15451 +7 + Partials 8565 8561 -4 

Files with missing lines	Coverage Δ
src/node_sqlite.cc	81.23% <100.00%> (+<0.01%)	⬆️

... and 29 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.

[TrevorBurnham]

The guard itself looks right to me: IsInCallback() is the correct predicate, since the crash needs a live sqlite3_step() frame, which is only reachable from a callback, and every step-reentrant entry point (xFunc, xStepBase/xValueBase, the authorizer, sqlite3changeset_apply) already takes CallbackDepthGuard. Placing it before FinalizeStatements() also means a rejected call leaves no partial state. Three non-blocking notes below, plus one on the docs.

doc/api/sqlite.md: the new throwing condition isn't documented. The database.deserialize() section only mentions that existing statements are finalized first; it'd help to state that the method throws ERR_INVALID_STATE when called from a user-defined function, aggregate, authorizer, or changeset filter/conflict callback. The same is missing for database.close() from #64743 — might be worth adding both here.

[TrevorBurnham]

Thanks for the quick turnaround on the commit message and the test rename — both look good, and keeping the test with its deserialize siblings is a fair call.

The C++ guard is unchanged and still correct. I built this branch and confirmed the #64795 repro now throws instead of crashing, that coverage extends past the tested UDF case to aggregate and authorizer callbacks, and that there are no false positives across connections (deserializing an unrelated database from another database's callback still works). The remaining comments are all on the doc commit, plus one on the Fixes: line.

One more docs point that isn't in the diff: database.close() (line 291) has carried this same callback restriction since 5cef767, but its docs still say only "An exception is thrown if the database is not open." Documenting the restriction for deserialize() while leaving close() undocumented is an odd asymmetry — probably worth a one-line addition there while you're in this section.

[TrevorBurnham]

Looks ready to 🚢 to me.

One non-blocking note: database.close() is guarded by the same IsInCallback() check as deserialize(), and throws ERR_INVALID_STATE under the same conditions, but its docs don't reflect that. I'd suggest copying the sentence you added to the docs for deserialize() over to the docs for database.close().

[trivikr]

The database.close() doc update us posted at #65090

I'll mark it ready for review post rebase after this PR is merged, as they share a reference link.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/75715/

[trivikr]

Landed in b2b7405

[trivikr]

Refs: #64795

deserialize() could be called from a user-defined function invoked during statement execution, tearing down the database connection while sqlite3_step() was still using it. Reuse the existing callback depth check to throw ERR_INVALID_STATE instead, matching the guard already in place for close().

---

Assisted-by: codex:gpt-5.6-sol

[nodejs-github-bot]

Review requested:

• @nodejs/sqlite

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.33%. Comparing base (bf2f995) to head (b2b7405).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #64796 +/- ##
==========================================
- Coverage 90.33% 90.33% -0.01% 
==========================================
Files 760 760 Lines 248522 248523 +1 Branches 46904 46906 +2 ==========================================
- Hits 224513 224511 -2 - Misses 15444 15451 +7 + Partials 8565 8561 -4 

Files with missing lines	Coverage Δ
src/node_sqlite.cc	81.23% <100.00%> (+<0.01%)	⬆️

... and 29 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.

[TrevorBurnham]

The guard itself looks right to me: IsInCallback() is the correct predicate, since the crash needs a live sqlite3_step() frame, which is only reachable from a callback, and every step-reentrant entry point (xFunc, xStepBase/xValueBase, the authorizer, sqlite3changeset_apply) already takes CallbackDepthGuard. Placing it before FinalizeStatements() also means a rejected call leaves no partial state. Three non-blocking notes below, plus one on the docs.

doc/api/sqlite.md: the new throwing condition isn't documented. The database.deserialize() section only mentions that existing statements are finalized first; it'd help to state that the method throws ERR_INVALID_STATE when called from a user-defined function, aggregate, authorizer, or changeset filter/conflict callback. The same is missing for database.close() from #64743 — might be worth adding both here.

[TrevorBurnham]

Thanks for the quick turnaround on the commit message and the test rename — both look good, and keeping the test with its deserialize siblings is a fair call.

The C++ guard is unchanged and still correct. I built this branch and confirmed the #64795 repro now throws instead of crashing, that coverage extends past the tested UDF case to aggregate and authorizer callbacks, and that there are no false positives across connections (deserializing an unrelated database from another database's callback still works). The remaining comments are all on the doc commit, plus one on the Fixes: line.

One more docs point that isn't in the diff: database.close() (line 291) has carried this same callback restriction since 5cef767, but its docs still say only "An exception is thrown if the database is not open." Documenting the restriction for deserialize() while leaving close() undocumented is an odd asymmetry — probably worth a one-line addition there while you're in this section.

[TrevorBurnham]

Looks ready to 🚢 to me.

One non-blocking note: database.close() is guarded by the same IsInCallback() check as deserialize(), and throws ERR_INVALID_STATE under the same conditions, but its docs don't reflect that. I'd suggest copying the sentence you added to the docs for deserialize() over to the docs for database.close().

[trivikr]

The database.close() doc update us posted at #65090

I'll mark it ready for review post rebase after this PR is merged, as they share a reference link.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/75715/

[trivikr]

Landed in b2b7405

[trivikr]

Refs: #64795

deserialize() could be called from a user-defined function invoked during statement execution, tearing down the database connection while sqlite3_step() was still using it. Reuse the existing callback depth check to throw ERR_INVALID_STATE instead, matching the guard already in place for close().

---

Assisted-by: codex:gpt-5.6-sol

[nodejs-github-bot]

Review requested:

• @nodejs/sqlite

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.33%. Comparing base (bf2f995) to head (b2b7405).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #64796 +/- ##
==========================================
- Coverage 90.33% 90.33% -0.01% 
==========================================
Files 760 760 Lines 248522 248523 +1 Branches 46904 46906 +2 ==========================================
- Hits 224513 224511 -2 - Misses 15444 15451 +7 + Partials 8565 8561 -4 

Files with missing lines	Coverage Δ
src/node_sqlite.cc	81.23% <100.00%> (+<0.01%)	⬆️

... and 29 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.

[TrevorBurnham]

The guard itself looks right to me: IsInCallback() is the correct predicate, since the crash needs a live sqlite3_step() frame, which is only reachable from a callback, and every step-reentrant entry point (xFunc, xStepBase/xValueBase, the authorizer, sqlite3changeset_apply) already takes CallbackDepthGuard. Placing it before FinalizeStatements() also means a rejected call leaves no partial state. Three non-blocking notes below, plus one on the docs.

doc/api/sqlite.md: the new throwing condition isn't documented. The database.deserialize() section only mentions that existing statements are finalized first; it'd help to state that the method throws ERR_INVALID_STATE when called from a user-defined function, aggregate, authorizer, or changeset filter/conflict callback. The same is missing for database.close() from #64743 — might be worth adding both here.

[TrevorBurnham]

Thanks for the quick turnaround on the commit message and the test rename — both look good, and keeping the test with its deserialize siblings is a fair call.

The C++ guard is unchanged and still correct. I built this branch and confirmed the #64795 repro now throws instead of crashing, that coverage extends past the tested UDF case to aggregate and authorizer callbacks, and that there are no false positives across connections (deserializing an unrelated database from another database's callback still works). The remaining comments are all on the doc commit, plus one on the Fixes: line.

One more docs point that isn't in the diff: database.close() (line 291) has carried this same callback restriction since 5cef767, but its docs still say only "An exception is thrown if the database is not open." Documenting the restriction for deserialize() while leaving close() undocumented is an odd asymmetry — probably worth a one-line addition there while you're in this section.

[TrevorBurnham]

Looks ready to 🚢 to me.

One non-blocking note: database.close() is guarded by the same IsInCallback() check as deserialize(), and throws ERR_INVALID_STATE under the same conditions, but its docs don't reflect that. I'd suggest copying the sentence you added to the docs for deserialize() over to the docs for database.close().

[trivikr]

The database.close() doc update us posted at #65090

I'll mark it ready for review post rebase after this PR is merged, as they share a reference link.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/75715/

[trivikr]

Landed in b2b7405

[trivikr]

Refs: #64795

deserialize() could be called from a user-defined function invoked during statement execution, tearing down the database connection while sqlite3_step() was still using it. Reuse the existing callback depth check to throw ERR_INVALID_STATE instead, matching the guard already in place for close().

---

Assisted-by: codex:gpt-5.6-sol

[nodejs-github-bot]

Review requested:

• @nodejs/sqlite

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.33%. Comparing base (bf2f995) to head (b2b7405).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #64796 +/- ##
==========================================
- Coverage 90.33% 90.33% -0.01% 
==========================================
Files 760 760 Lines 248522 248523 +1 Branches 46904 46906 +2 ==========================================
- Hits 224513 224511 -2 - Misses 15444 15451 +7 + Partials 8565 8561 -4 

Files with missing lines	Coverage Δ
src/node_sqlite.cc	81.23% <100.00%> (+<0.01%)	⬆️

... and 29 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.

[TrevorBurnham]

The guard itself looks right to me: IsInCallback() is the correct predicate, since the crash needs a live sqlite3_step() frame, which is only reachable from a callback, and every step-reentrant entry point (xFunc, xStepBase/xValueBase, the authorizer, sqlite3changeset_apply) already takes CallbackDepthGuard. Placing it before FinalizeStatements() also means a rejected call leaves no partial state. Three non-blocking notes below, plus one on the docs.

doc/api/sqlite.md: the new throwing condition isn't documented. The database.deserialize() section only mentions that existing statements are finalized first; it'd help to state that the method throws ERR_INVALID_STATE when called from a user-defined function, aggregate, authorizer, or changeset filter/conflict callback. The same is missing for database.close() from #64743 — might be worth adding both here.

[TrevorBurnham]

Thanks for the quick turnaround on the commit message and the test rename — both look good, and keeping the test with its deserialize siblings is a fair call.

The C++ guard is unchanged and still correct. I built this branch and confirmed the #64795 repro now throws instead of crashing, that coverage extends past the tested UDF case to aggregate and authorizer callbacks, and that there are no false positives across connections (deserializing an unrelated database from another database's callback still works). The remaining comments are all on the doc commit, plus one on the Fixes: line.

One more docs point that isn't in the diff: database.close() (line 291) has carried this same callback restriction since 5cef767, but its docs still say only "An exception is thrown if the database is not open." Documenting the restriction for deserialize() while leaving close() undocumented is an odd asymmetry — probably worth a one-line addition there while you're in this section.

[TrevorBurnham]

Looks ready to 🚢 to me.

One non-blocking note: database.close() is guarded by the same IsInCallback() check as deserialize(), and throws ERR_INVALID_STATE under the same conditions, but its docs don't reflect that. I'd suggest copying the sentence you added to the docs for deserialize() over to the docs for database.close().

[trivikr]

The database.close() doc update us posted at #65090

I'll mark it ready for review post rebase after this PR is merged, as they share a reference link.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/75715/

[trivikr]

Landed in b2b7405