Awaiting on pgcursor.close may never resolve

[FreeSlave]

I'm using pgcursor on Amazon Aurora PostgreSQL. If connection is terminated due to serverless scale event timeout, the promise returned by close methods of the Cursor never resolves. Probably because it never gets the readyForQuery event https://github.com/brianc/node-postgres/blob/master/packages/pg-cursor/index.js#L218

I made an assumption that the same can happen on regular postgresql if we manually abrupt the connection from the server side (terminating connection due to administrator command) while cursor is open and I was able to replicate it with this code (Obviously you need to fill configure clients with your settings):

const { Client } = require('pg')
const PgCursor = require('pg-cursor')

const client = new Client({
    user: process.env.POSTGRES_USER,
    host: process.env.POSTGRES_HOST,
    database: process.env.POSTGRES_DATABASE,
    password: process.env.POSTGRES_PASSWORD,
    port: +(process.env.POSTGRES_PORT || 5432),
})
client.on('error', function(err: Error) {
    console.error('In error handler:', err)
})

const terminatingClient = new Client({
    user: process.env.POSTGRES_USER,
    host: process.env.POSTGRES_HOST,
    database: process.env.POSTGRES_DATABASE,
    password: process.env.POSTGRES_PASSWORD,
    port: +(process.env.POSTGRES_PORT || 5432),
})
terminatingClient.on('error', function(err: Error) {
    console.error('terminatingClient In error handler:', err)
})

function terminatePostgresConnectionQuery(processID: number, db: string) {
    return `SELECT pg_terminate_backend(pid) FROM 
    pg_stat_activity WHERE 
    pid = ${processID}
    AND datname = '${db}';`
}

;(async() => {
    try {
        await client.connect()
        await terminatingClient.connect()

        const pgCursor = (client.query(
            new PgCursor(`SELECT NOW()`)
        ))
        console.log('Process id:', client.processID)
        await terminatingClient.query(
             terminatePostgresConnectionQuery(client.processID, process.env.POSTGRES_DATABASE ?? 'db'))

        console.log('Made terminatingQuery')
        await pgCursor.close()
        console.log('cursor closed')
    }
    catch(err) {
        console.error('In catch: ', err)
    } finally {
        try {
            await client.query('SELECT NOW()')
        } catch(err) {
            console.error('Error in finally', err)
        }
        console.log('Exiting')
        await client.end()
        await terminatingClient.end()
    }
})()

Here we create two connections. One is used to create a cursor and make a select query. Another is used to kill the first one via pg_terminate_backend.

It never reaches console.log('cursor closed').

[pasieronen]

I've just spent couple of hours debugging pg-query-stream related issues, and I think this bug is also the root cause of issues people are having with pg-query-stream.

In QueryStream._read function (https://github.com/brianc/node-postgres/blob/master/packages/pg-query-stream/src/index.ts#L55), if this.cursor.read leads to an error, then this.destroy gets called:

  public _read(size: number) {
    this.cursor.read(size, (err: Error, rows: any[]) => {
      if (err) {
        // https://nodejs.org/api/stream.html#stream_errors_while_reading
        this.destroy(err)
      } else {
        ...

Eventually this leads to calling QueryStream._destroy(), which has this code:

 public _destroy(_err: Error, cb: Function) {
    this.cursor.close((err?: Error) => {
      cb(err || _err)
    })
  }

Apparently the stream ends properly only when cb has been called... and if this.cursor.close never calls us back (or resolves the promise), then code using QueryStream just hangs.

So fixing this bug should also fix at least #2187, #2468, #2870, and probably #2588

I'm not quite sure how this bug should be fixed, though. PR #2807 proposed checking this.connection._ending, but for some reason, connection.sync() also sets _ending to true, so the check doesn't sound right. (Or perhaps connection.sync() setting _ending = true is wrong?)

[mootari]

This workaround (or a similar monkeypatch of stream.destroy) seems to do the trick:

import QueryStream from "pg-query-stream";

class QueryStreamAurora extends QueryStream {
  destroy(err) {
    if (err) this.emit("error", err);
    return super.destroy(err);
  }
}