Fix double readyForQuery

brianc · brianc · commit bcbb956a26fc · 2020-11-25T11:43:14.000-06:00
diff --git a/packages/pg-cursor/index.js b/packages/pg-cursor/index.js
@@ -155,8 +155,12 @@ Cursor.prototype.handleError = function (msg) {
     // only dispatch error events if we have a listener
     this.emit('error', msg)
   }
-  // call sync to keep this connection from hanging
-  this.connection.sync()
+  // If we have already received a ready for query event do not double-sync
+  // because this can cause a readyForQuery to be trigger a second time.
+  if (this.state !== 'done') {
+    // call sync to trigger a readyForQuery
+    this.connection.sync()
+  }
 }
 
 Cursor.prototype._getRows = function (rows, cb) {
diff --git a/packages/pg-query-stream/test/error.ts b/packages/pg-query-stream/test/error.ts
@@ -1,6 +1,7 @@
 import assert from 'assert'
 import helper from './helper'
 import QueryStream from '../src'
+import { Pool, Client } from 'pg'
 
 helper('error', function (client) {
   it('receives error on stream', function (done) {
@@ -21,3 +22,53 @@ helper('error', function (client) {
     client.query('SELECT NOW()', done)
   })
 })
+
+describe('error recovery', () => {
+  // created from https://github.com/chrisdickinson/pg-test-case
+  it('recovers from a streaming error in a transaction', async () => {
+    const pool = new Pool()
+    const client = await pool.connect()
+    await client.query(`CREATE TEMP TABLE frobnicators (
+      id serial primary key,
+      updated timestamp
+    )`)
+    await client.query(`BEGIN;`)
+    const query = new QueryStream(`INSERT INTO frobnicators ("updated") VALUES ($1) RETURNING "id"`, [Date.now()])
+    let error: Error | undefined = undefined
+    query.on('data', console.log).on('error', (e) => {
+      error = e
+    })
+    client.query(query) // useless callback necessitated by an older version of honeycomb-beeline
+
+    await client.query(`ROLLBACK`)
+    assert(error, 'Error should not be undefined')
+    const { rows } = await client.query('SELECT NOW()')
+    assert.strictEqual(rows.length, 1)
+    client.release()
+    const client2 = await pool.connect()
+    await client2.query(`BEGIN`)
+    client2.release()
+    pool.end()
+  })
+
+  // created from https://github.com/brianc/node-postgres/pull/2333
+  it.skip('handles an error on a stream after a plain text non-stream error', async () => {
+    const client = new Client()
+    const stmt = 'SELECT * FROM goose;'
+    await client.connect()
+    return new Promise((resolve, reject) => {
+      client.query(stmt).catch((e) => {
+        assert(e, 'Query should have rejected with an error')
+        const stream = new QueryStream('SELECT * FROM duck')
+        client.query(stream)
+        stream.on('error', () => {
+          console.log('stream error')
+          client.end((err) => {
+            console.log('client ended')
+            err ? reject(err) : resolve()
+          })
+        })
+      })
+    })
+  })
+})

Original file line number	Diff line number	Diff line change
`@@ -155,8 +155,12 @@ Cursor.prototype.handleError = function (msg) {`
`155`	`155`	`// only dispatch error events if we have a listener`
`156`	`156`	`this.emit('error', msg)`
`157`	`157`	`}`
`158`		`- // call sync to keep this connection from hanging`
`159`		`- this.connection.sync()`
	`158`	`+ // If we have already received a ready for query event do not double-sync`
	`159`	`+ // because this can cause a readyForQuery to be trigger a second time.`
	`160`	`+ if (this.state !== 'done') {`
	`161`	`+ // call sync to trigger a readyForQuery`
	`162`	`+ this.connection.sync()`
	`163`	`+ }`
`160`	`164`	`}`
`161`	`165`
`162`	`166`	`Cursor.prototype._getRows = function (rows, cb) {`