Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PostgresQueryRunner not handling idle connection errors #5999

Closed
joras opened this issue May 4, 2020 · 2 comments
Closed

PostgresQueryRunner not handling idle connection errors #5999

joras opened this issue May 4, 2020 · 2 comments

Comments

@joras
Copy link
Contributor

joras commented May 4, 2020

Issue type:

[ ] question
[x] bug report
[ ] feature request
[ ] documentation issue

Database system/driver:

[ ] cordova
[ ] mongodb
[ ] mssql
[ ] mysql / mariadb
[ ] oracle
[x] postgres
[ ] cockroachdb
[ ] sqlite
[ ] sqljs
[ ] react-native
[ ] expo

TypeORM version:

[ x] latest
[ ] @next
[ ] 0.x.x (or put your version here)

Steps to reproduce or a small repository showing the problem:

PostgresQueryRunner is not handling idle connection errors, thus all the connection related errors are unhandled and process usually dies with unexpected error.

    at Connection.<anonymous> (/Users/jaan/Projects/typeormtest/node_modules/pg/lib/client.js:275:71)
    at Object.onceWrapper (events.js:417:28)
    at Connection.emit (events.js:311:20)
    at Connection.EventEmitter.emit (domain.js:482:12)
    at Socket.<anonymous> (/Users/jaan/Projects/typeormtest/node_modules/pg/lib/connection.js:126:10)
    at Socket.emit (events.js:323:22)
    at Socket.EventEmitter.emit (domain.js:482:12)
    at endReadableNT (_stream_readable.js:1204:12)
    at processTicksAndRejections (internal/process/task_queues.js:84:21)

It is not quite obvious when reading node-postgres documentation, but it is there under https://node-postgres.com/api/client - 'event' section. The client returned from pool.connect() must handle on('error') events because this is where idle connection error are returned. It seems pool.on('error') is not enough.

Also if you look node-postgres pool.query() function you can see they are handling on('error') and client.query() errors.

Now that said, the issue is made worse by a bug in in node-postgres - when idle connection error handler is not attached, client.query() does not actually throw at all brianc/node-postgres#2191 .

Of course if this issue is fixed without fixing #5112 the connection would be returned to the pool until the pool fills with broken connections.

Small example to play with, if you stop postgres, the exceptions are not caught and process dies :
https://gist.github.com/joras/03bf07a7eb0ab0f7bc755f15b510b432

@hotsezus
Copy link

I am experiencing wrong handling of first query after internet disconnection because of this bug. It just hangs for infinite time while all of the next queries are throwing correct exceptions which can be handled on other levels.
Handling of exactly the same case on node-postgres level is described here pg.Client -> events

// walk over to server, unplug network cable
// process output: 'something bad has happened!' followed by stacktrace :P

@imnotjames
Copy link
Contributor

This has been merged so we're gonna close this out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants