porsager / porsager/postgres

Cloudflare/workerd: every sql.end() leaves an unhandled rejection (Stream was cancelled)

Open Beginner friendly
#1,202 0 comments 0 reactions 0 assignees View on GitHub

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.socket factory — cannot be used to attach a durable 'error' listener, since it expects an already-connected socket and cf/polyfills.js is not reachable through the package's exports map.

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

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.