Skip to content

Commit 5037bc8

Browse files
committed
fixup! sqlite: reject closing a session from a callback
Cover the `using` form of the rejected disposal, whose block error is demoted to SuppressedError, and note why the callback check has to stay below the changeset check in Session::Close(). Signed-off-by: Trevor Burnham <trevorburnham@gmail.com>
1 parent 6184852 commit 5037bc8

2 files changed

Lines changed: 31 additions & 0 deletions

File tree

‎src/node_sqlite.cc‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4366,6 +4366,8 @@ void Session::Close(const FunctionCallbackInfo<Value>& args) {
43664366
env, session->session_ == nullptr, "session is not open");
43674367
THROW_AND_RETURN_ON_BAD_STATE(
43684368
env, session->is_generating_changeset_, "session is currently in use");
4369+
// Checked last: changeset generation runs the authorizer, so both conditions
4370+
// hold in that case and the more specific message above has to win.
43694371
THROW_AND_RETURN_IF_SESSION_IN_CALLBACK(env, session);
43704372

43714373
session->Delete();

‎test/parallel/test-sqlite-session.js‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -735,6 +735,35 @@ suite('session.close() - from a callback', () => {
735735
});
736736
}
737737

738+
// Rejecting disposal has a cost: a `using` declaration inside a callback
739+
// demotes the block's own error to SuppressedError. Accepted for symmetry
740+
// with StatementSync's disposal, which throws for a busy statement the same
741+
// way. Pinned here so the trade-off is visible rather than surprising.
742+
it('demotes a callback error when disposal is rejected', (t) => {
743+
const database = new DatabaseSync(':memory:');
744+
database.exec('CREATE TABLE data(key INTEGER PRIMARY KEY)');
745+
let caught;
746+
747+
database.function('f', (x) => {
748+
try {
749+
using session = database.createSession();
750+
t.assert.ok(session);
751+
throw new Error('callback error');
752+
} catch (err) {
753+
caught = err;
754+
}
755+
return x;
756+
});
757+
758+
database.exec('SELECT f(1)');
759+
t.assert.ok(caught instanceof SuppressedError);
760+
t.assert.strictEqual(caught.suppressed.message, 'callback error');
761+
t.assert.strictEqual(
762+
caught.error.message,
763+
'session cannot be closed while in a callback',
764+
);
765+
});
766+
738767
it('leaves an already closed session disposable from a callback', (t) => {
739768
const database = new DatabaseSync(':memory:');
740769
database.exec('CREATE TABLE data(key INTEGER PRIMARY KEY)');

0 commit comments

Comments
 (0)