Thanks @matthieusieben! Is there any way you could write a test for this?

Hey @brianc I added a test that indeed fails on my setup (without the fix of course). Not sure this test is a 100% useful as I don't know if it will fail in every setup, and I don't know how to trigger the same error in other setups...

Hey @brianc I added a test that indeed fails on my setup (without the fix of course). Not sure this test is a 100% useful as I don't know if it will fail in every setup, and I don't know how to trigger the same error in other setups...

ah sweet I'll try it over here & see if I can repro. Either way this LGTM so I'll get it merged & released this week. Sorry for the delay. ❤️

Hey @matthieusieben - I was gonna merge this today. CI was broken. I fixed it...would you be able to rebase your branch on to current HEAD of my master branch? I did so locally & ran the tests & a pg-query-stream error recovery test broke.

Hey @brianc , my wife just gave birth which is why I didn't answer earlier.

I rebased & fixed the issue. The problem was due to the ordering of the operation in the _closePortal method that was preventing this.connection.sync() from being executed.

Should be good now.

CI still seem broken but I don't think its related to these changes:

connection-timeout-tests.js
  successful connection ✔
  expired connection timeout Message: listen EADDRINUSE: address already in use 127.0.0.1:54322
Error: listen EADDRINUSE: address already in use 127.0.0.1:54322
    at Server.setupListenHandle [as _listen2] (net.js:1280:14)
    at listenInCluster (net.js:1328:12)
    at GetAddrInfoReqWrap.doListen [as callback] (net.js:1461:7)
    at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:61:10)

RE-running CI worked:
https://github.com/matthieusieben/node-postgres/actions/runs/1507064878

There might be a race condition making the test suite fail ;-)

@brianc any change this gets merged any time soon ?

ah right on, lemme let the tests run & I'll get this integrated!