Skip to content

Client error: Connection terminated unexpectedly when server stops in the middle of running queries #2283

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

Open
HenriqueLBorges opened this issue Jul 17, 2020 · 10 comments

Comments

@HenriqueLBorges
Copy link

HenriqueLBorges commented Jul 17, 2020

Hello,

I'm using a connection pool. My program gets a client from the pool and releases it after the query.

   const client = await this.getClient();
   await client.query(statement);
   client.release();

I'm listening to all pool and client events. When my server stops the clients start to emit the following error event:

    Connection terminated unexpectedly
       at Connection.<anonymous> (/project/node_modules/pg/lib/client.js:272:71)
       at Object.onceWrapper (events.js:420:28)
       at Connection.emit (events.js:314:20)
       at Socket.<anonymous> (/project/node_modules/pg/lib/connection.js:102:10)
       at Socket.emit (events.js:326:22)
       at endReadableNT (_stream_readable.js:1226:12)
       at processTicksAndRejections (internal/process/task_queues.js:80:21)

The problem is, eventually I get an Unhandled 'error' event from clients. I already tried to use client.release(true) to destroy all the clients that emit an error event, tried to use pool.end() after a client sends an error event but nothing seems to work.

The only thing that works for me is calling client.release(true) after client.query(statement) every time even when there is no error, but there is no point in using a pool if I'm always destroying connections.

I'm using node-postgres 8.2.1.

Thanks in advance

@charmander
Copy link
Collaborator

Correct error handling when checking out a client is a bit cumbersome right now:


const pool = new Pool();

pool.on('error', error => {

An idle client was disconnected. Log an error, for example.

});

const clientErrorListener = error => {

An active client was disconnected. Log an error, for example.

When an active client encounters a connection error, in addition to emitting an 'error' event, its current and future queries will produce errors, which might be all the error handling you need if the client is only being used for a series of queries. Long-lived clients might need more work.

};

const client = await pool.connect();
let clientError = null;

client.on('error', clientErrorListener);

try {
    await client.query(statement);
} catch (err) {
    clientError = err;
} finally {
    client.release(clientError);
    client.off('error', clientErrorListener);
}

If you’re only making one query, though, the pool can do that for you.

await pool.query(statement);

@HenriqueLBorges
Copy link
Author

HenriqueLBorges commented Jul 20, 2020

Hello @charmander,

First, thanks for your reply. I tried both methods but unfortunately, I'm still experiencing errors.

First method:

try {
    const client = await this.pool.connect();
    client.on("error", (err) => this.errorProcess(err));
    await client.query(statement);
} catch (error) {
    err = error;
} finally {
    client?.release(err);
    client?.off("error", (error) => this.errorProcess(error));
}

Second method:

await this.pool.query(statement);

Both methods resulted on the same error:

Error: Connection terminated unexpectedly
    at Connection.<anonymous> (/project/node_modules/pg/lib/client.js:272:71)
    at Object.onceWrapper (events.js:420:28)
    at Connection.emit (events.js:314:20)
    at Socket.<anonymous> (/project/node_modules/pg/lib/connection.js:102:10)
    at Socket.emit (events.js:326:22)
    at endReadableNT (_stream_readable.js:1226:12)
    at processTicksAndRejections (internal/process/task_queues.js:80:21)
Emitted 'error' event on BoundPool instance at:
    at Client.idleListener (/project/node_modules/pg-pool/index.js:57:10)
    at Client.emit (events.js:326:22)
    at connectedErrorHandler (/project/node_modules/pg/lib/client.js:221:10)
    at Connection.<anonymous> (/project/node_modules/pg/lib/client.js:289:9)
    at Object.onceWrapper (events.js:420:28)
    [... lines matching original stack trace ...]

@charmander
Copy link
Collaborator

You still need the listener on the pool as well. Wherever you’re creating the pool,

pool.on('error', error => {
    console.error('idle client error:', error);
});

Also, off needs to be used with the original function passed to on, not just one with the same effect.

@HenriqueLBorges
Copy link
Author

HenriqueLBorges commented Jul 20, 2020

I refactored my code.

let error;
let client;
try {
    client = await this.pool.connect();
    client.on("error", clientErrorListener);
    await client.query(query);
    this.logInfo(`Row inserted`);
} catch (err) {
    error = err;
} finally {
    client?.off("error", clientErrorListener);
    client?.release(error);
}

Also my pool is listening to the error event:

this.pool.on("error", (err) => console.error('idle client error:', err));

Unfortunately, I'm still getting the same results.

@brianc
Copy link
Owner

brianc commented Jul 24, 2020

Do you have a self-contained script that can reproduce this problem? I'd like to turn it into a unit test & fix it.

@Lewiscowles1986
Copy link
Contributor

A Larger problem for me is that unexpected disconnections seem to lead to full traceback including keys if you specify them for server SSL.

@brianc
Copy link
Owner

brianc commented Aug 4, 2020

A Larger problem for me is that unexpected disconnections seem to lead to full traceback including keys if you specify them for server SSL.

that is an issue - i think we need to make that parameter non-enumerable. I can open a new issue on that one

@Lewiscowles1986
Copy link
Contributor

If you link to it, I'd be grateful to see the approach. There may be something I'm unaware of, but I am uncertain if it is node internals outputting or part of the library, where I guess an block-list or allow-list might be a reasonable thing to implement (or skipping config altogether) as you can then use something trivial (if it's application-side) such as Object.assign() (replace values with asterisks or similar "we can't let you see that".

@Lewiscowles1986
Copy link
Contributor

Lewiscowles1986 commented Aug 5, 2020

I've just tested and out-of-library this can be mitigated using the following after defining an object instance databaseConfig

Object.defineProperty(databaseConfig, 'password', {
  enumerable: false
})
Object.defineProperty(databaseConfig.ssl, 'key', {
  enumerable: false
})

This does not stop the connection, but does omit the private key and (if it was not already omitted) password if the server disconnects during running.

@rols2015
Copy link

please check: #2439 (comment)

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

No branches or pull requests

5 participants