From c04cc17d8a90a160cd0c5b5ef595261fd53c893a Mon Sep 17 00:00:00 2001 From: armaan4658 Date: Sat, 15 Feb 2025 13:27:35 +0530 Subject: [PATCH 1/2] Fix connection timeout test to properly handle errors and timeouts --- packages/pg/lib/client.js | 3 +- packages/pg/lib/utils.js | 10 +++++++ .../client/connection-timeout-tests.js | 30 ++++++++++--------- 3 files changed, 28 insertions(+), 15 deletions(-) diff --git a/packages/pg/lib/client.js b/packages/pg/lib/client.js index 527f62e4f..31c70bc88 100644 --- a/packages/pg/lib/client.js +++ b/packages/pg/lib/client.js @@ -10,6 +10,7 @@ var Query = require('./query') var defaults = require('./defaults') var Connection = require('./connection') const crypto = require('./crypto/utils') +const { ConnectionTimeoutError } = "./utils" class Client extends EventEmitter { constructor(config) { @@ -102,7 +103,7 @@ class Client extends EventEmitter { if (this._connectionTimeoutMillis > 0) { this.connectionTimeoutHandle = setTimeout(() => { con._ending = true - con.stream.destroy(new Error('timeout expired')) + con.stream.destroy(new ConnectionTimeoutError('timeout expired')) }, this._connectionTimeoutMillis) } diff --git a/packages/pg/lib/utils.js b/packages/pg/lib/utils.js index 3bcc4e525..e01a21d0a 100644 --- a/packages/pg/lib/utils.js +++ b/packages/pg/lib/utils.js @@ -194,6 +194,15 @@ const escapeLiteral = function (str) { return escaped } +class ConnectionTimeoutError extends Error { + constructor(message) { + super(message); + this.name = 'ConnectionTimeoutError'; + this.code = 'CONNECTION_TIMEOUT'; + } +} + + module.exports = { prepareValue: function prepareValueWrapper(value) { // this ensures that extra arguments do not get passed into prepareValue @@ -203,4 +212,5 @@ module.exports = { normalizeQueryConfig, escapeIdentifier, escapeLiteral, + ConnectionTimeoutError } diff --git a/packages/pg/test/integration/client/connection-timeout-tests.js b/packages/pg/test/integration/client/connection-timeout-tests.js index 3d6b83664..01f4c468f 100644 --- a/packages/pg/test/integration/client/connection-timeout-tests.js +++ b/packages/pg/test/integration/client/connection-timeout-tests.js @@ -3,7 +3,7 @@ const net = require('net') const buffers = require('../../test-buffers') const helper = require('./test-helper') const assert = require('assert') - +const { ConnectionTimeoutError } = require('../../../lib/utils') const suite = new helper.Suite() const options = { @@ -68,22 +68,24 @@ suite.test('successful connection', (done) => { }) suite.test('expired connection timeout', (done) => { - const opts = { ...options, port: options.port + 1 } + const opts = { ...options, port: options.port + 1 }; serverWithConnectionTimeout(opts.port, opts.connectionTimeoutMillis * 2, (closeServer) => { const timeoutId = setTimeout(() => { - throw new Error('Client should have emitted an error but it did not.') - }, 3000) + done(new Error('Client should have emitted an error but it did not.')); + }, 3000); - const client = new helper.Client(opts) + const client = new helper.Client(opts); client .connect() - .then(() => client.end()) - .then(() => closeServer(() => done(new Error('Connection timeout should have expired but it did not.')))) - .catch((err) => { - assert(err instanceof Error) - assert(/timeout expired\s*/.test(err.message)) - closeServer(done) + .then(() => { + clearTimeout(timeoutId); + return client.end().then(() => closeServer(() => done(new Error('Connection timeout should have expired but it did not.')))); }) - .then(() => clearTimeout(timeoutId)) - }) -}) + .catch((err) => { + clearTimeout(timeoutId); + assert(err instanceof ConnectionTimeoutError); + assert.strictEqual(err.code, 'CONNECTION_TIMEOUT'); + closeServer(done); + }); + }); +}); From 82826e2c37c29bb3e8695b1cc9a5468f3194e834 Mon Sep 17 00:00:00 2001 From: armaan4658 Date: Sat, 15 Feb 2025 13:43:41 +0530 Subject: [PATCH 2/2] Formatted code with eslint --- packages/pg/lib/client.js | 2 +- packages/pg/lib/utils.js | 9 +++--- .../client/connection-timeout-tests.js | 28 ++++++++++--------- 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/packages/pg/lib/client.js b/packages/pg/lib/client.js index 31c70bc88..ae4fb2757 100644 --- a/packages/pg/lib/client.js +++ b/packages/pg/lib/client.js @@ -10,7 +10,7 @@ var Query = require('./query') var defaults = require('./defaults') var Connection = require('./connection') const crypto = require('./crypto/utils') -const { ConnectionTimeoutError } = "./utils" +const { ConnectionTimeoutError } = './utils' class Client extends EventEmitter { constructor(config) { diff --git a/packages/pg/lib/utils.js b/packages/pg/lib/utils.js index e01a21d0a..bac1b513c 100644 --- a/packages/pg/lib/utils.js +++ b/packages/pg/lib/utils.js @@ -196,13 +196,12 @@ const escapeLiteral = function (str) { class ConnectionTimeoutError extends Error { constructor(message) { - super(message); - this.name = 'ConnectionTimeoutError'; - this.code = 'CONNECTION_TIMEOUT'; + super(message) + this.name = 'ConnectionTimeoutError' + this.code = 'CONNECTION_TIMEOUT' } } - module.exports = { prepareValue: function prepareValueWrapper(value) { // this ensures that extra arguments do not get passed into prepareValue @@ -212,5 +211,5 @@ module.exports = { normalizeQueryConfig, escapeIdentifier, escapeLiteral, - ConnectionTimeoutError + ConnectionTimeoutError, } diff --git a/packages/pg/test/integration/client/connection-timeout-tests.js b/packages/pg/test/integration/client/connection-timeout-tests.js index 01f4c468f..35737c4a8 100644 --- a/packages/pg/test/integration/client/connection-timeout-tests.js +++ b/packages/pg/test/integration/client/connection-timeout-tests.js @@ -68,24 +68,26 @@ suite.test('successful connection', (done) => { }) suite.test('expired connection timeout', (done) => { - const opts = { ...options, port: options.port + 1 }; + const opts = { ...options, port: options.port + 1 } serverWithConnectionTimeout(opts.port, opts.connectionTimeoutMillis * 2, (closeServer) => { const timeoutId = setTimeout(() => { - done(new Error('Client should have emitted an error but it did not.')); - }, 3000); + done(new Error('Client should have emitted an error but it did not.')) + }, 3000) - const client = new helper.Client(opts); + const client = new helper.Client(opts) client .connect() .then(() => { - clearTimeout(timeoutId); - return client.end().then(() => closeServer(() => done(new Error('Connection timeout should have expired but it did not.')))); + clearTimeout(timeoutId) + return client + .end() + .then(() => closeServer(() => done(new Error('Connection timeout should have expired but it did not.')))) }) .catch((err) => { - clearTimeout(timeoutId); - assert(err instanceof ConnectionTimeoutError); - assert.strictEqual(err.code, 'CONNECTION_TIMEOUT'); - closeServer(done); - }); - }); -}); + clearTimeout(timeoutId) + assert(err instanceof ConnectionTimeoutError) + assert.strictEqual(err.code, 'CONNECTION_TIMEOUT') + closeServer(done) + }) + }) +})