HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Promises with NodeJS and BlueBird

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
withpromisesnodejsandbluebird

Problem

I'm using bluebird for promises in my Node/Express application and wrote an API call in which the user passes in a JSON Web Token that contains their user information, and then I decode the token, and pull up the events that user should see based off their userId.

If anyone could give me advice on how I can clean this code up while also using the proper promise practices, that would be great.

/routes.js

app.post('/api/events/', require('./views/api/index').events);


/views/api/index.js

```
var B_Promise = require('bluebird');
var jwt = require('jsonwebtoken');

exports.events = function(req, res) {
var results = {};
var errors = [];

var validateEmptyFields = function() {
return new B_Promise(function(resolve, reject) {
var token = req.body.token || req.param('token') || req.headers['x-access-token'];
if (!token) {
return reject('Please provide the token parameter');
}

resolve(token);
});
};

var getUser = function(token) {
return new B_Promise(function(resolve, reject) {
jwt.verify(token, req.app.config.api.secret, function(err, decoded) {
if (err) {
return reject(err);
}

resolve(decoded);
});
});
};

var getEvents = function(user) {
return new B_Promise(function(resolve, reject) {
req.app.db.getConnection(function(err, connection){
if (err) {
return reject(err);
}

/ jshint multistr: true /
connection.query('SELECT e.* FROM events e \
INNER JOIN event_to_groups etg ON e.id=etg.event_id \
INNER JOIN user_to_groups utg ON utg.group_id=etg.group_id \
WHERE utg.user_id=?', user.id, function(err, events) {
if (err) {
return reject(err);
}

results.events = events;
resolve();
});

connection.release();
});
});
};

validateEmptyFields()
.then(getUser)
.then

Solution

-
I would just name the variable for Bluebird promises as Promise as it mostly acts like the standard Promise object in ES6. That way, if you happen to run an ES6-compatible Node.js, you can just remove the import, and you'll be set.

-
In the promise constructor, you need not do a return. The code will also feel awkward to a new developer, thinking Promise needs a return. You can simply do:

jwt.verify(token, req.app.config.api.secret, function(err, decoded) {
  if (err) reject(err); 
  else resolve(decoded);
});


-
Bluebird has a static method called Promise.promisify which converts callback-style APIs into promise-returning ones. Just make sure you follow it's guidelines in that:


function should conform to node.js convention of accepting a callback as last argument and calling that callback with error as the first argument and success value on the second argument.

-
If your operation needs to return a promise, but it's not async, then you can simply return a resolved or rejected promise instantly using Promise.resolve and Promise.reject static methods, respectively.

var validateEmptyFields = function() {
  var token = req.body.token || req.param('token') || req.headers['x-access-token'];
  return token ? Promise.resolve(token) : Promise.reject('Please provide the token parameter')
};


-
In getEvents, I suggest you resolve with events. Then assemble results in the same way you assembled errors in catch. The idea is that your promise-generating functions should not be causing side-effects. It should only be aware of the fact that you called it with some arguments, and it resolves or rejects and nothing more.

-
Move out that SQL query from the logic. It's messy.

And so, without further ado:

var Promise = require('bluebird');
var jwt = require('jsonwebtoken');

var EVENT_QUERY = '\
SELECT e.* FROM events e \
INNER JOIN event_to_groups etg ON e.id=etg.event_id \
INNER JOIN user_to_groups utg ON utg.group_id=etg.group_id \
WHERE utg.user_id=?';

exports.events = function(req, res) {
  var results = {};
  var errors = [];

  var validateEmptyFields = function() {
    var token = req.body.token || req.param('token') || req.headers['x-access-token'];
    if (!token) Promise.reject('Please provide the token parameter');
    else Promise.resolve(token);
  };

  var getUser = function(token) {
    return new Promise(function(resolve, reject) {
      jwt.verify(token, req.app.config.api.secret, function(err, decoded) {
        if (err) return reject(err);
        else resolve(decoded);
      });
    });
  };

  var getEvents = function(user) {
    return new Promise(function(resolve, reject) {
      req.app.db.getConnection(function(err, connection) {
        if (err) return reject(err);

        connection.query(EVENT_QUERY, user.id, function(err, events) {
          if (err) return reject(err);
          else resolve(events);
        });

        connection.release();
      });
    });
  };

  validateEmptyFields()
    .then(getUser)
    .then(getEvents)
    .then(function(events) {
      results.events = events;
    }, function(err) {
      errors.push(err);
    })
    .finally(function() {
      res.json({
        results: results,
        errors: errors
      });
    });
};

Code Snippets

jwt.verify(token, req.app.config.api.secret, function(err, decoded) {
  if (err) reject(err); 
  else resolve(decoded);
});
var validateEmptyFields = function() {
  var token = req.body.token || req.param('token') || req.headers['x-access-token'];
  return token ? Promise.resolve(token) : Promise.reject('Please provide the token parameter')
};
var Promise = require('bluebird');
var jwt = require('jsonwebtoken');

var EVENT_QUERY = '\
SELECT e.* FROM events e \
INNER JOIN event_to_groups etg ON e.id=etg.event_id \
INNER JOIN user_to_groups utg ON utg.group_id=etg.group_id \
WHERE utg.user_id=?';

exports.events = function(req, res) {
  var results = {};
  var errors = [];

  var validateEmptyFields = function() {
    var token = req.body.token || req.param('token') || req.headers['x-access-token'];
    if (!token) Promise.reject('Please provide the token parameter');
    else Promise.resolve(token);
  };

  var getUser = function(token) {
    return new Promise(function(resolve, reject) {
      jwt.verify(token, req.app.config.api.secret, function(err, decoded) {
        if (err) return reject(err);
        else resolve(decoded);
      });
    });
  };

  var getEvents = function(user) {
    return new Promise(function(resolve, reject) {
      req.app.db.getConnection(function(err, connection) {
        if (err) return reject(err);

        connection.query(EVENT_QUERY, user.id, function(err, events) {
          if (err) return reject(err);
          else resolve(events);
        });

        connection.release();
      });
    });
  };

  validateEmptyFields()
    .then(getUser)
    .then(getEvents)
    .then(function(events) {
      results.events = events;
    }, function(err) {
      errors.push(err);
    })
    .finally(function() {
      res.json({
        results: results,
        errors: errors
      });
    });
};

Context

StackExchange Code Review Q#105566, answer score: 4

Revisions (0)

No revisions yet.