Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 3 additions & 29 deletions src/js/internal/sql/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,35 +128,9 @@ class Query<T, Handle extends BaseQueryHandle<any>> extends PublicPromise<T> {
}

async #runAsync() {
const { [_handler]: handler, [_queryStatus]: status } = this;

if (
status &
(SQLQueryStatus.executed | SQLQueryStatus.error | SQLQueryStatus.cancelled | SQLQueryStatus.invalidHandle)
) {
return;
}

if (this[_flags] & SQLQueryFlags.notTagged) {
this.reject(this[_adapter].notTaggedCallError());
return;
}

this[_queryStatus] |= SQLQueryStatus.executed;
const handle = this.#getQueryHandle();

if (!handle) {
return this;
}

await Promise.$resolve();

try {
return handler(this, handle);
} catch (err) {
this[_queryStatus] |= SQLQueryStatus.error;
this.reject(err as Error);
}
// Enqueue synchronously (same as execute()) so a same-tick close() sees
// the query as pending; #run()'s executed-status guard handles re-entry.
Comment on lines +131 to +132

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code

return this.#run();
}

get active() {
Expand Down
2 changes: 1 addition & 1 deletion src/js/internal/sql/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1228,7 +1228,7 @@ abstract class BaseSQLAdapter<PooledConnection extends BasePooledConnection, Con
}

let timeout = options?.timeout;
if (timeout) {
if (timeout != null) {
timeout = Number(timeout);
if (timeout > 2 ** 31 || timeout < 0 || timeout !== timeout) {
throw $ERR_INVALID_ARG_VALUE("options.timeout", timeout, "must be a non-negative integer less than 2^31");
Expand Down
10 changes: 10 additions & 0 deletions test/js/sql/sql-mysql.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -971,6 +971,16 @@ if (isDockerEnabled()) {
return expect(await promise).toEqual([{ x: 0 }]);
});

// Same contract as the .execute() case above, but via .then(): the query
// was handed to the pool one microtask late, so a same-tick end() ran
// first and rejected it with ERR_MYSQL_CONNECTION_CLOSED.
test("Connection end does not cancel a query awaited in the same tick", async () => {
const sql = new SQL({ ...getOptions(), max: 1 });
await sql`select 1 as x`;
const [rows] = await Promise.all([sql`select 1 as x`.then(r => r), sql.end()]);
expect(rows).toEqual([{ x: 1 }]);
});

test("Connection destroyed", async () => {
const sql = new SQL(getOptions());
process.nextTick(() => sql.end({ timeout: 0 }));
Expand Down
17 changes: 17 additions & 0 deletions test/js/sql/sql.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1790,6 +1790,23 @@ if (isDockerEnabled()) {
return expect(await promise).toEqual([{ x: "" }]);
});

// Same contract as the .execute() case above, but via .then(): the query
// was handed to the pool one microtask late, so a same-tick end() ran
// first and rejected it with ERR_POSTGRES_CONNECTION_CLOSED.
test("Connection end does not cancel a query awaited in the same tick", async () => {
const sql = postgres({ ...options, max: 1 });
await sql`select 1 as x`;
const [rows] = await Promise.all([sql`select 1 as x`.then(r => r), sql.end()]);
expect(rows).toEqual([{ x: 1 }]);
});

test("Connection end with a timeout does not cancel a query awaited in the same tick", async () => {
const sql = postgres({ ...options, max: 1 });
await sql`select 1 as x`;
const [rows] = await Promise.all([sql`select 1 as x`.then(r => r), sql.end({ timeout: 5 })]);
expect(rows).toEqual([{ x: 1 }]);
});

test("Connection destroyed", async () => {
const sql = postgres(options);
process.nextTick(() => sql.end({ timeout: 0 }));
Expand Down
14 changes: 12 additions & 2 deletions test/js/sql/sqlite-sql.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { randomUUIDv7, SQL } from "bun";
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, mock, test } from "bun:test";
import { tempDirWithFiles } from "harness";
import { isDebug, tempDirWithFiles } from "harness";
import { existsSync } from "node:fs";
import { rm, stat } from "node:fs/promises";
import { join } from "node:path";
Expand Down Expand Up @@ -1835,6 +1835,16 @@ describe("Connection management", () => {
}
});

// Query.then() used to defer the pool hand-off by one microtask, so a
// close() in the same synchronous block ran first, saw zero pending
// queries, and rejected the already-awaited query with "Connection closed".
test("close() drains a query awaited in the same tick", async () => {
const sql = new SQL("sqlite://:memory:");
await sql`SELECT 1 AS x`;
const [rows] = await Promise.all([sql`SELECT 42 AS x`.then(r => r), sql.close()]);
expect(rows).toEqual([{ x: 42 }]);
});

test("reserve throws for SQLite", async () => {
const sql = new SQL("sqlite://:memory:");

Expand Down Expand Up @@ -2014,7 +2024,7 @@ describe("Memory and resource management", () => {

await sql`CREATE TABLE stmt_test (id INTEGER PRIMARY KEY, value TEXT)`;

const iterations = 10000;
const iterations = isDebug ? 1000 : 10000;

for (let i = 0; i < iterations; i++) {
await sql`INSERT INTO stmt_test (id, value) VALUES (${i}, ${"test" + i})`;
Expand Down
Loading