Cloudflare/workerd: every sql.end() leaves an unhandled rejection (Stream was cancelled)
Nobody has claimed this yet.
- Dominant language
- JavaScript
- Stars
- 8.7k
- Forks
- 374
- Avg merge
- 11d 16h
- Merged PRs (30d)
- 1
Description
Summary
On the Cloudflare Workers build (cf/), every sql.end() leaves an unhandled promise rejection with Error: Stream was cancelled.
In workerd this is reported as an unhandled rejection on every close. Under @cloudflare/vitest-plugin it fails the test run — vitest exits non-zero on an unhandled error even when every test passes — and in a Worker it means one unhandled rejection per request for the common "client per request, ctx.waitUntil(sql.end())" pattern that the Cloudflare Hyperdrive docs recommend.
Version: postgres@3.4.9, wrangler@4.x, workerd via @cloudflare/vitest-plugin.
Reproduction
import postgres from 'postgres'
const sql = postgres(env.HYPERDRIVE.connectionString, { max: 1, fetch_types: false })
await sql`SELECT 1`
await sql.end()
// -> Unhandled Rejection: Error: Stream was cancelled.
// at read (node_modules/postgres/cf/polyfills.js:201)
Reproduces with await sql.end(), a non-awaited sql.end(), and sql.end({ timeout: 0 }). It does not reproduce if the connection is never closed.
Cause
cf/src/connection.js closed() tears the socket down:
socket.removeAllListeners()
socket = null
The socket polyfill's read() loop in cf/polyfills.js is still pending at that point. It rejects with Stream was cancelled, and its catch calls:
function error(err) {
tcp.emit('error', err) // <- no listeners left
tcp.emit('close')
}
tcp is a node:events EventEmitter, which throws when 'error' is emitted with no registered listener. That throw happens inside read(), an async function nobody awaits, so it surfaces as an unhandled rejection.
So it is a benign teardown race being escalated into an unhandled rejection by EventEmitter's special-casing of 'error'.
Suggested fix
Emit only when someone is listening:
function error(err) {
if (tcp.listenerCount('error') > 0)
tcp.emit('error', err)
tcp.emit('close')
}
This keeps the existing behaviour for every consumer that has a listener attached, and drops the post-teardown emission that has nowhere to go. Happy to open a PR if that shape looks right.
Workarounds considered
- Not calling
sql.end()— avoids it, but leaks a connection per client, which is not viable when the client is per-request. - The
options.socketfactory — cannot be used to attach a durable'error'listener, since it expects an already-connected socket andcf/polyfills.jsis not reachable through the package'sexportsmap.
We are currently carrying the one-line change above as a patch-package patch.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in cf/src/connection.js at closed() and follow the pending read loop in cf/polyfills.js through its error handler. Reproduce sql.end() in a Cloudflare/workerd or vitest environment, then verify teardown no longer creates an unhandled rejection while sockets with error listeners retain their existing behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript, node.js, postgresql
- Domain
- backend, cloud, databases
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 82/100