Skip to content

fix: support callback as second argument to CloudflareSocket.write - #3747

Merged
charmander merged 3 commits into
brianc:masterfrom
zfaustk:fix/cloudflare-socket-close-event
Aug 14, 2026
Merged

fix: support callback as second argument to CloudflareSocket.write#3747
charmander merged 3 commits into
brianc:masterfrom
zfaustk:fix/cloudflare-socket-close-event

Conversation

@zfaustk

@zfaustk zfaustk commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Connection.end() calls write(data, callback). CloudflareSocket.write previously treated the callback as an encoding, so it never ran.

This adds explicit overloads for write(data, callback) and write(data, encoding, callback). The runtime dispatch stays in the existing write method; close and event behavior are unchanged.

Validation:

  • focused pg-cloudflare runtime test: 3 passed;
  • yarn build, yarn lint, Prettier, and git diff --check: passed;
  • type fixture: valid overloads compile, while write(data, callback, callback) fails with TS2345.

The full PostgreSQL/libpq integration suite and a real Cloudflare Worker E2E were not run locally.

Prepared with Codex assistance.

Emit the close event when the explicit close operation resolves and support the write(data, callback) overload used by Connection.end().

Fixes brianc#3152

@charmander charmander left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn’t look right. Maybe start with a PR for just the write overload and a corresponding test that exercises it, ideally typed correctly with an overload instead of accepting (data, callback, callback).

@zfaustk zfaustk changed the title fix: close Cloudflare sockets after failed connections fix: support Cloudflare write callbacks Aug 13, 2026
@zfaustk

zfaustk commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Thanks — I narrowed this to the write overload only.

The updated method now has explicit overloads for write(data, callback) and
write(data, encoding, callback). The focused test checks that the two-argument
callback runs exactly once, and a TypeScript fixture confirms that
write(data, callback, callback) is rejected. I removed the close-event and
closed-promise changes from this PR.

Comment thread packages/pg-esm-test/pg-cloudflare.test.js Outdated
## Context
- Principle: Callback tests must observe callback completion directly across the supported Node matrix
- Why: The write overload regression should fail on a missing or duplicate callback without relying on event-loop timing

## Key Deltas
- pg-cloudflare callback test: waited one event-loop tick and counted callbacks afterward -> awaits the callback and rejects a duplicate invocation inside it; why: the assertion now follows the callback contract while remaining compatible with Node 16. Key refs: packages/pg-esm-test/pg-cloudflare.test.js:21

## Verification
- Result: passed
@charmander charmander changed the title fix: support Cloudflare write callbacks fix: support callback as second argument to CloudflareSocket.write Aug 14, 2026
@charmander
charmander merged commit c9e5761 into brianc:master Aug 14, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants