Cloudflare polyfill: unhandled rejection when a connection closes (read() emits 'error' after listeners are removed)
Nobody has claimed this yet.
- Dominant language
- JavaScript
- Stars
- 8.7k
- Forks
- 374
- Avg merge
- 11d 16h
- Merged PRs (30d)
- 1
Description
Version
postgres@3.4.9, running on Cloudflare Workers (cf/polyfills.js) behind Hyperdrive.
Summary
Closing a connection intermittently produces an unhandled promise rejection. The rejection escapes from inside read()'s own catch block, so there is no way for calling code to handle it. On Workers this surfaces as an unhandledRejection logged against whatever request happened to be in flight:
⨯ unhandledRejection: at async read (worker.js:...)
It is cosmetic — the request still succeeds — but it is unactionable log noise attributed to innocent requests, and it fires on essentially every connection teardown.
Cause
Three pieces in cf/polyfills.js combine:
-
read()is invoked fire-and-forget with no.catch()(line 163):tcp.ssl ? readFirst() : read() -
read()'s error path callserror()(lines 197–206), which emits'error'on the EventEmitter (lines 213–216):async function read() { try { let done, value while (({ done, value } = await tcp.reader.read(), !done)) tcp.emit('data', Buffer.from(value)) } catch (err) { error(err) // <-- runs inside the catch } } function error(err) { tcp.emit('error', err) // <-- throws if no 'error' listener tcp.emit('close') } -
src/connection.jsremoves every listener on close (line 447, insideclosed()):socket.removeAllListeners()
Closing the socket starts two things concurrently:
tcp.raw.closedresolves →close()→emit('close')→connection.js closed()→removeAllListeners()- the pending
tcp.reader.read()rejects →catch→error(err)→emit('error', err)
If the listener removal wins that race, emit('error') has no listeners. Node's EventEmitter contract is to throw in that case, and because the throw happens inside read()'s catch, nothing catches it — read()'s promise rejects with no handler attached.
This is timing-dependent, which matches the intermittent behaviour we see.
Reproduction
Any Workers deployment that opens a client per request and calls sql.end() when the request finishes will hit it within a handful of requests. It reproduces with both end() and end({ timeout: 1 }), i.e. graceful and forced teardown alike, since both end with tcp.raw.close() while a read is pending.
Suggested fix
Either would resolve it:
- Make
error()tolerate a listener-less socket, e.g.tcp.listenerCount('error') && tcp.emit('error', err)before emitting'close'. - Attach a rejection handler where
read()is started:tcp.ssl ? readFirst() : read().catch(() => {}).
Note readFirst() (lines 208–211) has no try/catch at all, so it can reject directly for the SSL path — worth covering in the same change.
Happy to open a PR if a preferred direction is indicated.
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/polyfills.js at the fire-and-forget read() and readFirst() calls, then trace the error() path alongside src/connection.js closed(), which removes all listeners. Reproduce teardown with end() and end({ timeout: 1 }) in a Cloudflare Workers deployment; done means connection closing no longer produces an unhandled rejection on the read path.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript, postgresql
- Domain
- backend, databases
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100