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

client.query doesn't call back if connection has been lost #1322

Closed
pipmurphy opened this issue Jun 12, 2017 · 8 comments
Closed

client.query doesn't call back if connection has been lost #1322

pipmurphy opened this issue Jun 12, 2017 · 8 comments
Labels

Comments

@pipmurphy
Copy link

Hi - we've come across a scenario where client.query doesn't invoke the callback as we would expect:

If the connection is initially successful, but later disconnects (for example due to network issues), subsequent calls to client.query just seem to vanish into the ether. We had expected them to invoke the callback with an error object. Is this intended behaviour? If so, is there a workaround?

Having dug into lib/client.js line 307, I wonder if this is related:

Client.prototype._pulseQueryQueue = function() {
  if(this.readyForQuery===true) {
   ...

If the connection isn't ready for query, nothing seems to happen!

We're using pg@6.2.2 with node 6.9.1, and AWS Aurora Postgres.

Thanks in advance for any guidance you can give.

@brianc
Copy link
Owner

brianc commented Jun 12, 2017 via email

@brianc brianc modified the milestone: pg@7.0 Jun 12, 2017
@brianc brianc added the bug label Jun 12, 2017
@pipmurphy
Copy link
Author

Thanks for coming back to me so quickly @brianc :)

We've upgraded to pg@6.2.4 but sadly the behaviour seems to be the same :/

Here's an example test script I've put together to show the behaviour:

var pg=require('pg');

var pgClient = new pg.Client('made up url');

console.log('about to connect');

pgClient.connect( function( connectError ) {

  console.log('connected', connectError);

  console.log('about to query');

  pgClient.query('select * from sometable', function( queryError, result ) {

    console.log('queried',queryError);

  });

});

When I run this, it correctly outputs the error for 'connected', but it never displays 'queried'...

@brianc
Copy link
Owner

brianc commented Jun 12, 2017

Ahh okay - I can make the query result in an error on a client that has had its connection terminated by either a graceful or unexpected connection interruption - I see what you're saying about queries just disappearing into a client that isn't connected. Would that help?

@pipmurphy
Copy link
Author

Thanks @brianc, that would be perfect - I do think every callback like this needs to be invoked, if only with an error - that's certainly been my expectation in using this package (which is fantastic, btw) and the callback-with-error pattern generally.

@jussikinnula
Copy link

I'm having the problem as well in unit tests (using real database connection with test database(s)). When database connection fails, the tests continue timing out since pgClient.query() doesn't give the callback.

@ccakes
Copy link

ccakes commented Oct 10, 2017

Seems like query() should throw an error somewhere if there isn't an active connection. I recently switched from Pool to Client in a test script and blindly assumed that usage would be the same. Without calling dbh.connect() none of the error callbacks are triggered however no connection is established and everything seems to fail silently.

Simple example, seems to be identical with most pg & pg.native. I didn't open a new issue because I'm assuming the underlying cause is the same.

const process = require('process');
const { Client } = require('pg');

const db = new Client({
  connectionString: 'postgres://test@localhost:5433/test'  
});
db.on('error', err => console.log('ERR|> ', err));

// db.connect();

db.query('select now() as now')
  .then(res => console.log('RES|> ', res.rows))
  .then(() => process.exit(0))
  .catch(err => console.log('QERR|>', err));

@charmander

This comment has been minimized.

@charmander charmander marked this as a duplicate of #1454 Nov 14, 2017
@charmander
Copy link
Collaborator

@ccakes The queries are queued with the assumption that you’ll call connect() at some point in the future. I don’t think the query queue on clients is a good part of pg, but that, at least, is intended behaviour.

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

No branches or pull requests

5 participants