sql(sqlite): reject an in-flight begin() when the instance is closed

[robobun]

Problem

• With the Bun.SQL SQLite adapter, the sql.begin(cb) promise stays pending forever when sql.close() runs during cb and cb never finishes. After a forced close, the PostgreSQL and MySQL adapters reject begin() with ERR_POSTGRES_CONNECTION_CLOSED / ERR_MYSQL_CONNECTION_CLOSED.
• If cb continues after the close, its queries reject with the raw RangeError: Cannot use a closed database, and begin() rejects with Error: Database has closed.
• The cause: begin() registers its close handler through the optional adapter hook attachConnectionCloseHandler (src/js/bun/sql.ts:650). SQLiteAdapter does not implement the hook, so close() (src/js/internal/sql/sqlite.ts:457) notifies nothing.

Fix

• SQLiteAdapter implements the hook and detachConnectionCloseHandler with a set of handlers. close() calls each handler with ERR_SQLITE_CONNECTION_CLOSED after the user onclose callback, the order the pooled adapters use.
• The handler is the shared onTransactionDisconnected. It marks the transaction closed and rejects begin(). Later tagged-template queries from cb reject with the same error, and no COMMIT or ROLLBACK goes to the closed database.
• close() on SQLite stays immediate and still ignores timeout.
• Verified: test/js/sql/sqlite-sql.test.ts. Two new tests fail on 1.4.3-canary.1 and pass with this change. Notes lists the other suites.

Background

• sql.begin(cb) returns a promise from Promise.withResolvers(). It settles when cb settles, or when the close handler of the connection fires.
• The PostgreSQL and MySQL adapters keep close handlers on each pooled connection and call them when the socket closes.
• The SQLite adapter has no pool. Its connection is one bun:sqlite Database, and close() closes it at once.

Notes

Repro (from the report):

import { SQL } from "bun";
const sql = new SQL({ adapter: "sqlite", filename: ":memory:" });
const state = {};
const watch = (name, p) => {
  state[name] = "PENDING";
  Promise.resolve(p).then(() => (state[name] = "resolved"), e => (state[name] = "rejected: " + (e.code ?? e.message)));
};
watch("begin", sql.begin(async tx => { await tx`select 1`; await new Promise(() => {}); }));
await new Promise(r => setTimeout(r, 100));
watch("close({ timeout: 1 })", sql.close({ timeout: 1 }));
await new Promise(r => setTimeout(r, 3000));
watch("query after close", sql`select 2`);
await new Promise(r => setTimeout(r, 500));
console.log(JSON.stringify(state)); process.exit(0);

• 1.4.3-canary.1: {"begin":"PENDING","close({ timeout: 1 })":"resolved","query after close":"rejected: ERR_SQLITE_CONNECTION_CLOSED"}
• This branch: {"begin":"rejected: ERR_SQLITE_CONNECTION_CLOSED","close({ timeout: 1 })":"resolved","query after close":"rejected: ERR_SQLITE_CONNECTION_CLOSED"}

What begin() does when close() runs during the callback, SQLite adapter:

callback after the close	1.4.3-canary.1	this branch
never settles	pending forever	rejects, ERR_SQLITE_CONNECTION_CLOSED
sends a tagged query	query: RangeError: Cannot use a closed database, begin(): Error: Database has closed	both reject, ERR_SQLITE_CONNECTION_CLOSED
returns a value	Error: Database has closed (from COMMIT)	rejects, ERR_SQLITE_CONNECTION_CLOSED
throws	Error: Database has closed (from ROLLBACK)	rejects, ERR_SQLITE_CONNECTION_CLOSED

The close rolls the open transaction back, before and after this change. Checked with a file database: the row that the callback inserted is gone when the file is opened again.

The same script against a local PostgreSQL with close({ timeout: 1 }) and a callback that never settles gives begin() rejected with ERR_POSTGRES_CONNECTION_CLOSED. That is the behavior this change matches. begin() is rejected before close() resolves on both adapters, and the first new test asserts that with Bun.peek.status, so a missing rejection fails at once and does not wait for the test timeout.

Not changed here:

• A graceful close() on PostgreSQL and MySQL waits for an open transaction, up to timeout seconds when one is given. The SQLite adapter never waited, and this change keeps that. To make it wait would turn a close() that resolves today into one that can stay pending.
• A tx.savepoint(cb) promise whose cb never settles stays pending after a forced close on every adapter (checked on PostgreSQL too). That promise belongs to an async function that awaits cb, so nothing outside can reject it. begin() still rejects.
• tx.unsafe() and tx.file() do not read the transaction state on any adapter. The shared code in src/js/bun/sql.ts:681 sends them to the captured connection. In a callback that outlives the close they still reject with the raw error: RangeError: Cannot use a closed database on SQLite, Error: connection must be a PostgresSQLConnection on PostgreSQL. The same gap lets a tx handle kept after begin() settled run unsafe() on the released connection. sql: reject tx.unsafe()/tx.file() on a settled transaction or released reserved handle #33751 had a fix for that and was closed as stale, not on merit. It is a separate change to shared code and it is tracked on its own.

Suites run with the debug build on Linux:

• test/js/sql/sqlite-sql.test.ts: 252 pass, 0 fail.
• test/js/sql/sql-onconnect-onclose-throw.test.ts, sqlite-url-parsing.test.ts, adapter-override.test.ts, adapter-env-var-precedence.test.ts, sql-close-pending-connection.test.ts: 289 pass, 0 fail.

---

[human-review] gate passed · iteration 1 · 2 files touched

fails on main (without fix)

ASAN without fix: 2 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/sql/sqlite-sql.test.ts
bun test v1.4.3 (c6b7fcb5b)

test/js/sql/sqlite-sql.test.ts:
(pass) Connection & Initialization > common default connection strings > should parse common connection strings [68.48ms]
(pass) Connection & Initialization > should connect to in-memory SQLite database [10.53ms]
(pass) Connection & Initialization > should connect to file-based SQLite database [39.99ms]
(pass) Connection & Initialization > should handle connection with options object [89.57ms]
(pass) Connection & Initialization > onconnect and onclose callbacks are invoked for SQLite [15.05ms]
(pass) Connection & Initialization > onconnect receives Error when open fails (readonly non-existent) [20.74ms]
(pass) Connection & Initialization > should create database file if it doesn't exist [36.05ms]
(pass) Connection & Initialization > should work with relative paths [33.87ms]
(pass) Connection & Initialization > Environment Variable Handling > should use DATABASE_URL for SQLite when it's a SQLite URL [40.98ms]
(pass) Connection & Initialization > E
... (truncated)

release without fix: all passed
bun test v1.4.3-canary.1 (c87a07137)

test/js/sql/sqlite-sql.test.ts:
(pass) Connection & Initialization > common default connection strings > should parse common connection strings [1.37ms]
(pass) Connection & Initialization > should connect to in-memory SQLite database [0.30ms]
(pass) Connection & Initialization > should connect to file-based SQLite database [1.20ms]
(pass) Connection & Initialization > should handle connection with options object [1.63ms]
(pass) Connection & Initialization > onconnect and onclose callbacks are invoked for SQLite [0.36ms]
(pass) Connection & Initialization > onconnect receives Error when open fails (readonly non-existent) [0.51ms]
(pass) Connection & Initialization > should create database file if it doesn't exist [13.45ms]
(pass) Connection & Initialization > should work with relative paths [14.04ms]
(pass) Connection & Initialization > Environment Variable Handling > should use DATABASE_URL for SQLite when it's a SQLite URL [12.21ms]
(pass) Connection & Initialization > Environment Variable Handling > should handle DATABASE_URL with :memory: [0.50ms]
(pass) Connection & Initialization > Environment Variable Handling > should han
... (truncated)

passes on PR (with fix)

ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/sql/sqlite-sql.test.ts
bun test v1.4.3 (c6b7fcb5b)

test/js/sql/sqlite-sql.test.ts:
(pass) Connection & Initialization > common default connection strings > should parse common connection strings [67.93ms]
(pass) Connection & Initialization > should connect to in-memory SQLite database [11.25ms]
(pass) Connection & Initialization > should connect to file-based SQLite database [39.02ms]
(pass) Connection & Initialization > should handle connection with options object [89.19ms]
(pass) Connection & Initialization > onconnect and onclose callbacks are invoked for SQLite [15.40ms]
(pass) Connection & Initialization > onconnect receives Error when open fails (readonly non-existent) [21.36ms]
(pass) Connection & Initialization > should create database file if it doesn't exist [41.55ms]
(pass) Connection & Initialization > should work with relative paths [36.73ms]
(pass) Connection & Initialization > Environment Variable Handling > should use DATABASE_URL for SQLite when it's a SQLite URL [44.42ms]
(pass) Connection & Initialization > E
... (truncated)

release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped) in 783ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[1/125] gen JS modules (bundle-modules)
Preprocess modules (6713ms)
Bundle modules (49ms)
Postprocesss modules (19ms)
Bundle Functions (420ms)
Generate Code (26ms)

[7.24s] Bundled "src/js" for production
  2603 kb
  197 internal modules
  13 native modules
  50 internal functions across 16 files
[1/8] cargo bun_runtime → libbun_runtime.a
�[1m�[33mwarning�[0m�[1m: binary `bun_shim_impl` should have a kebab-case name�[0m
   �[1m�[94m|�[0m
�[1m�[94m 1�[0m �[1m�[94m|�[0m /workspace/bun/build/release/rust-target/.../bun_shim_impl
   �[1m�[94m|�[0m                                              �[1m�[33m^^^^^^^^^^^^^�[0m
   �[1m�[94m|�[0m
   �[1m�[94m= �[0m�[1mnote�[0m: `cargo::non_kebab_case_bins` is set to `warn` by default
�[1m�[96mhelp�[0m: to change the binary name to `bun-shim-impl`, convert `bin.name`
  �[1m�[94m--> �[0msrc/install/windows-shim/Cargo.toml:41:8
   �[1m�[94m|�[0m
�[1m�[94m41�[0m �[91m- �[0mname = �[91m"bun_shim_impl"�[0m
�[1m�[94m41�[0m �[92m+ �[0mname = �[92m"bun-shim-impl"�[0m
   �[1m�[9
... (truncated)

diff hotspot

src/js/internal/sql/sqlite.ts  | 20 +++++++++++++++++++
 test/js/sql/sqlite-sql.test.ts | 44 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+)

gate history · 2 passed · 0 rejected · iteration 1

evidence per changed file

file                            reads  edits  tests
src/js/internal/sql/sqlite.ts       1      3     13
test/js/sql/sqlite-sql.test.ts      2      1     13

[robobun]

^{Updated 10:42 AM PT - Sep 17th, 2026}

✅ @robobun, your commit 1891e4de70f34e90fa5d2262992b697609abef7b passed in Build #117156! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 43116

That installs a local version of the PR into your bun-43116 executable, so you can run:

bun-43116 --bun

[robobun]

Status: reproduced and fixed here. Ready for review.

How I reproduced it: the script in the Notes block of the description. sql.begin() gets a callback that never settles, then sql.close({ timeout: 1 }) runs. On 1.4.3-canary.1 (c6b7fcb, Linux x64) the script prints {"begin":"PENDING","close({ timeout: 1 })":"resolved","query after close":"rejected: ERR_SQLITE_CONNECTION_CLOSED"}. With this branch begin is rejected: ERR_SQLITE_CONNECTION_CLOSED. The same script against a local PostgreSQL rejects begin() with ERR_POSTGRES_CONNECTION_CLOSED.

The two new tests in test/js/sql/sqlite-sql.test.ts (close() rejects a begin() whose callback is still running, a begin() callback that outlives close() gets ERR_SQLITE_CONNECTION_CLOSED from its queries) fail on that canary and pass with a debug build of this branch.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: 56bef9b0-7b5a-43c7-8688-18cfbe6cb081

📥 Commits

Reviewing files that changed from the base of the PR and between b52d513 and c87a071.

📒 Files selected for processing (2)

• src/js/internal/sql/sqlite.ts
• test/js/sql/sqlite-sql.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review.

---

Walkthrough

SQLiteAdapter now tracks active connection-close handlers and invokes them when the database closes. Tests cover suspended transactions and subsequent queries receiving ERR_SQLITE_CONNECTION_CLOSED.

Changes

SQLite connection closure

Layer / File(s)	Summary
Connection-close handler lifecycle src/js/internal/sql/sqlite.ts	SQLiteAdapter tracks active handlers, exposes attach and detach methods, and invokes remaining handlers with a connection-closed error after closing the database.
Forced closure transaction behavior test/js/sql/sqlite-sql.test.ts	Tests verify that forced closure rejects suspended transactions and causes later callback queries to receive ERR_SQLITE_CONNECTION_CLOSED.

Suggested reviewers: jarred-sumner

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 1891e

The SQLite close-handling change has no identified unresolved behavior that should block merging.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and concisely describes the main change: rejecting an in-flight SQLite begin() when the instance closes.
Description check	✅ Passed	The description clearly explains the problem, fix, behavior, scope, and verification results. It does not use the template headings exactly, but it provides the required information in equivalent sect…

---

_{Comment @coderabbitai help to get the list of available commands.}

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline finding, I also checked the close-handler re-entrancy and throw paths: the only handler registered is onTransactionDisconnected, which just flips connectionState and calls promise/Query reject (no detach during iteration, and the set is swapped before the loop anyway), so a throwing handler leaving begin() pending is not reachable from in-tree code. sql.reserve() throws for SQLite, so no reserved-connection handler path needs the same hook.

Extended reasoning...

Findings were reported inline, so this body only records what else was examined. The candidate "handler throws during close() leaves other begin() promises pending" was traced through src/js/bun/sql.ts onTransactionDisconnected: it sets the closed bit, rejects each tracked Query (Query.reject in src/js/internal/sql/query.ts only updates status flags and calls the stored reject), and rejects the transaction promise — none of which throws or mutates the adapter's handler set. The post-close COMMIT/ROLLBACK path was also checked: run_internal_transaction_sql short-circuits on the closed bit, so nothing is sent to the closed bun:sqlite Database and the duplicate reject is a no-op. Not approving because a verified finding is being posted inline and another verified finding was dropped from the posted set.