From 4da0065c53340c4e54783d2287d4dc2d7362d2ef Mon Sep 17 00:00:00 2001 From: Joaquin Colacci Date: Wed, 13 Nov 2024 13:26:07 +0100 Subject: [PATCH 1/2] Extend test for issue 3174 - add cursor cases --- packages/pg/lib/client.js | 10 ++++++++++ .../pg/test/integration/gh-issues/3174-tests.js | 15 +++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/packages/pg/lib/client.js b/packages/pg/lib/client.js index 527f62e4f..0c80b9c25 100644 --- a/packages/pg/lib/client.js +++ b/packages/pg/lib/client.js @@ -357,11 +357,21 @@ class Client extends EventEmitter { } _handleRowDescription(msg) { + if (this.activeQuery == null) { + const error = new Error('Received unexpected rowDescription message from backend.') + this._handleErrorEvent(error) + return + } // delegate rowDescription to active query this.activeQuery.handleRowDescription(msg) } _handleDataRow(msg) { + if (this.activeQuery == null) { + const error = new Error('Received unexpected dataRow message from backend.') + this._handleErrorEvent(error) + return + } // delegate dataRow to active query this.activeQuery.handleDataRow(msg) } diff --git a/packages/pg/test/integration/gh-issues/3174-tests.js b/packages/pg/test/integration/gh-issues/3174-tests.js index 49ac5905a..c32a74fba 100644 --- a/packages/pg/test/integration/gh-issues/3174-tests.js +++ b/packages/pg/test/integration/gh-issues/3174-tests.js @@ -3,6 +3,7 @@ const buffers = require('../../test-buffers') const helper = require('../test-helper') const assert = require('assert') const cli = require('../../cli') +const Cursor = require('../../../../pg-cursor') const suite = new helper.Suite() @@ -106,6 +107,18 @@ const testErrorBuffer = (bufferName, errorBuffer) => { await assert.rejects(() => client.query('SELECTR NOW()'), 'Further queries on the client should reject') } + // Same run but using cursor + const cursor = await client.query(new Cursor('SELECT NOW()')) + cursor.read(100, () => {}) + await cursor.close() + await delay(50) + + if (!cli.native) { + assert(errorHit) + // further queries on the client should fail since its in an invalid state + await assert.rejects(() => client.query('SELECTR NOW()'), 'Further queries on the client should reject') + } + await closeServer() }) @@ -164,4 +177,6 @@ const testErrorBuffer = (bufferName, errorBuffer) => { if (!helper.args.native) { testErrorBuffer('parseComplete', buffers.parseComplete()) testErrorBuffer('commandComplete', buffers.commandComplete('f')) + testErrorBuffer('rowDescription', buffers.rowDescription()) + testErrorBuffer('dataRow', buffers.dataRow()) } From 563f97789642e4f523bd3cef06c0418b5b0e61c2 Mon Sep 17 00:00:00 2001 From: Joaquin Colacci Date: Thu, 14 Nov 2024 12:01:36 +0100 Subject: [PATCH 2/2] Extend test for issue 3174 - add suite test cases for cursor and null check for pg-cursor/handleDataRow. --- packages/pg-cursor/index.js | 6 ++ .../test/integration/gh-issues/3174-tests.js | 88 ++++++++++++++++--- 2 files changed, 84 insertions(+), 10 deletions(-) diff --git a/packages/pg-cursor/index.js b/packages/pg-cursor/index.js index a6017d96c..b8777dddc 100644 --- a/packages/pg-cursor/index.js +++ b/packages/pg-cursor/index.js @@ -112,6 +112,12 @@ class Cursor extends EventEmitter { handleDataRow(msg) { const row = this._result.parseRow(msg.fields) this.emit('row', row, this._result) + + if (this._rows == null) { + const error = new Error('Received unexpected dataRow message from backend.') + this.handleError(error) + return + } this._rows.push(row) } diff --git a/packages/pg/test/integration/gh-issues/3174-tests.js b/packages/pg/test/integration/gh-issues/3174-tests.js index c32a74fba..d2270b147 100644 --- a/packages/pg/test/integration/gh-issues/3174-tests.js +++ b/packages/pg/test/integration/gh-issues/3174-tests.js @@ -104,25 +104,89 @@ const testErrorBuffer = (bufferName, errorBuffer) => { if (!cli.native) { assert(errorHit) // further queries on the client should fail since its in an invalid state - await assert.rejects(() => client.query('SELECTR NOW()'), 'Further queries on the client should reject') + await assert.rejects(() => client.query('SELECT NOW()'), 'Further queries on the client should reject') } + await closeServer() + }) + + suite.testAsync(`Out of order ${bufferName} on extended query is catchable`, async () => { + const closeServer = await new Promise((resolve, reject) => { + return startMockServer(options.port, errorBuffer, (closeServer) => resolve(closeServer)) + }) + const client = new helper.Client(options) + await client.connect() + + let errorHit = false + client.on('error', () => { + errorHit = true + }) + + await client.query('SELECT $1', ['foo']) + await delay(40) + + // the native client only emits a notice message and keeps on its merry way + if (!cli.native) { + assert(errorHit) + // further queries on the client should fail since its in an invalid state + await assert.rejects(() => client.query('SELECT NOW()'), 'Further queries on the client should reject') + } + + await client.end() + + await closeServer() + }) + + suite.testAsync(`Out of order ${bufferName} on pool is catchable`, async () => { + const closeServer = await new Promise((resolve, reject) => { + return startMockServer(options.port, errorBuffer, (closeServer) => resolve(closeServer)) + }) + const pool = new helper.pg.Pool(options) + + let errorHit = false + pool.on('error', () => { + errorHit = true + }) + + await pool.query('SELECT $1', ['foo']) + await delay(100) + + if (!cli.native) { + assert(errorHit) + assert.strictEqual(pool.idleCount, 0, 'Pool should have no idle clients') + assert.strictEqual(pool.totalCount, 0, 'Pool should have no connected clients') + } + + await pool.end() + await closeServer() + }) + + suite.testAsync(`Out of order ${bufferName} on simple query using cursors is catchable`, async () => { + const closeServer = await new Promise((resolve, reject) => { + return startMockServer(options.port, errorBuffer, (closeServer) => resolve(closeServer)) + }) + const client = new helper.Client(options) + await client.connect() + + let errorHit = false + client.on('error', () => { + errorHit = true + }) - // Same run but using cursor const cursor = await client.query(new Cursor('SELECT NOW()')) cursor.read(100, () => {}) await cursor.close() await delay(50) + // the native client only emits a notice message and keeps on its merry way if (!cli.native) { assert(errorHit) // further queries on the client should fail since its in an invalid state - await assert.rejects(() => client.query('SELECTR NOW()'), 'Further queries on the client should reject') + await assert.rejects(() => client.query('SELECT NOW()'), 'Further queries on the client should reject') } - await closeServer() }) - suite.testAsync(`Out of order ${bufferName} on extended query is catchable`, async () => { + suite.testAsync(`Out of order ${bufferName} on extended query using cursors is catchable`, async () => { const closeServer = await new Promise((resolve, reject) => { return startMockServer(options.port, errorBuffer, (closeServer) => resolve(closeServer)) }) @@ -134,14 +198,16 @@ const testErrorBuffer = (bufferName, errorBuffer) => { errorHit = true }) - await client.query('SELECT $1', ['foo']) - await delay(40) + const cursor = await client.query(new Cursor('SELECT $1', ['foo'])) + cursor.read(100, () => {}) + await cursor.close() + await delay(50) // the native client only emits a notice message and keeps on its merry way if (!cli.native) { assert(errorHit) // further queries on the client should fail since its in an invalid state - await assert.rejects(() => client.query('SELECTR NOW()'), 'Further queries on the client should reject') + await assert.rejects(() => client.query('SELECT NOW()'), 'Further queries on the client should reject') } await client.end() @@ -149,7 +215,7 @@ const testErrorBuffer = (bufferName, errorBuffer) => { await closeServer() }) - suite.testAsync(`Out of order ${bufferName} on pool is catchable`, async () => { + suite.testAsync(`Out of order ${bufferName} on pool using cursors is catchable`, async () => { const closeServer = await new Promise((resolve, reject) => { return startMockServer(options.port, errorBuffer, (closeServer) => resolve(closeServer)) }) @@ -160,7 +226,9 @@ const testErrorBuffer = (bufferName, errorBuffer) => { errorHit = true }) - await pool.query('SELECT $1', ['foo']) + const cursor = await pool.query(new Cursor('SELECT $1', ['foo'])) + cursor.read(100, () => {}) + await cursor.close() await delay(100) if (!cli.native) {