Skip to content

Fix promise-based connect() and orphan 'end' events during load-balan…#15

Open
HarshDaryani896 wants to merge 2 commits into
masterfrom
bug-fixes
Open

Fix promise-based connect() and orphan 'end' events during load-balan…#15
HarshDaryani896 wants to merge 2 commits into
masterfrom
bug-fixes

Conversation

@HarshDaryani896

@HarshDaryani896 HarshDaryani896 commented Jun 5, 2026

Copy link
Copy Markdown

…ced failover

Summary

Three fixes to Client in packages/pg/lib/client.js that surface when load balancing is enabled and _connect() performs an internal retry/failover to a different host, or when no eligible server can be found.

Changes

1. connect() returns a Promise when no callback is provided

The load-balance branch wraps its work in lock.acquire().then(...) but never returned that chain, so calling connect() without a callback returned undefined synchronously. As a result await client.connect() did not actually wait for the connection (including any internal retry/failover) to complete.

We now wrap the callback-style path in a Promise at the top of connect(), so both the callback and promise forms behave consistently with upstream pg.

2. Ignore 'end' events from orphaned connections

During load-balanced failover, _connect() may replace this.connection with a new Connection on a different host. The old Connection's listeners remain attached, and when its socket eventually fires 'end', the regular handler would call _errorAllQueries(): incorrectly rejecting queries that belong to the new, healthy connection.

The 'end' handler now detects this case (this.connection !== con) and bails out early, logging at silly level.

3. Release the lock and surface the error when no server can be found

The load-balance branch returns its work as lock.acquire().then(...), but a synchronous throw from that path: most commonly getLeastLoadedServer() raising Error: Could not find a least loaded server. (e.g. every eligible host is stopped), or topology-key parsing: was never handled. The rejection leaked as an unhandled promise rejection while the user callback was never invoked, so await client.connect() hung forever, and the lock acquired at the top of the branch was never released: deadlocking every subsequent connect().

We now:

  • return the lock.acquire().then(...) chain (and the inner getConnection() / getServersInfo() chains) so the failure propagates instead of being dropped, and
  • add an outer .catch() that releases the lock and surfaces the error through the callback.

connect() now rejects promptly with the underlying error instead of hanging, and the lock is always released.

Testing

Ran test for node-postgres driver present in driver-examples repo.

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.

1 participant