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 re-connection doesn't work #1352

Closed
sibedge opened this issue Jun 30, 2017 · 10 comments
Closed

Client re-connection doesn't work #1352

sibedge opened this issue Jun 30, 2017 · 10 comments
Milestone

Comments

@sibedge
Copy link
Contributor

sibedge commented Jun 30, 2017

I'm running a simple test against the latest pg, under Node.js 8.1.2:

const pg = require('pg');

var client = new pg.Client(connection);

function test() {
    client.connect(err => {
        if (err) {
            console.log(err);
            return;
        }
        client.query('select 123', [], (err, data) => {
            if (err) {
                console.log(err);
            } else {
                console.log('DATA:', data.rows[0]);
            }
            client.end();
        });
    });
}

setInterval(test, 1000);

And it outputs the following, every time:

DATA: anonymous { '?column?': 123 }
DATA: undefined
DATA: undefined
DATA: undefined
DATA: undefined
DATA: undefined
DATA: undefined
DATA: undefined
events.js:182
      throw er; // Unhandled 'error' event
      ^

error: invalid frontend message type 0
    at Connection.parseE (D:\NodeJS\tests\node_modules\pg\lib\connection.js:567:11)
    at Connection.parseMessage (D:\NodeJS\tests\node_modules\pg\lib\connection.js:391:17)
    at Socket.<anonymous> (D:\NodeJS\tests\node_modules\pg\lib\connection.js:129:22)
    at emitOne (events.js:120:20)
    at Socket.emit (events.js:210:7)
    at addChunk (_stream_readable.js:252:12)
    at readableAddChunk (_stream_readable.js:239:11)
    at Socket.Readable.push (_stream_readable.js:197:10)
    at TCP.onread (net.js:588:20)

After I successfully call client.end(), it will successfully re-connect, successfully execute the query, just won't return any data. This looks odd.

And this - error: invalid frontend message type 0 doesn't make any sense.

Could somebody, please explain, why we get such an odd output?

UPDATE

And if just after creating the client, I add the following:

client.on('error', e => {
    console.log(e);
});

The output gets absolutely flooded with a huge error dumb, so big, I can't even paste it here.

@charmander
Copy link
Collaborator

I’m almost certain clients aren’t meant to be connected more than once (@brianc, could you confirm this?); treat them as unusable after calling .end().

- var client = new pg.Client(connection);
-
 function test() {
+    var client = new pg.Client(connection);
+
     client.connect(err => {
         if (err) {
             console.log(err);
             return;
         }
         client.query('select 123', [], (err, data) => {
             if (err) {
                 console.log(err);
             } else {
                 console.log('DATA:', data.rows[0]);
             }
             client.end();
         });
     });
 }

@sibedge
Copy link
Contributor Author

sibedge commented Jun 30, 2017

I’m almost certain clients aren’t meant to be connected more than once...

And what is the reason for that?

@charmander
Copy link
Collaborator

They’re wrappers around a connection and should be associated with one connection.

@brianc
Copy link
Owner

brianc commented Jun 30, 2017

Yeah though it's not really documented clients are cheap to instantiate and should be considered 'used up' once they've been disconnected. There's a bit of a state machine inside the client w/ a bunch of event handlers being established after the connect event. Your best bet is going to be to throw the old one away & make a new one. The connection handshake over tcp is the part that takes a little bit of time, but reusing an existing client wouldn't save any time there as reconnecting would still need to happen - it would also introduce additional complexity in ensuring the old event handlers were disposed and the new ones set up correctly. I'll re-open this & add it to the 7.0 milestone to return an error if a client has connect called on it more than once - hopefully that will make things more clear.

@brianc brianc reopened this Jun 30, 2017
@brianc brianc added this to the pg@7.0 milestone Jun 30, 2017
@sibedge
Copy link
Contributor Author

sibedge commented Jul 1, 2017

It would be nice! The Client should throw an error when someone is trying to reconnect or call .query after calling .end(), the same as pg-pool should be throwing an error when someone calls .connect or .query after pool.end() has been called.

brianc added a commit that referenced this issue Jul 14, 2017
Clients are not reusable.  This changes the client to raise errors whenever you try to reconnect a client that's already been used.  They're cheap to create: just instantiate a new one (or use the pool) 😉.

Closes #1352
brianc added a commit that referenced this issue Jul 15, 2017
Clients are not reusable.  This changes the client to raise errors whenever you try to reconnect a client that's already been used.  They're cheap to create: just instantiate a new one (or use the pool) 😉.

Closes #1352
@brianc
Copy link
Owner

brianc commented Jul 15, 2017

Closed with #1365

@brianc brianc closed this as completed Jul 15, 2017
@waruwaruwaru
Copy link

waruwaruwaru commented Nov 27, 2017

I have a similar problem.

`const jwtLogin = new JwtStrategy(jwtOptions, function(payload, done) {

console.log('after connect');

client.connect();
client.query("SELECT FROM users where id = $1", [payload.sub], function(err, data) {

if (err) { return done(err, false); }
if (data) {
  console.log('inside data');
  client.end((err) => {
    console.log('client has been disconnected');
    if (err) {
      console.log('there has been error disconnecting from client: ', err);
    }
  });
  return done(null, data);
} else {
  console.log('hereeeeeeeeeeeeee?');
  client.end();
  return done(null, false);
}

});
});`

The first time I hit my endpoint that runs this code, everything works fine. I get my data back from the database fetch and it console.logged "client has been disconnected" but when I hit this endpoint again, my code only console.log('after connect') and none of my other code runs. I get
Unhandled promise rejection (rejection id: 1): Error: Client has already been connected. You cannot reuse a client.

Am I using client.end() incorrectly?

@charmander
Copy link
Collaborator

@waruwaruwaru Don’t end the client if you want to continue using it. If you want to create a new client each time instead of keeping one open, you’ll have to do just that (client = new Client(…); client.connect(…) in function (payload, done)). If you want a connection pool (this is the usual case), use Pool instead.

@waruwaruwaru
Copy link

@charmander Thank you.

@goodinmech
Copy link

goodinmech commented Jan 23, 2018

Someone might get misunderstood the comment above, therefore I shall note:

  1. Create a new client each time is slow
  2. Keep one open client is unreliable
  3. Use a Pool is the right way (https://node-postgres.com/features/pooling)

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