Skip to content

add sql listen, unlisten, notify for postgres - #29710

Closed
versecafe wants to merge 3 commits into
oven-sh:mainfrom
versecafe:pg-listen-notify
Closed

add sql listen, unlisten, notify for postgres#29710
versecafe wants to merge 3 commits into
oven-sh:mainfrom
versecafe:pg-listen-notify

Conversation

@versecafe

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds sql.listen, sql.unlisten, sql.notify support for postgres matching the postgres.js api

import { SQL } from "bun";
const sql = new SQL("postgres://localhost/mydb");

const { state, unlisten } = await sql.listen(
  "user_events",
  payload => console.log("got", JSON.parse(payload)),
  () => console.log("listening on backend pid", state.pid),
);

await sql.notify("user_events", JSON.stringify({ type: "signup", id: 42 }));
// -> listening on backend pid 84231
// -> got { type: "signup", id: 42 }

// Auto-reconnects with exponential backoff if the connection drops;
// onlisten fires again, state.pid updates in place.

await unlisten();   // remove just this listener
await sql.close();  // closes the dedicated listen connection

How did you verify your code works?

17 tests added and some manual checks

@versecafe
versecafe requested a review from alii as a code owner April 25, 2026 06:08

@claude claude Bot left a comment

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.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@versecafe

versecafe commented Apr 25, 2026

Copy link
Copy Markdown
Contributor Author

could probably delete the commented out listen/notify pseudo code from robobun in sql.test.ts but didn't want to touch extra stuff

@coderabbitai

coderabbitai Bot commented Apr 25, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

Adds PostgreSQL LISTEN/NOTIFY pub/sub to Bun SQL: listen(), unlisten(), and notify() methods, a lazily-created shared listen connection, automatic reconnect with exponential backoff and jitter, in-place mutable listener state updates on reconnect, and per-channel failure tracking with idempotent unlisten behavior.

Changes

Cohort / File(s) Summary
Documentation
docs/runtime/sql.mdx
Adds LISTEN/NOTIFY docs and runnable example; documents runtime behavior (lazy dedicated listen connection, reconnect/backoff, shared state semantics), notify() semantics, error handling, and removes prior "not implemented" markers.
Type Definitions
packages/bun-types/sql.d.ts
Adds listen(), unlisten(), and notify() method signatures to the SQL class, including returned shared state and unlisten() promise.
Public JS Surface
src/js/bun/sql.ts, src/js/bun/.../sql
Adds listen, unlisten, and notify on the default SQL object; validates channel/payload; forwards to adapter when supported or returns a standardized "not supported" rejected Promise; ensures lazy initialization.
Postgres adapter implementation
src/js/internal/sql/postgres.ts
Implements LISTEN/NOTIFY subsystem: single reusable listen connection, per-channel handler sets and onlisten callbacks, coordinated registrations, exponential backoff + jitter and reconnect with re-issuing LISTEN, shared mutable { pid, secret } updated in-place, per-channel failure counting and drop logic, idempotent unlisten, and shutdown ordering to close listen subsystem first.
Connection classes & protocol
src/bun.js/api/sql.classes.ts, src/sql/postgres/PostgresSQLConnection.zig, src/sql/postgres/PostgresRequest.zig, src/sql/postgres/protocol/NotificationResponse.zig
Adds onnotification getter/setter and read-only processId/secretKey to connection class surface; extends protocol handling for 'A' (Notification) messages, decodes and dispatches notifications to JS via microtask (with safe buffer copies), and tightens decode error handling.
Native password handling
src/js/internal/sql/postgres.ts (helper)
Extracts resolvePostgresPassword() and ensures connection creation receives an eagerly-resolved password string.
Tests
test/js/sql/sql.test.ts
Adds comprehensive LISTEN/NOTIFY tests: end-to-end delivery, handler registration/unregistration semantics, shared listener state (PID/secret), reconnect behavior, argument validation, empty payloads, and unlisten idempotency.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'add sql listen, unlisten, notify for postgres' accurately and clearly summarizes the main changes in the PR, which adds three new methods to the SQL API for PostgreSQL.
Description check ✅ Passed The PR description follows the required template with both sections completed. It provides a clear example of the new API, explains the functionality including auto-reconnect behavior, and documents verification through 17 added tests.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 8

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/bun-types/sql.d.ts`:
- Around line 885-889: The listen declaration incorrectly types onlisten as
taking no arguments; update the listen signature so onlisten is typed to receive
the mutable listen state object (e.g. onlisten?: (state: { pid: number; secret:
number }) => void) so callers get the pid/secret on initial connect and
reconnect; keep the existing onnotify and return shape (state and unlisten)
unchanged and only change the onlisten parameter type in the listen function
declaration.

In `@src/js/bun/sql.ts`:
- Around line 940-956: Attach stable stubs for sql.listen, sql.unlisten, and
sql.notify even when pool.listen is missing so the API shape is consistent; if
pool.listen exists keep the real implementations (pool.listen, pool.unlisten,
sql.unsafe...), otherwise set sql.listen and sql.unlisten to functions that
immediately return a rejected Promise with a clear Error like "adapter doesn't
support LISTEN/NOTIFY", and set sql.notify to a stub that first validates
channel/payload the same way as the real notify (check non-empty string, no null
bytes, payload type) and then returns a rejected Promise with the same
adapter-unsupported Error; update the code paths around pool.listen, sql.listen,
sql.unlisten, and sql.notify (and the lazy default client code paths such as
lazyDefaultSQL / new SQL) so callers always see these methods but get a
consistent rejection when the adapter lacks LISTEN/NOTIFY.

In `@src/js/internal/sql/postgres.ts`:
- Line 1565: Extract a private helper to centralize channel quoting and replace
duplicated channel.replaceAll('"','""') occurrences: add a method like
`#quoteChannel`(channel) that returns the channel wrapped in double quotes with
internal quotes escaped (e.g. `"${...}"`), then update callers such as the
LISTEN/UNLISTEN invocations that call this.#runListenQuery(conn, `LISTEN
"${channel.replaceAll('"', '""')}"`) (and the similar UNLISTEN sites) to use
this.#quoteChannel(channel) instead (e.g. `LISTEN
${this.#quoteChannel(channel)}`) so all quoting logic is in one place.
- Around line 1430-1452: In `#closeListen`(), reset the backoff by setting
this.#listenReconnectDelay back to 250 (ms) when clearing timers/connection
state so a subsequent re-listen starts from the documented baseline; locate the
method named `#closeListen` and add the single assignment
this.#listenReconnectDelay = 250 alongside the existing clears for
`#listenReconnectTimer`, `#listenConnectPromise`, `#listenChannels`, and
`#listenChannelFailures`.
- Around line 1699-1712: The returned unlisten closure currently only forwards
onnotify to this.unlisten(channel, onnotify) and therefore leaves the onlisten
callback (registered in listen()) in `#listenOnlistenCallbacks`, causing it to
fire after reconnects; update the closure returned by listen() so it also
removes the specific onlisten callback from
this.#listenOnlistenCallbacks[channel] (remove only that callback from the Set,
and if the Set becomes empty delete the map entry) before/after calling
this.unlisten(channel, onnotify), mirroring the rollback cleanup logic in
listen() so the exact onlisten installed by this listen() call is removed even
when sibling listeners exist.
- Around line 1454-1515: The `#createListenConnection` method duplicates password
resolution and createPostgresConnection wiring that already exists in
PooledPostgresConnection.createConnection; extract a shared helper (e.g.,
resolvePassword(info)) to perform the password callback/promise unwrap used in
both places, and create a thin connection-factory function that encapsulates the
createPostgresConnection invocation and its success/error callbacks so both
`#createListenConnection` and PooledPostgresConnection.createConnection call the
same factory; update `#createListenConnection` to call resolvePassword(info) and
the shared factory (keeping the existing listen-specific logic such as setting
`#listenConnection`, `#listenState`, onnotification, ref(), and reconnect handling).
- Around line 1665-1693: The current race comes from using isNewChannel computed
after we already inserted this call's handler into this.#listenChannels, so
concurrent listen() calls can skip issuing LISTEN and leave a handler registered
without an active LISTEN; fix by detecting whether THIS call actually
created/added the channel (capture previous size/creation status before
inserting) and only skip `#runListenQuery` when a previous caller truly issued
LISTEN, e.g. read prevSize = this.#listenChannels.get(channel)?.size ?? 0, if
prevSize === 0 then this call is responsible to await
this.#ensureListenConnection() and await this.#runListenQuery(...), otherwise
just add handler and don’t assume LISTEN succeeded; alternatively always call
`#runListenQuery` (idempotent on server) and keep the existing rollback logic
around `#listenChannels` and `#listenOnlistenCallbacks` using those same symbols
(`#listenChannels`, `#listenOnlistenCallbacks`, `#ensureListenConnection`,
`#runListenQuery`) so failed LISTEN removes only this call's handlers.

In `@test/js/sql/sql.test.ts`:
- Around line 12388-12419: The LISTEN/NOTIFY tests (e.g., the tests named
"sql.listen and sql.notify round-trip a payload" and "sql.listen receives
multiple notifications in order") currently await Promise.withResolvers()
without a per-assertion timeout, which can hang the suite; update each test that
uses Promise.withResolvers() together with db.listen/db.notify (also at the
other noted ranges) to await the subscriber-resolved promise with a bounded
timeout: wrap the Promise.withResolvers().promise in a timeout helper (e.g.,
withTimeout) or race it against a short timeout and fail if it elapses, and
ensure the resolver is invoked from the db.listen callback when the expected
condition is met, then unlisten as before; reference db.listen, db.notify,
db.unlisten, Promise.withResolvers, and the test names to locate the places to
change.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 0c19f071-45b5-41c8-b170-bac9a5545dce

📥 Commits

Reviewing files that changed from the base of the PR and between 75b7475 and 05630a8.

📒 Files selected for processing (9)
  • docs/runtime/sql.mdx
  • packages/bun-types/sql.d.ts
  • src/bun.js/api/sql.classes.ts
  • src/js/bun/sql.ts
  • src/js/internal/sql/postgres.ts
  • src/sql/postgres/PostgresRequest.zig
  • src/sql/postgres/PostgresSQLConnection.zig
  • src/sql/postgres/protocol/NotificationResponse.zig
  • test/js/sql/sql.test.ts

Comment thread packages/bun-types/sql.d.ts
Comment thread src/js/bun/sql.ts
Comment thread src/js/internal/sql/postgres.ts
Comment thread src/js/internal/sql/postgres.ts
Comment thread src/js/internal/sql/postgres.ts Outdated
Comment thread src/js/internal/sql/postgres.ts Outdated
Comment thread src/js/internal/sql/postgres.ts
Comment thread test/js/sql/sql.test.ts

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/js/bun/sql.ts`:
- Around line 962-963: The helper unsupported() currently returns
Promise.reject(...) which is inconsistent and uses a tamperable public API;
change its implementation to return the JSC intrinsic Promise.$reject(new
Error("LISTEN/NOTIFY is not supported by this adapter (PostgreSQL only)")) so it
matches other rejections in this file (see uses of Promise.$reject) and keeps
private global usage consistent with the coding guidelines.

In `@src/js/internal/sql/postgres.ts`:
- Line 1556: The reconnect backoff timer (this.#listenReconnectTimer) can keep
the process alive after unlisten() because it isn't unref()'d; modify the code
that assigns this.#listenReconnectTimer = setTimeout(...) to call .unref() on
the returned timer handle (and keep the reference for potential clearTimeout),
and also update unlisten() to clear and null the timer when `#listenChannels`
becomes empty to be safe; reference the private field
this.#listenReconnectTimer, the unlisten() method, and the existing
conn.ref()/conn.unref() logic when adding the unref() and clearTimeout handling.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3161425e-5110-4799-b363-2f05a7e006b5

📥 Commits

Reviewing files that changed from the base of the PR and between 05630a8 and 4659ea5.

📒 Files selected for processing (3)
  • packages/bun-types/sql.d.ts
  • src/js/bun/sql.ts
  • src/js/internal/sql/postgres.ts

Comment thread src/js/bun/sql.ts Outdated
Comment thread src/js/internal/sql/postgres.ts Outdated

@coderabbitai coderabbitai Bot left a comment

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.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/js/internal/sql/postgres.ts (1)

1035-1047: ⚠️ Potential issue | 🟡 Minor

Move #closeListen() after the options.timeout validation.

this.#closeListen() runs unconditionally at the top of close(), before Number(timeout) / range validation. If the caller passes an invalid timeout (NaN, negative, or > 2**31), the listen connection is already torn down and #listenChannels / #listenOnlistenCallbacks are cleared, but then the function throws and this.closed is never set. The listen subsystem is now permanently dead while the pool is still "open", and a second close() call also re-runs #closeListen() (no-op) and throws again on the same bad input.

Pre-PR, validation came before the only destructive step (this.closed = true + #close()). Restoring that ordering keeps close() failure-atomic w.r.t. the listen subsystem.

♻️ Suggested fix
   async close(options?: { timeout?: number }): Promise<void> {
     if (this.closed) {
       return;
     }
 
-    this.#closeListen();
-
     let timeout = options?.timeout;
     if (timeout) {
       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");
       }
+      this.#closeListen();
       this.closed = true;
       ...
     } else {
+      this.#closeListen();
       this.closed = true;
       ...
     }
   }

Or simply hoist the validation above #closeListen() and keep a single call site.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/js/internal/sql/postgres.ts` around lines 1035 - 1047, The close() method
currently calls the private `#closeListen`() before validating options.timeout,
which can destructively tear down listen state then throw and leave this.closed
false; move the timeout validation (Number conversion and range/NaN checks for
options.timeout) to run before calling this.#closeListen() so any invalid
timeout throws without altering listen state, then proceed to call
this.#closeListen() and the rest of the teardown (this.closed and `#close`) as
before; target the async close(...) function and its timeout handling code paths
to apply this reorder, keeping a single call site for this.#closeListen().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/js/internal/sql/postgres.ts`:
- Around line 1035-1047: The close() method currently calls the private
`#closeListen`() before validating options.timeout, which can destructively tear
down listen state then throw and leave this.closed false; move the timeout
validation (Number conversion and range/NaN checks for options.timeout) to run
before calling this.#closeListen() so any invalid timeout throws without
altering listen state, then proceed to call this.#closeListen() and the rest of
the teardown (this.closed and `#close`) as before; target the async close(...)
function and its timeout handling code paths to apply this reorder, keeping a
single call site for this.#closeListen().

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: ba79efac-17b5-49fd-b4dc-51fdfa5b81fe

📥 Commits

Reviewing files that changed from the base of the PR and between 4659ea5 and ba2089c.

📒 Files selected for processing (2)
  • src/js/bun/sql.ts
  • src/js/internal/sql/postgres.ts

Pom4H added a commit to Pom4H/workflow that referenced this pull request Apr 26, 2026
Extract the pg LISTEN/NOTIFY transport (with self-healing reconnect)
into a `ListenAdapter` interface so the streamer no longer hard-depends
on `pg.Client`. Default behaviour is unchanged — `createPgListenAdapter`
preserves the exponential backoff (250ms→30s) reconnect loop and the
streamer keeps its 5s polling fallback for chunks missed during gaps.

Adds:
- `ListenAdapter` / `ListenSubscription` types
- `createPgListenAdapter(pool)` — production default
- `createBunSqlListenAdapter()` — stub that throws until oven-sh/bun#29710
  lands native LISTEN/NOTIFY in `bun:sql`
- `listenAdapter` config option on `PostgresWorldConfig`

Re-exports the new symbols from the package entry. Tests: 104 passing.
@versecafe

Copy link
Copy Markdown
Contributor Author

leaving this to be converted by robobun/claude to the rust codebase

@robobun

robobun commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

Opened #32089 with this branch rebased onto main: the native side is now implemented in the Rust port (src/sql_jsc/postgres/) since the .zig files are no longer compiled. Your three commits are preserved unchanged on that branch, so the JS/types/docs/tests stay credited to you.

@robobun

robobun commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Thanks for this, @versecafe. As you suggested, this branch was carried over to the Rust port as #32089: it started from your commits (you are credited as co-author on the commits there), and it has since been reworked there following maintainer review (listen() now resolves to a subscription object with unlisten() and await using, sql.unlisten() and the state object were dropped at alii's request). The dedicated listen connection, reconnect with backoff, onlisten on every (re)subscribe and pg_notify based notify() all came from here.

The .zig files this PR modifies no longer exist on main, so closing this one in favor of #32089, which is the one under review now. If you want to weigh in on the API shape, that thread is the place.

@robobun robobun closed this Aug 12, 2026
cirospaciari added a commit that referenced this pull request Aug 14, 2026
### What does this PR do?

Adds `sql.listen()` and `sql.notify()` for PostgreSQL
(`LISTEN`/`NOTIFY`). Originally #29710 by @versecafe; this is a
from-scratch rewrite of both the native and the JS side, rebased onto
current main (design and numbers below, the earlier implementation is
referred to as v1).

```ts
import { sql } from "bun";

const subscription = await sql.listen("orders", payload => {
  console.log("new order", JSON.parse(payload));
});

await sql.notify("orders", JSON.stringify({ id: 42 }));
await subscription.unlisten();

// or scoped:
await using subscription = await sql.listen("orders", handleOrder);
```

- `listen(channel, onnotify, onlisten?)` resolves once the server
acknowledged the `LISTEN`, with a `ListenSubscription` (`channel`,
`unlisten()`, `Symbol.asyncDispose`) that removes exactly the
registration that call made. Every call is its own registration; several
on one channel share one server-side `LISTEN`. `onlisten()` runs on the
acknowledgement and again after every reconnect (the place to catch up
on missed notifications). Shape per the review discussion below; there
is no `sql.unlisten()` and no pid/secret exposure.
- `notify(channel, payload?)` is `SELECT pg_notify($1, $2)` on the
handle it is called on, so `tx.notify()` is delivered on commit and
dropped on rollback. Reserved and transaction handles have both methods.
- All subscriptions of a client share one dedicated connection, opened
by the first `listen()` and closed when the last subscription is
removed; while anything is subscribed it keeps the process alive
(#32127). If it drops, it reconnects with jittered exponential backoff
(250ms to 32s) and re-subscribes every channel. `sql.close()` tears it
down, including mid-handshake.
- Channel names are quoted as identifiers and limited to PostgreSQL's 63
identifier bytes; a throwing callback is reported as an uncaught
exception. Non-Postgres adapters reject with "PostgreSQL only".

### Design

Hot path (one `NotificationResponse`), native side in
`PostgresSQLConnection.rs`:

1. `NotificationResponse::decode` yields the channel and payload as
slices into the read buffer (`string_within`), nothing is copied. v1
copied both into `Vec`s.
2. The channel slice is looked up in a small per-connection table of
interned `JSString`s (`Strong`, at most 256 entries), so after the first
notification on a channel the channel name costs a scan of that table
and no allocation. v1 created a new string every time.
3. The payload becomes one `JSString` (`create_utf8_for_js`, the string
owns the only copy).
4. The JS callback is invoked synchronously with `run_callback`, the
same way query results are delivered; exceptions surface as uncaught. v1
queued a microtask per notification.

`ref()` on a connection now sets `KEEP_ALIVE_REQUESTED` so the
idle-unref at the end of `on_data` leaves the listen connection ref'd.

JS side (`src/js/internal/sql/postgres.ts`): one `ListenConnection` per
adapter holding a `Map<channel, Channel>` plus the connection, sweep
timer and backoff fields; a `ListenSubscription` only remembers its
channel and callbacks and hands them back on `unlisten()`. Dispatch is
`Map.get` followed by a direct call (a lone listener is stored as the
bare function; several are stored in an array that is replaced rather
than mutated, so `listen()`/`unlisten()` from inside a callback cannot
disturb the dispatch in progress). A `Channel` entry's identity scopes
its `LISTEN` round trip: removing the last registration deletes the
entry synchronously, and anything resuming after an await compares its
entry against the map, which removes the re-add / unlisten-during-ack
races v1 needed separate bookkeeping for. There is exactly one repair
mechanism: a connection drop, connect failure or rejected `LISTEN` nulls
the affected entries' `ready` promise and arms `#scheduleSweep()`, which
re-issues `LISTEN` for those entries. v1 kept five channel-keyed maps
and sets (listeners, onlisten callbacks, in-flight round trips, channels
registered on the current connection, failure counters) consistent by
hand for the same job.

### Numbers

Both builds are release builds of this branch's base with the respective
implementation applied, measured against a scripted backend in a
separate process that writes pre-built frames (it sustains over 10M
frames/s into a sink, so the client is what is measured). 4 channels,
medians of 5 runs of 1M notifications, two passes each.

| payload | v1 | this PR |
| --- | --- | --- |
| 32 bytes | 3.65M to 4.10M notifications/s (244 to 274 ns each) | 4.91M
to 5.37M notifications/s (186 to 204 ns each) |
| 256 bytes | 2.74M to 2.83M/s | 2.86M to 2.89M/s (both bounded by
socket reads on this machine) |

JS heap objects per notification (`bun:jsc` `heapStats()` delta across a
blast with no collection in between): v1 allocated 2 strings plus a Set
iterator (until the JIT elides it), this PR allocates the 1 payload
string. Native allocations per notification went from two `Vec`s plus
two string buffers plus a microtask entry to one string buffer.

### How did you verify your code works?

`test/js/sql/postgres-listen-notify.test.ts`, 56 tests:

- 44 run against a scripted backend in the test process (which can hold
a `LISTEN` ack, fail one, or drop the connection on demand) or in a
subprocess: routing and ordering, thousands of notifications in one
read, UTF-8 and quoting, the 63-byte limit, listeners added or removed
from inside a callback, shared round trips, the same callback registered
twice being two subscriptions, `onlisten` timing, `unlisten()` and
`await using` removing exactly their own registration (idempotent, and a
no-op on a handle that outlived `close()`), a re-listen while the
UNLISTEN is in flight, reconnect (re-subscribe, an unlistened
subscription's `onlisten` no longer firing, retry with warning,
cancelled by unlistening, a shared listen() whose LISTEN is rejected
still repairing the other listeners), `close()` during a round trip,
during the handshake and in the same tick as a listen() on the live
connection, argument validation on Postgres and SQLite, `notify()` going
through the pool, and in a subprocess: the subscription keeping the
process alive (also across the backoff), `close()` letting it exit,
throwing `onnotify` and `onlisten` callbacks reaching
`uncaughtException` without affecting the other listeners, and an RSS
check over 2 x 24 MiB of payloads that fails when one string per
notification is leaked (verified by injecting that leak).
- 12 run against the docker postgres: payload round trips including
empty and omitted payloads, commit/rollback delivery, and the ten
LISTEN/NOTIFY tests of postgres.js's own suite ported under their
original names (weird and upper-case channel names, double listen,
unlisten variants, reconnect after the listening backend is terminated),
with their delays replaced by waiting for the deliveries. These also
pass against a local PostgreSQL 17.

All of them fail on main (`sql.listen is not a function`).

Fixes #18214
Fixes #32127

<!-- robobun:evidence:begin -->

---

**[review]** gate passed · iteration 30 · 11 files touched

<details><summary>fails on main (without fix)</summary>

```console
ASAN without fix: 44 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/sql/postgres-listen-notify.test.ts
bun test v1.4.0 (915989f)

test/js/sql/postgres-listen-notify.test.ts:
failed to connect to the docker API at unix:///var/run/docker.sock; check if the path is correct and if the daemon is running: dial unix /var/run/docker.sock: connect: no such file or directory
149 |     await using sql = client(server.url);
150 |     const got: string[] = [];
151 |     const third = gate();
152 |     const count = third.after(3);
153 | 
154 |     const subscription = await sql.listen("orders", payload => {
                                         ^
TypeError: sql.listen is not a function. (In 'sql.listen("orders", (payload) => {
      got.push(payload);
      count();
    })', 'sql.listen' is undefined)
      at <anonymous> (/workspace/bun/test/js/sql/postgres-listen-notify.test.ts:154:36)
(fail) listen > routes notifications to the channel's listener, in order [243.12ms]
177 |     const count = done.after(total);
178 |     const listener = (channel: keyof typeof counts) => () => {
179 |       counts[cha
... (truncated)

release without fix: all passed
bun test v1.4.0-canary.1 (06ec075)

test/js/sql/postgres-listen-notify.test.ts:
failed to connect to the docker API at unix:///var/run/docker.sock; check if the path is correct and if the daemon is running: dial unix /var/run/docker.sock: connect: no such file or directory
(pass) listen > routes notifications to the channel's listener, in order [19.61ms]
(pass) listen > thousands of notifications across channels arriving in one read [11.79ms]
(pass) listen > channel names and payloads are UTF-8 [2.00ms]
(pass) listen > channel names are quoted as identifiers [1.94ms]
(pass) listen > channel names are limited to PostgreSQL's 63 identifier bytes, counted in UTF-8 [2.79ms]
(pass) listen > several listeners on one channel share one LISTEN and each receives [2.13ms]
(pass) listen > concurrent listen() calls on a new channel share its round trip [2.06ms]
(pass) listen > a listener that unlistens itself mid-dispatch does not starve the others [3.26ms]
(pass) listen > a listener added from inside a callback receives the next notification, not the current one [2.05ms]
(pass) listen > registering the same callback twice is two subscriptions, each removed by its own handle [
... (truncated)
```

</details>

<details><summary>passes on PR (with fix)</summary>

```console
ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/sql/postgres-listen-notify.test.ts
bun test v1.4.0 (915989f)

test/js/sql/postgres-listen-notify.test.ts:
failed to connect to the docker API at unix:///var/run/docker.sock; check if the path is correct and if the daemon is running: dial unix /var/run/docker.sock: connect: no such file or directory
(pass) listen > routes notifications to the channel's listener, in order [583.97ms]
(pass) listen > thousands of notifications across channels arriving in one read [1465.10ms]
(pass) listen > channel names and payloads are UTF-8 [50.49ms]
(pass) listen > channel names are quoted as identifiers [43.60ms]
(pass) listen > channel names are limited to PostgreSQL's 63 identifier bytes, counted in UTF-8 [57.35ms]
(pass) listen > several listeners on one channel share one LISTEN and each receives [53.34ms]
(pass) listen > concurrent listen() calls on a new channel share its round trip [48.07ms]
(pass) listen > a listener that unlistens itself mid-dispatch does not starve the others [61.40ms]
(pass) listen > a listener added from inside a 
... (truncated)

release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped) in 678ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[1/30] gen ZigGeneratedClasses.{cpp,h,rs}
Found 2 classes from /workspace/bun/src/jsc/resolve_message.classes.ts
  - ResolveMessage (15 fields)
  - BuildMessage (10 fields)
Found 1 classes from /workspace/bun/src/runtime/api/Archive.classes.ts
  - Archive (4 fields, 1 class fields)
Found 2 classes from /workspace/bun/src/runtime/api/BunObject.classes.ts
  - ResourceUsage (8 fields)
  - Subprocess (20 fields)
Found 1 classes from /workspace/bun/src/runtime/api/cron.classes.ts
  - CronJob (5 fields)
Found 3 classes from /workspace/bun/src/runtime/api/filesystem_router.classes.ts
  - FileSystemRouter (5 fields)
  - FrameworkFileSystemRouter (2 fields)
  - MatchedRoute (8 fields)
Found 1 classes from /workspace/bun/src/runtime/api/Glob.classes.ts
  - Glob (5 fields)
Found 1 classes from /workspace/bun/src/runtime/api/h2.classes.ts
  - H2FrameParser (32 fields)
Found 9 classes from /workspace/bun/src/runtime/api/html_rewriter.classes.ts
  - HTMLRewriter (3 fields)
  - TextChunk (7 fields)
  - DocType (5 fields)
... (truncated)
```

</details>

<details><summary>diff hotspot</summary>

```
docs/runtime/sql.mdx                              |   60 +-
 packages/bun-types/sql.d.ts                       |   62 ++
 src/js/bun/sql.ts                                 |   51 +
 src/js/internal/sql/postgres.ts                   |  336 ++++++
 src/js/internal/sql/shared.ts                     |   13 +-
 src/runtime/api/sql.classes.ts                    |   14 +-
 src/sql/postgres/protocol/NotificationResponse.rs |   16 +-
 src/sql/shared/ConnectionFlags.rs                 |    2 +
 src/sql_jsc/jsc.rs                                |    4 +-
 src/sql_jsc/postgres/PostgresSQLConnection.rs     |   92 +-
 test/js/sql/postgres-listen-notify.test.ts        | 1238 +++++++++++++++++++++
 11 files changed, 1863 insertions(+), 25 deletions(-)
```

</details>

**gate history** · 8 passed · 1 rejected · iteration 30

<details><summary>evidence per changed file</summary>

```
file                                               reads  edits  tests
docs/runtime/sql.mdx                                   6     11      8
packages/bun-types/sql.d.ts                           12      8      6
src/js/bun/sql.ts                                     12     18     12
src/js/internal/sql/postgres.ts                       32     58     38
src/js/internal/sql/shared.ts                          1      0      3
src/runtime/api/sql.classes.ts                         2      2      1
src/sql/postgres/protocol/NotificationResponse.rs      3      2      0
src/sql/shared/ConnectionFlags.rs                      2      2      0
src/sql_jsc/jsc.rs                                     2      4      1
src/sql_jsc/postgres/PostgresSQLConnection.rs         14     18      2
test/js/sql/postgres-listen-notify.test.ts            17     38      0
```

</details>

<!-- robobun:evidence:end -->

---------

Co-authored-by: versecafe <147033096+versecafe@users.noreply.github.com>
Co-authored-by: Ciro Spaciari <ciro.spaciari@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants