6

I want to promisify node-postgres' pg.connect method along with the inner connection.query method provided in the callback.

I can .promisify the latter, but I need to implement the first one manually (if I'm missing something here, please explain).

The thing is, I'm not sure if this code is correct or should be improved? The code is working, I just want to know if I'm using Bluebird as meant.

// aliases
var asPromise = Promise.promisify;

// save reference to original method
var connect = pg.connect.bind(pg);

// promisify method
pg.connect = function (data) {
  var deferred = Promise.defer();

  connect(data, function promisify(err, connection, release) {
    if (err) return deferred.reject(err);

    // promisify query factory
    connection.query = asPromise(connection.query, connection);

    // resolve promised connection
    deferred.resolve([connection,release]);
  });

  return deferred.promise;
};
luisfarzati
  • 8,649
  • 6
  • 29
  • 27
  • 1
    Just so, if you are not using a promise library that offers automatic connections and transactions management, you are effectively wasting that which is the best part about promises when it comes to databases. Head-on promisification like one implemented by Bluebird is a great generic tool, but it does no justice whatsoever to working with databases. – vitaly-t Mar 30 '15 at 01:24
  • 1
    For possible future readers, if wondering why promisifying by hand might be a bad idea: 1) the initiating function might throw exception that would go unnoticed 2) the resulting callback might be called multiple times. More info at http://bytearcher.com/articles/pitfalls-of-promisifying-by-band/ – pspi Nov 05 '15 at 09:02

4 Answers4

4

Throw all that horrible callback code away, then do this somewhere in your application initialization:

var pg = require("pg");
var Promise = require("bluebird");

Object.keys(pg).forEach(function(key) {
    var Class = pg[key];
    if (typeof Class === "function") {
        Promise.promisifyAll(Class.prototype);
        Promise.promisifyAll(Class);
    }
})
Promise.promisifyAll(pg);

Later in anywhere you can use the pg module as if it was designed to use promises to begin with:

// Later
// Don't even need to require bluebird here
var pg = require("pg");
// Note how it's the pg API but with *Async suffix
pg.connectAsync(...).spread(function(connection, release) {
     return connection.queryAsync("...")
         .then(function(result) {
            console.log("rows", result.rows);
         })
         .finally(function() {
            // Creating a superfluous anonymous function cos I am
            // unsure of your JS skill level
            release();
         });
});
Esailija
  • 138,174
  • 23
  • 272
  • 326
  • Thanks for your reply, @Esailija. My previous code was similar to yours, but what I don't like of `.promisifyAll` is the *Async suffix. In Node, methods don't declare their "async"ness in their name due to async being naturally the default. I would prefer to keep this consistency across my code. With this in mind, do you suggest alternatives to my manual wrapping? – luisfarzati May 27 '14 at 10:19
  • 1
    @luisfarzati yes, build Bluebird yourself and change the `AFTER_PROMISIFIED_SUFFIX` to whatever you want to. Alternatively, call `.promisify` and not `.promisifyAll` – Benjamin Gruenbaum May 27 '14 at 11:33
  • @luisfarzati the Async suffix doesn't mean asyncness but that the method returns a promise which is certainly not the default in node. Some suffix is necessary, Async was chosen because it is the same suffix used in C# for methods that return C# promises. – Esailija May 27 '14 at 11:57
  • 1
    @luisfarzati also not sure what you were planning but if you are worrying about consistency, it implies you plan to mix callbacks and promises, which is a severe anti pattern. – Esailija May 27 '14 at 12:00
  • @BenjaminGruenbaum thanks, I guess I'm doing fine with `.promisify`, then. @Esailija, you are right about promises vs async methods -- still semantically incorrect, then. I understand the rationale, though. My concern is just about how the code is read, which is subjective enough I know. I'm not mixing callbacks and promises anywhere in the code, this is being put in a helper library that wraps `pg.connect` and provides a promisified one. Other than that, I'm using promises all the time! – luisfarzati May 27 '14 at 20:54
  • So, given this scenario, if I want to provide a promisified `pg.connect` and its inner `connect.query`, while keeping the method name intact (an arguable decision, I agree)... am I doing ok? – luisfarzati May 27 '14 at 21:04
  • (EDIT: nvm about pending vs defer, I've just seen the source code --which I should do more often!--, it wasn't clear to me that they were synonims). – luisfarzati May 27 '14 at 21:04
  • 1
    @luisfarzati I would just change the suffix if "Async" is not fine for you (I will add an option for this in next version, never realized someone could have a problem with it). You should read the api documentation for public interface, any method in the source code that is not documented is either internal or deprecated. – Esailija May 28 '14 at 07:59
  • @Esailija: yes, this is what I'm planning to... I wasn't feeling comfortable trying to be smarter than bluebird only for being fussy with the name, so I've changed my mind to promisify them all using your code above and then renaming the couple of methods I'm going to use. About the suffix option, that'd be great! I'm really happy with bluebird and already evangelizing it among my colleagues :) Thanks for such amazing work! – luisfarzati May 29 '14 at 05:42
  • One of important virtues of the promises - it lets you reconsider the approach to the underlying API, swap the protocol for simpler and higher level at the same time, that which head-on promise injection doesn't, and you end up with the same low-level API with just THEN support. That's lazy coding. – vitaly-t Mar 29 '15 at 22:16
  • @Esailija Hi sir, could you help me on this topic? http://stackoverflow.com/q/38802095/6599627 thanks – mahdi pishguy Aug 06 '16 at 08:22
4

By now there are a number of libraries which do this for you:

Community
  • 1
  • 1
Jonathan Hall
  • 75,165
  • 16
  • 143
  • 189
  • 1
    You do realize the author of postgres-bluebird asked the original question? – Spencer Rathbun Mar 12 '15 at 15:14
  • @SpencerRathbun: Answers are not only for OPs--they are for future visitors as well. If the OP wishes to provide an answer providing a link to postgres-bluebird to get the credit, that would be appropriate. – Jonathan Hall Mar 12 '15 at 17:46
  • 1
    The first answer was to throw away one horrible approach and replace it with another horrible approach. When it comes to database usage promises can automatically manage connections and transactions lifespan, when done properly, of which you get none through head-on API promisification. – vitaly-t Mar 29 '15 at 22:27
  • @dman: that sounds like a general problem with node-postgres maintaining backward compatibility, and nothing to do specifically with these libraries. In any case, you're welcome to provide your own answer. I don't use node.js any more, so won't be updating my own answers. – Jonathan Hall Sep 21 '16 at 09:28
3

Update for bluebird 3:

The pg.connectAsync(...).spread(function(connection, release) { ... }) call will not work anymore, because the API of bluebird has changed: http://bluebirdjs.com/docs/new-in-bluebird-3.html#promisification-api-changes .

The problem is that promisifyAll in bluebird 3 does not handle multiple arguments by default. This results in the .spread() call reporting a TypeError like the following:

TypeError: expecting an array or an iterable object but got [object Null]

To solve this, you can explicitly enable multiple arguments for connect / connectAsync. Do the following after all the promisifying stuff mentioned above:

...
pg.connectAsync = Promise.promisify(pg.connect, { multiArgs: true });
Oliver
  • 279
  • 2
  • 8
0

I suggest to modify Petka Antonov solution a bit

var Promise = require('bluebird');
var pg = require('pg');
Object.keys(pg).forEach(function (key) {
  var Cls = null;
  try {
    Cls = pg[key];
    if (typeof Cls === 'function') {
      Promise.promisifyAll(Cls.prototype);
      Promise.promisifyAll(Cls);
    }
  } catch (e) {
    console.log(e);
  }
});
Promise.promisifyAll(pg);

here 'pg[key] wrapped up in try-catch block because pg[key] can retrun error when attempt to access pg['native']

Vlad Ankudinov
  • 1,936
  • 1
  • 14
  • 22