search

brianc / node-pg-pool

A connection pool for node-postgres
MIT License
180 stars 64 forks source link

Remove idleListener when client is in-use #123

Closed johanneswuerbach closed 5 years ago

johanneswuerbach commented 5 years ago

When a client is in-use, the error handling should be done by the consumer and not by the pool itself as this otherwise might cause errors to be handled multiple times.

The second commit is the actual fix here.

Based on https://github.com/brianc/node-pg-pool/pull/115

vitaly-t commented 5 years ago

@brianc Just poking 👉, to get this one merged

johanneswuerbach commented 5 years ago

@brianc sorry for the spam, but a review would be appreciated. Currently we need to float patches to keep up-to-date :-(

brianc commented 5 years ago

nope! Sorry I totally dropped the ball here. I'll merge this & push a new patch version today.

charmander commented 4 years ago

This means that for correct, complete error handling, people have to add an error listener and (for now) remember to remove it before returning the client to the pool.

The pool really needs to start wrapping its clients…

charmander commented 4 years ago

(Behaving exactly like ZeroPool plus an idle error event is probably a good target. Maybe requiring an explicit error handler, even if it’s the current default err => { throw err; }, would be helpful for the next major version.)

Gobie commented 4 years ago

This PR breaks pool.query error handling. It removes error handler on connect and attaches it back in release. pool.query calls client.query, which can emit error like ETIMEDOUT. This error can't be caught now.

johanneswuerbach commented 4 years ago

Sorry, I send a patch PR. .query should also add an error handler to the checked out client.

johanneswuerbach commented 4 years ago

PR https://github.com/brianc/node-pg-pool/pull/131