From ff43e8d32d8069f4071df2ca57931c74f6556a2e Mon Sep 17 00:00:00 2001
From: Harry Kao <harry@hairycow.name>
Date: Mon, 24 Oct 2022 05:23:24 -0700
Subject: [PATCH] fix(postgres): invalidate connection after client-side
 timeout (#15144)

* Invalidate connection after client-side timeout.

The `query_timeout` feature of the `pg` package helps handle stuck TCP
connections more quickly and gracefully by implementing a client-side
timeout:

https://github.com/brianc/node-postgres/issues/1713

Sequelize started passing this dialect-specific option through to `pg` here:

https://github.com/sequelize/sequelize/pull/13258

I believe we also want to invalidate the connection when a client-side
timeout occurs. We shouldn't try to reuse the stuck connection
because...it's stuck.

This PR updates the error handling code so that the connection is
invalidated if the error matches the one thrown from here:

https://github.com/brianc/node-postgres/blob/5538df6b446f4b4f921947b460fe38acb897e579/packages/pg/lib/client.js#L529

* Add comment with link to PR.

* Add tests, shorten comment.

* Skip tests for postgres-native.

* Update src/dialects/postgres/query.js

Co-authored-by: Rik Smale <13023439+WikiRik@users.noreply.github.com>

Co-authored-by: Rik Smale <13023439+WikiRik@users.noreply.github.com>
---
 src/dialects/postgres/query.js                |  2 +
 .../dialects/postgres/query.test.js           | 46 ++++++++++++++++++-
 2 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/src/dialects/postgres/query.js b/src/dialects/postgres/query.js
index 4f4f444cb081..e83af9e6bd18 100644
--- a/src/dialects/postgres/query.js
+++ b/src/dialects/postgres/query.js
@@ -51,6 +51,8 @@ export class PostgresQuery extends AbstractQuery {
         || /Unable to set non-blocking to true/i.test(error)
         || /SSL SYSCALL error: EOF detected/i.test(error)
         || /Local: Authentication failure/i.test(error)
+        // https://github.com/sequelize/sequelize/pull/15144
+        || error.message === 'Query read timeout'
       ) {
         connection._invalid = true;
       }
diff --git a/test/integration/dialects/postgres/query.test.js b/test/integration/dialects/postgres/query.test.js
index 012c40cf2490..a18911d21c39 100644
--- a/test/integration/dialects/postgres/query.test.js
+++ b/test/integration/dialects/postgres/query.test.js
@@ -6,7 +6,7 @@ const expect = chai.expect;
 const Support = require('../../support');
 
 const dialect = Support.getTestDialect();
-const { DataTypes } = require('@sequelize/core');
+const { DatabaseError, DataTypes } = require('@sequelize/core');
 
 if (dialect.startsWith('postgres')) {
   describe('[POSTGRES] Query', () => {
@@ -221,5 +221,49 @@ if (dialect.startsWith('postgres')) {
         order: [['order_0', 'DESC']],
       });
     });
+
+    describe('Connection Invalidation', () => {
+      if (process.env.DIALECT === 'postgres-native') {
+        // native driver doesn't support statement_timeout or query_timeout
+        return;
+      }
+
+      async function setUp(clientQueryTimeoutMs) {
+        const sequelize = Support.createSequelizeInstance({
+          dialectOptions: {
+            statement_timeout: 500, // ms
+            query_timeout: clientQueryTimeoutMs,
+          },
+          pool: {
+            max: 1, // having only one helps us know whether the connection was invalidated
+            idle: 60_000,
+          },
+        });
+
+        return { sequelize, originalPid: await getConnectionPid(sequelize) };
+      }
+
+      async function getConnectionPid(sequelize) {
+        const connection = await sequelize.connectionManager.getConnection();
+        const pid = connection.processID;
+        sequelize.connectionManager.releaseConnection(connection);
+
+        return pid;
+      }
+
+      it('reuses connection after statement timeout', async () => {
+        // client timeout > statement timeout means that the query should fail with a statement timeout
+        const { sequelize, originalPid } = await setUp(10_000);
+        await expect(sequelize.query('select pg_sleep(1)')).to.eventually.be.rejectedWith(DatabaseError, 'canceling statement due to statement timeout');
+        expect(await getConnectionPid(sequelize)).to.equal(originalPid);
+      });
+
+      it('invalidates connection after client-side query timeout', async () => {
+        // client timeout < statement timeout means that the query should fail with a read timeout
+        const { sequelize, originalPid } = await setUp(250);
+        await expect(sequelize.query('select pg_sleep(1)')).to.eventually.be.rejectedWith(DatabaseError, 'Query read timeout');
+        expect(await getConnectionPid(sequelize)).to.not.equal(originalPid);
+      });
+    });
   });
 }