fix(pg-cloudflare): emit write errors that have no callback

[askalf]

Staging PR for branch review before submission upstream. Closed once submitted; the branch is kept until upstream resolves.

[sprayberry-redline]

Automated review from the Sprayberry Labs fleet code reviewer.
Reviewed by the GPT gating lane (gating review).

Verdict: APPROVE — ready for the operator to submit; no blocking issues found.

I verified the stated base behavior against 9683053c1eac7f4a1f8910df0d7abdb5f0cb25a7: CloudflareSocket.write() supplies the no-op default callback, while Connection._send() invokes this.stream.write(buffer) without one. Thus a rejected writer promise is currently discarded. The changed branch now preserves supplied-callback behavior and emits the rejection only when no callback exists; Connection already subscribes to the stream's error event. I also reasoned through both new regression tests: the emission test fails on base and succeeds with the new rejection branch, while the callback test prevents double-reporting.

OSS-candidate checks: all required facts-sheet sections are present; the base repro and error propagation path are grounded in the base source; the body includes verbatim before/after test output and static-tool evidence; the fork reports no CI checks, which is disclosed rather than represented as green CI. I repeated upstream searches for pg-cloudflare write across open and closed PRs and issues and found no competing result. The upstream policy files queried are absent, the policy section accurately records that absence and the disclosed formatter/linter/typecheck evidence, and the single commit contains no prohibited AI attribution. The two touched files are narrowly scoped to one write-error reporting defect.

What's good: this is a minimal, transport-local correction that turns a previously silent async failure into the error path the client already uses, with a focused test for each side of the callback/no-callback contract.

I did not run the test suite locally; the PR body supplies the executed base-versus-fixed evidence, and this fork currently has no reported CI checks.

[sprayberry-secondread]

Automated review from the Sprayberry Labs fleet code reviewer.

Reviewed by the Claude second-opinion lane (second opinion, non-gating; the gating review is posted separately).

Verdict: No blocking issues found; the fix is correct in the path that matters and the PR is well-scoped for this upstream. Recommend ready-for-operator.

What I verified

The bug is real and the fix closes it. Connection._send() (packages/pg/lib/connection.js:175, return this.stream.write(buffer)) and the direct calls at connection.js:148/152 (this.stream.write(serialize.requestSsl()), this.stream.write(serialize.startup(config))) all call write() with no callback. Pre-fix, CloudflareSocket.write()'s callback: (error?: unknown) => void = () => {} default meant this._cfWriter!.write(data).then(..., (err) => callback(err)) silently swallowed the rejection — nothing observes it. Post-fix (packages/pg-cloudflare/src/index.ts:109-113):

if (callback) {
  callback(err)
} else {
  this.emit('error', err)
}

this now surfaces it.

I checked the specific risk this kind of fix usually carries — an emit('error') with no listener throws in Node and would crash worse than the original hang. Traced the actual call path: Connection.connect() (connection.js:53-61) registers this.stream.on('error', reportStreamError) synchronously right after this.stream.connect(port, host), before any write() calls can happen (writes only happen from 'connect'-triggered code or later). For the Cloudflare TLS-upgrade path, stream.getSecureStream() (packages/pg/lib/stream.js, getCloudflareStreamFuncs) calls options.socket.startTls(options) and returns the same CloudflareSocket object — startTls() reassigns _cfWriter/_cfReader internally but doesn't recreate the EventEmitter, so the listener attached at connect() survives the upgrade. I could not construct a path through pg's own Connection/Client where write() reaches the emit('error', err) branch with zero listeners attached. This does not reproduce as a bug against the code this PR actually feeds.

Scope and idiom match upstream's own recent history on this exact file. I pulled the three cited precedents (#3735, #3747, #3752) via the GitHub API: all three are single-purpose fixes to packages/pg-cloudflare/src/index.ts with a test added to packages/pg-esm-test/pg-cloudflare.test.js, no CHANGELOG entry (CHANGELOG.md states plainly: "We do not include break-fix version release in this file"), and titles either fix(pg-cloudflare): ... (2 of 3) or plain fix: ... (1 of 3). The suggested title fix(pg-cloudflare): emit write errors that have no callback matches the majority convention. #3747's overload style (write(data, callback) / write(data, encoding, callback) split) is reused verbatim rather than reinvented — this PR only widens callback to optional and branches on its presence, which is the minimal diff against that precedent. The new tests (packages/pg-esm-test/pg-cloudflare.test.js:41-68) follow the file's existing convention exactly: a bare object assigned to socket._cfWriter with a write stub, no Workers runtime needed, matching every other test in that file.

Repo policy claims check out. Confirmed CONTRIBUTING.md, AGENTS.md, CODE_OF_CONDUCT.md, and any PR template are all absent from brianc/node-postgres (root listing has no such files), so there's genuinely no AI-contribution policy to violate and no template to fill in — the PR body's "Policy" section is accurate, not a gap the maintainer would push back on.

One thing correctly left out of scope, worth flagging so it isn't lost. CloudflareSocket.writable is set true once in connect() (index.ts:55) and is never reset to false on write failure or close — so after this fix reports an error, Connection._send() (connection.js:172-175, gated on this.stream.writable) will still attempt further writes onto a socket that just failed. That's a real second bug, but it's a distinct fix with its own test shape, and bundling it here would break the single-purpose-PR pattern the three precedents establish. Leaving it for a follow-up is the right call for this submission, not a gap in this PR.

Tests

The two new tests are appropriately scoped: one confirms emit('error') fires when no callback is given, the other confirms the callback path still short-circuits emit('error') (guards against regressing #3747's callback support). No gaps I'd ask for here — a write-failure-during-end()/destroy() case would be nice-to-have but isn't part of this bug and isn't tested by the precedent PRs either.

Disclosure facts for the operator

Findings/trace/tests above are my own independent verification, not a restatement of the PR body's disclosure section — I re-derived the caller graph and the precedent-PR comparison from the diff and the GitHub API rather than trusting the PR's claims.