patternjavascriptMinor
Promises with NodeJS and BlueBird
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
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
/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
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
-
In the promise constructor, you need not do a
-
Bluebird has a static method called
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
-
In
-
Move out that SQL query from the logic. It's messy.
And so, without further ado:
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.