Repository navigation
Conversation
Ending a connection left work behind that could still run afterwards:
- reconnect() discarded its timer, so after a dropped connection
scheduled a reconnect, end() could resolve and the timer still opened
a new socket. A socket still opening was left alive until
connect_timeout.
- connect() did not re-check terminated after awaiting an async custom
socket factory, so a socket returned after end({ timeout }) still
started a backend.
Keep the reconnect timer and clear it in terminate(), destroy a socket
that is still opening, and drop a socket the factory returns after
termination. Two tests cover the reconnect and the late socket factory.
|
This might also fix IlyaSemenov/pg-event-bus#2. We reproduced this with postgres 3.4.9 and Bun 1.3.14:
Your reconnect test closes a connection after it has disconnected. Our case closes it during the first connection attempt. Could you add a test for that too? I haven't tested this PR against our example yet, so I can't confirm it fixes our issue. |
Also make the socket factory test use node:net, since Deno's net polyfill ignores the connect callback and that test timed out there.
|
The PR already covers this case: on master, the opening socket stays alive until connect_timeout, but with the PR it's closed and the process exits. A regression test for ending during the first connection attempt was added in commit 1278f98; it fails on master and passes on the PR under ESM, CJS, and Deno, with the full test suites passing on all three. The same commit also fixes the PR's earlier socket-factory test under Deno, which now uses node:net since Deno's net polyfill ignores the connect callback. Bun was not tested. |
|
Thanks! This fully fixes my scenario (and I tested that it works on Bun, too). |
After a dropped connection schedules a reconnect,
sql.end()can resolve before the timer fires, and the timer then opens a new connection. A socket that is still opening when the connection ends is also left alive untilconnect_timeout.This keeps the reconnect timer and clears it in
terminate(). An open socket is ended as before; a socket that is still opening is destroyed.It also handles an async
socketfactory that is still pending when the connection is destroyed (end({ timeout })).connect()did not re-checkterminatedafter awaiting the factory, so the late socket completed startup afterend()had resolved and left an idle backend. Now the late socket's listeners are removed, it is destroyed andconnect()returns.The first new test terminates a LISTEN backend, calls
end()during the backoff, and checks that no backend reappears. A second test ends the client while the factory is pending and checks that the query is rejected withCONNECTION_DESTROYED, no backend appears, the socket is destroyed andoncloseis not called. Both fail on master and pass with the change. The full ESM and CJS suites pass. The opening-socket case has no committed test: reproducing it needs a socket held in DNS lookup, which is too contrived for the suite. Generatedcjs/anddeno/files are left for the release build.