Calling `.end` on a client (or pool) does not wait for connections to close

[JamesMcNee]

Hi there, it seems that there is a bug around client.end / pool.end in that when awaited (or using callbacks) the promise can resolve before the underlying connection / stream has disconnected.

Digging through the code a bit, it seems like the issue is in this bit of code: https://github.com/brianc/node-postgres/blob/54eb0fa216aaccd727765641e7d1cf5da2bc483d/packages/pg/lib/connection.js#L189-L199

I think here we should hook into the callback from the sockets end event: https://nodejs.org/api/net.html#socketenddata-encoding-callback and ensure that the original promise does not resolve (or callback fire) before observing this.

[opyate]

Yes, please add a callback to client.release() so I can pass resolve and wrap it in a Promise (or better yet, make release return a Promise).

The issue manifested itself like this for me: I use this package to verify DB writes in E2E tests, and Jest had this error:

Jest did not exit one second after the test run has completed.

Running Jest with --detectOpenHandles then showed:

Jest has detected the following 1 open handle potentially keeping Jest from exiting:

  ●  TCPWRAP

       7 |
       8 | async function withTransaction(it: Function) {
    >  9 |   const client = await pool.connect()
         |                             ^
      10 |   try {
      11 |     await client.query('BEGIN')
      12 |     await it(client)

      at Connection.connect (node_modules/pg/lib/connection.js:44:17)
      at Client._connect (node_modules/pg/lib/client.js:113:11)
      at Client.connect (node_modules/pg/lib/client.js:162:12)
      at BoundPool.newClient (node_modules/pg-pool/index.js:237:12)
      at BoundPool.connect (node_modules/pg-pool/index.js:212:10)
      at withTransaction (src/helpers/db.ts:9:29)

My code for reference

async function withTransaction(it: Function) {
  const client = await pool.connect()
  try {
    await client.query('BEGIN')
    await it(client)
    await client.query('COMMIT')
  } catch (error) {
    console.error('Error in transaction:', error)
    await client.query('ROLLBACK')
    throw error
  } finally {
    client.release()
  }
}

[JamesMcNee]

@opyate the Pool suffers from the same issue I raised, however your issue may be more around you not calling .end on the pool. When you .release a client, it simply returns it to the pool so that it might be reused, therefore, depending on your pool configuration, it is valid for the connection to remain open.

[opyate]

Ah, of course, I missed .end in the doc, thanks :+1:

Here's my solution for anyone else landing here.

File setupFilesAfterEnv.ts:

import { Pool } from 'pg'

beforeAll(() => {
  (global as any).pool = new Pool();  
});

afterAll(async () => {
  await (global as any).pool.end();
});

And my helper becomes:

const client = await (global as any).pool.connect()

[ch0c0l8ra1n]

@JamesMcNee

the problem is with pool.end()

client.end works as you'd expect, at least when you don't provide a callback.

pool.end() returns immediately, before the connections are truly closed. I am assuming the reason most people wound up on this thread is because of their jest tests throwing an "open handle" error.

To wait till all the connections in a pool are truly closed you can wait for all clients in the pool to end(). Whenever I have to wait till all the connections are truly closed, I use this helper function in my database class:

async closeAndWait(){
    // @ts-ignore
    const clients = this.pool._clients as Client[];
    
    await Promise.all(clients.map(client => client.end()));
}

or if you don't want to end the clients yourself:

async closeAndWait(){
    // @ts-ignore
    const clients = this.pool._clients as Client[];


    const endPromises = Promise.all(
        clients.map(client => new Promise<void>(resolve => {
            client.once("end", resolve);
            })
        )
    );

    this.pool.end();
    await endPromises;
}

And then on test files:

afterAll(async () => {
    await db.closeAndWait();
});

I am not sure why pool.end is working for @opyate but I am assuming it's something to do with how inidividual clients work in pg-pool vs running pool.query.

[charmander]

The reason pool.end() might appear to be working/work for some purposes, note, is that it does wait for all clients to be idle before resolving.

[ch0c0l8ra1n]

@charmander I did a little more digging. It appears to be working for opayate because he's presumably calling client.release() which then ends the connection instantly, so when he calls pool.end(), there are no connections to close.

I think the Pool class should have a endAndWait function like the one I mentioned earlier.

Should I submit a pull request?

[charmander]

I’d think end itself should work like that (next major version?), but haven’t searched for and reviewed all the conversations about it.

[AanshKot]

I'm currently running into this issue as well and pool.end isn't working for me, Jest is detecting open handles after test execution. My flow is something like follows:

1. Global setup: I initialize a new instance of the connection pool.
2. Before each test: I request a new client using await pool.connect() and start a transaction on that client.
3. Tests execute successfully and finish.
4. After each test: I roll back the transaction and release the client back to the pool using await client.release().
5. Global teardown: I call await poolInstance.end() to drain and close the connection pool.

Despite this, Jest still reports open handles like the ones below. From what I can tell, all clients are being properly released and the pool is being explicitly shut down, so it's unclear what's still holding the event loop open.

TCPWRAP 
client = await pool.connect()

and

TIMEOUT
client = await pool.connect()

A hacky fix I found in this stack overflow post was to add a timeout before requesting a connection and that seemed appease Jest.

    beforeEach(async () => {
        await new Promise((resolve) => setTimeout(resolve, 100)); //timeout added
        client = await pool.connect();
        // ... start transaction and setup a temp table
    });

    afterEach(async () => {
        //rollback transaction 
        client = await pool.release();
    });