Skip to content

Commit 2e65c4c

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 eac9abc commit 2e65c4c

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
@@ -4356,6 +4356,8 @@ void Session::Close(const FunctionCallbackInfo<Value>& args) {
43564356
env, session->session_ == nullptr, "session is not open");
43574357
THROW_AND_RETURN_ON_BAD_STATE(
43584358
env, session->is_generating_changeset_, "session is currently in use");
4359+
// Checked last: changeset generation runs the authorizer, so both conditions
4360+
// hold in that case and the more specific message above has to win.
43594361
THROW_AND_RETURN_IF_SESSION_IN_CALLBACK(env, session);
43604362

43614363
session->Delete();

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

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

756+
// Rejecting disposal has a cost: a `using` declaration inside a callback
757+
// demotes the block's own error to SuppressedError. Accepted for symmetry
758+
// with StatementSync's disposal, which throws for a busy statement the same
759+
// way. Pinned here so the trade-off is visible rather than surprising.
760+
it('demotes a callback error when disposal is rejected', (t) => {
761+
const database = new DatabaseSync(':memory:');
762+
database.exec('CREATE TABLE data(key INTEGER PRIMARY KEY)');
763+
let caught;
764+
765+
database.function('f', (x) => {
766+
try {
767+
using session = database.createSession();
768+
t.assert.ok(session);
769+
throw new Error('callback error');
770+
} catch (err) {
771+
caught = err;
772+
}
773+
return x;
774+
});
775+
776+
database.exec('SELECT f(1)');
777+
t.assert.ok(caught instanceof SuppressedError);
778+
t.assert.strictEqual(caught.suppressed.message, 'callback error');
779+
t.assert.strictEqual(
780+
caught.error.message,
781+
'session cannot be closed while in a callback',
782+
);
783+
});
784+
756785
it('leaves an already closed session disposable from a callback', (t) => {
757786
const database = new DatabaseSync(':memory:');
758787
database.exec('CREATE TABLE data(key INTEGER PRIMARY KEY)');

0 commit comments

Comments
 (0)