fix: Client#end callback being called multiple times when first is no-op#3676
Conversation
…no-op (and unwanted retained listener even when not called multiple times)
…pre-connect behaviour closer to pg As usual, the native client is extra full of bugs and inconsistencies, so this is just “good enough”.
There was a problem hiding this comment.
Pull request overview
Fixes a bug where Client#end's callback could be invoked multiple times when the first call was a no-op (e.g., calling end() before ever connecting). In packages/pg/lib/client.js, after invoking the user callback in the no-op branch the function now returns immediately, preventing fall-through into the regular end logic that would attach the same callback to the connection's end event. The native client (packages/pg/lib/native/client.js) is adjusted so that the deferred end scheduled on connect no longer re-invokes the original user callback (it uses a no-op), and it now only defers when actively connecting. A regression test is added.
Changes:
- Add early
returnafter invoking the no-opcb()inClient#end(packages/pg/lib/client.js). - Restrict deferred end-on-connect to the
_connecting && !_connectedcase and decouple the deferred call from the user callback in the native client. - Add an integration test verifying that the no-op
Client#endcallback is fired exactly once.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/pg/lib/client.js | Adds return after no-op callback invocation to prevent double-callback fall-through. |
| packages/pg/lib/native/client.js | Tightens deferred-end condition to _connecting && !_connected and decouples scheduled re-end from the original callback. |
| packages/pg/test/integration/client/api-tests.js | New integration test asserting the no-op end callback is invoked exactly once. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
No description provided.