Reject query immediately if connection is being terminated

[juliusza]

Consider the following example:

const client = new Client({user: "acn", password: "foo", database: "shard_1"})

async function main() {
    await client.connect();

    console.log("Client connected, ending connection");
    client.end();

    const promise = client.query('SELECT pg_sleep(20)');

    console.log("Now waiting for DB to respond")

    const res = await promise
    console.log(res.rows)

    process.exit(0)
}

main()

Error will be emitted:

Error: Connection terminated
    at Connection.con.once (/opt/acn/head/api/node_modules/pg/lib/client.js:170:29)
    at Object.onceWrapper (events.js:313:30)
    at emitNone (events.js:111:20)
    at Connection.emit (events.js:208:7)
    at Socket.<anonymous> (/opt/acn/head/api/node_modules/pg/lib/connection.js:128:10)
    at emitNone (events.js:111:20)
    at Socket.emit (events.js:208:7)
    at endReadableNT (_stream_readable.js:1064:12)
    at _combinedTickCallback (internal/process/next_tick.js:138:11)
    at process._tickCallback (internal/process/next_tick.js:180:9)

It's not exactly clear what's going on. Now it is much more obvious if we reject query early:

Error: Could not accept query, because connection has ended
    at Client.query (/opt/acn/head/api/node_modules/pg/lib/client.js:378:22)
    at main (/opt/acn/head/api/test.js:10:28)
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:188:7)

This is quite an edge case, but I'm debugging this exact issue on my production environment. It's hard to know if TCP connections just die or client is ended externally.

[brianc]

Thanks! Could you write a test for this behavior?

[sehrope]

The non-callback case should return a rejected Promise. Otherwise await client.query(...) would resolve to undefined. I think all that needs to be changed is the return clause on L376 to:

return result

I haven't actually tried out the patch but that jumped out at me so figured to comment now if you're going to pursue this further. Could include both styles in a test as well (i.e. with callback and without).

[charmander]

This is fixed by #1503:

@@ -389,6 +435,20 @@ Client.prototype.query = function (config, values, callback) {
     query._result._getTypeParser = this._types.getTypeParser.bind(this._types)
   }
 
+  if (!this._queryable) {
+    process.nextTick(() => {
+      query.handleError(new Error('Client has encountered a connection error and is not queryable'), this.connection)
+    })
+    return result
+  }
+
+  if (this._ending) {
+    process.nextTick(() => {
+      query.handleError(new Error('Client was closed and is not queryable'), this.connection)
+    })
+    return result
+  }
+
   this.queryQueue.push(query)
   this._pulseQueryQueue()
   return result

[juliusza]

Thanks a lot for comments and review. It has been noted that #1503 implements a similar fix as well. I will not pursue this further until the mentioned pull is rejected or merged.

[charmander]

Fixed in pg 7.5.0.