Skip to content

Cloudflare polyfill: unhandled rejection when a connection closes (read() emits 'error' after listeners are removed) #1196

Description

@EunusHosen

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:

  1. read() is invoked fire-and-forget with no .catch() (line 163):

    tcp.ssl ? readFirst() : read()
  2. read()'s error path calls error() (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')
    }
  3. src/connection.js removes every listener on close (line 447, inside closed()):

    socket.removeAllListeners()

Closing the socket starts two things concurrently:

  • tcp.raw.closed resolves → close()emit('close')connection.js closed()removeAllListeners()
  • the pending tcp.reader.read() rejects → catcherror(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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions