Skip to content

pg doesn't invoke callback if server accepts the connection and immediately disconnects #534

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

Closed
jess-sheneberger opened this issue Mar 13, 2014 · 15 comments · Fixed by #546
Milestone

Comments

@jess-sheneberger
Copy link

I've got a short repro of the issue: https://gist.github.com/jess-sheneberger/9532352

When I run the code in the gist, the output I get is:

server listening
client connecting
server connected
server socket destroyed.
server closed

...and then the program ends, meaning client.connect's callback was never invoked. I expected to see another line after "server closed", something like "Error on connect: " or "client connected".

Some context: I'm using postgres behind a HAProxy. Sometimes HAProxy accepts a connection, fails to connect to postgres and then disconnects. This causes my initialization scripts to fail consistently.

I can reproduce this on both my Ubuntu 12.04 VM and on OSX Mavericks.

@brianc
Copy link
Owner

brianc commented Mar 13, 2014

Hey! thanks for the awesome example code. I'll try to get this fixed for the v3.0 release (this weekend). Cheers.

@brianc brianc added this to the v3.0 milestone Mar 13, 2014
@jess-sheneberger
Copy link
Author

No problem, thanks for responding so quickly. Let me know if I can be of any assistance.

@strk
Copy link
Contributor

strk commented Mar 14, 2014

I might be experiencing this or a similar problem with [email protected] and pg@master (2.11.0+, 2716f95):
CartoDB/CartoDB-SQL-API#135

@strk
Copy link
Contributor

strk commented Mar 14, 2014

2.4.0 is also affected

@strk
Copy link
Contributor

strk commented Mar 17, 2014

The native module handles this better (although with obscure error):

server listening
client connecting
server connected
server socket destroyed.
server closed
Error on connect: Error: Unknown native driver error: 

@strk
Copy link
Contributor

strk commented Mar 17, 2014

@brianc the Connection object only sends 'error' or 'end' events. When the other end closes the connection, an 'end' is received by the "stream" object, not an 'error'. How to turn that into an 'error' based on expectance ?

@strk
Copy link
Contributor

strk commented Mar 17, 2014

Could we say, for example, that getting 'end' before getting 'readyForQuery' has to be considered an error ?

@strk
Copy link
Contributor

strk commented Mar 17, 2014

This seems to fix this case for me:

diff --git a/lib/client.js b/lib/client.js
index 490e1d1..73f910a 100644
--- a/lib/client.js
+++ b/lib/client.js
@@ -170,9 +170,17 @@ Client.prototype.connect = function(callback) {
       return self.emit('error', error);
     }
     callback(error);
+    callback = null;
   });

   con.once('end', function() {
+    if ( callback ) {
+      // haven't received a connection message yet !
+      var err = new Error("Stream unexpectedly ended before getting ready for query execution");
+      callback(err);
+      callback = null;
+      return;
+    }
     if(self.activeQuery) {
       var disconnectError = new Error('Stream unexpectedly ended during query execution');
       self.activeQuery.handleError(disconnectError);

@strk
Copy link
Contributor

strk commented Mar 17, 2014

Basically, if "callback" is still set, it means it wasn't called on "readyForQuery"

@strk
Copy link
Contributor

strk commented Mar 17, 2014

The patch is meant for 2.11.1

strk pushed a commit to CartoDB/node-postgres that referenced this issue Mar 17, 2014
strk pushed a commit to CartoDB/node-postgres that referenced this issue Mar 17, 2014
strk pushed a commit to CartoDB/node-postgres that referenced this issue Mar 18, 2014
strk pushed a commit to CartoDB/node-postgres that referenced this issue Mar 18, 2014
Test adapted by that provided by Jess Sheneberger in brianc#534
strk pushed a commit to CartoDB/node-postgres that referenced this issue Mar 18, 2014
This time, I hope, travis will confirm that the fix works with
node-0.10 but not with node-0.8
strk pushed a commit to CartoDB/node-postgres that referenced this issue Mar 18, 2014
Should fix missing connect callback call with node-0.8 (brianc#534)
@strk
Copy link
Contributor

strk commented Mar 18, 2014

@jess-sheneberger could you test #546 ?

strk pushed a commit to CartoDB/node-postgres that referenced this issue Mar 18, 2014
Should fix missing connect callback call with node-0.8 (brianc#534)
@jess-sheneberger
Copy link
Author

I’ll try to take a look later today. Thanks for the fix!

@jess-sheneberger
Copy link
Author

Works for me. Thanks again for fixing this!

@brianc
Copy link
Owner

brianc commented Mar 19, 2014

Sweet - I'll get to these ASAP, test 'em and merge them in.

@jess-sheneberger
Copy link
Author

@brianc Any idea when you'll be able to merge the fix to this one?

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