`pool.on('error')` doesn’t populate client error listeners as well

[ynahmany]

Hi, I came across a strange behavior recently. Env setup:

Linux- Ubuntu 18. Docker - 19.06 Node and Postgres running inside docker using docker-compose.

When I manually restarted(or shut down) Postgres docker -> the Node app crashes with Error:

Error: server closed the connection unexpectedly
 This probably means the server terminated abnormally
 before or while processing the request.
    at module.exports.Client._readError (/workspaces/node_modules/pg-native/index.js:154:13)
    at module.exports.Client._read (/workspaces/node_modules/pg-native/index.js:203:17)
    at PQ.emit (events.js:315:20)
Emitted 'error' event on  instance at:
    at module.exports.<anonymous> (/workspaces/node_modules/pg/lib/native/client.js:99:14)
at module.exports.emit (events.js:315:20)
at module.exports.Client._readError (/workspaces/node_modules/pg-native/index.js:155:8)
     at module.exports.Client._read (/workspaces/node_modules/pg-native/index.js:203:17)
at PQ.emit (events.js:315:20)

I configured the on.error(callback) listener yet I keep getting this error which causes my node application to crash

The solution was to add on.error to the client that is being returned from the callback above.

export const pool = new pg.native.Pool(config);

const handleConnectionFailure = (e: Error, _client: pg.PoolClient) => {
     _client.on('error', (err) => { // did the trick );
});
pool.connect(handleConnectionFailure);
pool.on('error', (err) => {
    // didnt help
});

[charmander]

That’s right – you need to add error handlers when checking clients out of the pool, in addition to the pool-level error handler, and remove them before returning clients to the pool. Correct error handling isn’t very easy with pg at the moment.

[ynahmany]

@charmander Thanks for the quick reply.

For future use, this one also work perfectly:

export let pool = new pg.native.Pool(config);

pool.on('connect', (_client: pg.PoolClient) => {
  // On each new client initiated, need to register for error(this is a serious bug on pg, the client throw errors although it should not)
  _client.on('error', (err: Error) => {
    console.log(err);
  });
});

[jszopi]

I notice that @ynahmany 's comment with the solution was linked from other issues and got quite a few :+1:s. I worry that the comment there:

(this is a serious bug on pg, the client throw errors although it should not)

may leave people with a poor impression of the library, while it's actually WAD. @charmander I notice you want to make this issue the "canonical" one for "error ergonomics". What options are you considering?

Looking at the events on the Pool, I feel like error should be qualified to be a clientError (to avoid confusion that it's the whole Pool that enters an error state). The existing event could then be qualified further to be an idleClientError, while the one that @ynahmany is after would be an activeClientError. Having both side-by-side would perhaps make the reader stop and think how they are different and which one they want to handle.

[jszopi]

One source of confusion may be that the the docs for the error event on the Client currently say:

When the client is in the process of connecting, dispatching a query, or disconnecting it will catch and foward errors from the PostgreSQL server to the respective client.connect client.query or client.end callback/promise; however, the client maintains a long-lived connection to the PostgreSQL back-end and due to network partitions, back-end crashes, fail-overs, etc the client can (and over a long enough time period will) eventually be disconnected while it is idle. To handle this you may want to attach an error listener to a client to catch errors.

AFAICT the word "idle" simply means a client not currently being (dis)connected or executing a query. The docs for the Pool use the word "idle" to mean clients which are not currently checked-out / acquired. Perhaps we need a glossary!

[jszopi]

One source of confusion may be that...

Correction: I now think that the two uses of the word "idle" mean the same thing, because the pool only checks out / acquires a client for the duration of query execution.

[jcalfee]

and remove them before returning clients to the pool

Are you saying there is a way to unregistered listeners of events?

[charmander]

Are you saying there is a way to [unregister] listeners of events?

@jcalfee Yes, the usual way.

[charmander]

Correction: I now think that the two uses of the word "idle" mean the same thing, because the pool only checks out / acquires a client for the duration of query execution.

This is true for pool.query, which handles connection errors already, but not for pool.connect.

[cattermo]

Does anyone know how to test this? What error will trigger this new error handler? I have seen the app crash in production (due to error handlers missing I guess) but I don't know how to trigger that locally.

[cattermo]

Does anyone know how to test this? What error will trigger this new error handler? I have seen the app crash in production (due to error handlers missing I guess) but I don't know how to trigger that locally.

Just found the answer for my own question. If you start postgres with docker, start a long running query and then kill postgres with docker kill db_container_name the pool client error is triggered.