Skip to content

Conversation

brianc
Copy link
Owner

@brianc brianc commented Aug 11, 2016

The first simple example shows: https://github.com/brianc/node-postgres/blame/master/README.md#L40

  1. client.end() should be called after client.connect()
  2. client.end() takes a callback

Client.end() documentation https://github.com/brianc/node-postgres/wiki/Client#method-end
states:

  1. "Clients created via the pg#connect method will be automatically disconnected or placed back into the connection pool and should not have their #end method called."
  2. "end() : null" i.e. end() does not take callback parameter

I don't understand where "pg#connect" method refers to. I can't find one. But in the simple example client.connect is used, so does this mean end() should not be called?

I stumbled on this when I took the simple example and wanted to move calling end() to server shutdown hook, but passing a callback to the end() did not work.

A long standing bug was the pure JS client didn't accept or call a callback on `client.end`.  This is inconsistent with both the documentation & general node patterns.

This fixes the issue & adds a test.  The issue did not exist in the native version of the client.
@brianc
Copy link
Owner

brianc commented Aug 11, 2016

  1. pg.connect is a global shortcut to access the pool and keep references to your pools inside a global singleton. Your best bet is to use the pool directly as some time in the far future I will deprecate pg.connect. Generally global singletons like that are kinda nasty & hard to maintain in big applications...The pg.connect API has been around for years so my steps to migrate to a user instantiated pool are first undocument pg.connect, second add a deprecation warning in the next major version bump of node-postgres, and finally remove it after another 6 months or so of deprecation...in the mean time just check out the pool documentation. 😉 I updated the wiki to reference the pool documentation too.

  2. end() not taking a callback parameter was a bug. I've attached a PR to fix it. I'll merge it & push out a new minor version with the API addition here in just a minute. 😄 Thanks for the bug report!

closes #1107

@brianc brianc merged commit a536afb into master Aug 11, 2016
@brianc brianc deleted the 1106-end-callback branch June 8, 2017 03:57
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

Successfully merging this pull request may close these issues.

1 participant