fix: end stream on Connection.end()

[Vratislav]

We are having issues with both pg 6.4.2 and pg 7.4.1 when disconnecting from Azure Postgres managed database. The socket never gets closed. I am able to replicate it with this simple script:
(excuse my TypeScript)

import * as pg from 'pg';
const opts = {
  host: process.env.DB_HOST,
  password: process.env.DB_PASSWORD,
  port: Number(process.env.DB_PORT),
  user: process.env.DB_USER,
  database: process.env.DB_DB,
  ssl: Boolean(process.env.DB_SSL),
};

console.log(opts);

async function testDb() {
  const client = new pg.Client(opts);

  await client.connect();
  console.log('Connection opened');
  const result = await client.query('SELECT 1+1');
  console.log(result);
  await client.end(err => {
    // This never gets called on Azure Postgres
    console.log('Connection closed.');
    console.log(err);
  });
}

testDb().catch(e => {
  console.log(`Error during execution: ${e}`);
});

The socket always stays hanging as can be verified by:

myMac:~ user$ lsof -P -i :5432
COMMAND    PID USER   FD   TYPE             DEVICE SIZE/OFF NODE NAME
node     52890  user   15u  IPv4 0xfddc5262efdff88b      0t0  TCP myMac:59923->191.237.232.75:5432 (ESTABLISHED)

When looking through official postgres documentation, the following passage about the connection termination can be found:

The normal, graceful termination procedure is that the frontend sends a Terminate message and immediately closes the connection. On receipt of this message, the backend closes the connection and terminates.

(taken from https://www.postgresql.org/docs/9.6/static/protocol-flow.html#AEN113373)

It seems that Azure postgres is really waiting for the client to close the connection first, otherwise it won't close the socket.

This PR adds a this.stream.end() after the termination message has been sent to the SQL server. This resolves our issue with Azure.

If needed I can provide instance of Microsoft Azure Postgres for replication and testing.

Also, since we are running on v6.4.2 in combination with sequelize, I can also offer this fix for the 6.x.x branch:

v6.4.2...Vratislav:end-stream

[Elexy]

Great work @Vratislav LGTM

[IvanJov]

Looks good @Vratislav 👍

[rahulbreddy]

:LGTM:

We have the same problem too - #1627.
Can someone please accept this PR?

[brianc]

ah nice! sorry for the delay on this. Will merge & push a new version today.

[jacqt]

Not sure if this is too much to ask, but would it be possible to also provide a version bump for 6.x.x? Unfortunately, we also use sequelize which does not support > [email protected], so we will be using a fork with the fix in the meantime. Many thanks!

[btai24]

I am getting intermittent SequelizeDatabaseError: read ETIMEDOUT with Azure postgres databases while not getting that error when connected to an RDS database. I'm not sure if this ticket correlates with the issue we are having. We are on [email protected] and I'm curious whats the latest v6 version I can upgrade to see if this fix improves things? I don't see the fix in the version tagged 6.4.2.

Full stack trace:

SequelizeDatabaseError: read ETIMEDOUT
at Query.formatError (/var/www/soxhub/api/node_modules/sequelize/lib/dialects/postgres/query.js:357:14)
at Query.<anonymous> (/var/www/soxhub/api/node_modules/sequelize/lib/dialects/postgres/query.js:88:19)
at Query.emit (events.js:182:13)
at Query.EventEmitter.emit (domain.js:442:20)
at Query.handleError (/var/www/soxhub/api/node_modules/pg/lib/query.js:143:8)
at Connection.<anonymous> (/var/www/soxhub/api/node_modules/pg/lib/client.js:180:26)
at Connection.emit (events.js:182:13)
at Connection.EventEmitter.emit (domain.js:442:20)
at TLSSocket.<anonymous> (/var/www/soxhub/api/node_modules/pg/lib/connection.js:121:12)
at TLSSocket.emit (events.js:182:13)
at TLSSocket.EventEmitter.emit (domain.js:442:20)
at emitErrorNT (internal/streams/destroy.js:82:8)
at emitErrorAndCloseNT (internal/streams/destroy.js:50:3)
at process._tickCallback (internal/process/next_tick.js:63:19)