Fetching user details, posts, and comments using promises in ExpressJS

Question

I'm creating an API service in NodeJS using Express (version 4) Framework. Created an MVC architecture for the same (no views, only model, and controller as this is an API).

Also, there are routes. When a request is made, the router redirects the request to the controller function and from there functions written in models are called and the response is returned to users in JSON format.

I gotta make multiple asynchronous requests from the controller for some functions. I earlier nested the asynchronous calls (callback hell) before I switched to Promises.

My NodeJS version is 6.12 and it does not support async/await.

I wanna make sure the way I implemented promises is correct.

Here is a sample function from my controller using which I fetch user details. The user passes a userID and using that I fetch user details. And using details from the 1st async request, I fetch user's posts and after that, I fetch user comments. I wanna make sure my implementation is correct.

Note: I removed error handling from some asynchronous requests to make the code look more understandable.

exports.fetchUserInformation = function (req, res) {
 var response = {};
 if (req.query.userId) {
 var userId = req.query.userId;
 response['userId'] = userId;
 new Promise(function (resolve, reject) {
 var param = {
 userId: userId
 };
 //1 - Call to Model function to fetch user info
 User.getUserDetails(param, function (err, rows) {
 if (err) {
 reject(err);
 } else {
 resolve(rows);
 }
 });
 }).then(function (result1) {
 var result2;
 response['userDetails'] = result1;
 //Pass result from previous promise to fetch next result
 var params1 = {
 userId: userId,
 result: result1
 };
 //2 - Call to Model function to fetch user timeline posts
 User.fetchUserPosts(params1, function (err, rows) {
 result2 = rows;
 });
 }).then(function (result2) {
 response['userPosts'] = result2;
 var params2 = {
 userId: userId,
 userType: 'user'
 };
 User.fetchUserComments(params2, function (err, rows) {
 response['userComments'] = rows;
 //return the response to user
 res.json({
 success: true,
 response: response
 });
 });
 }).catch(function (error) {
 //error logging starts
 var logData = {
 stackTrace: error
 };
 logHlpr.logThisError(logData); //calling another function to log data
 //error logging ends
 res.json({
 success: false
 });
 });
 }
};

I've few doubts:

I'm making 3 async requests to the Model - User.getUserDetails, User.fetchUserPosts, User.fetchUserComments using one promise. Should I implement 3 promises?

I'm pretty new to NodeJS and promises.

• Roamer-1888: Are you sure this works? What causes stage 3 to wait for stage 2, and how does result2 get passed through from stage 2 to stage 3?
• Smokey: I updated the code. @Roamer-1888
• 301_Moved_Permanently: Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers .
• 301_Moved_Permanently: @JohnDoe If you have new code, then you have a new question. See point 2 in the previous link.
• 200_success: @JohnDoe If you want your new code to be reviewed, then you should ask a follow-up question. As it stands, your self-"answer" is just a code dump, and will soon be deleted because it is not a review.

Answer 1

If you use 1 promise, you would encounter a long chain of then method and all of async operations would be executed in sequence style which increases response time.

If you create 3 promises, all of that can be performed in parallel. So the answer is yes, you should implement with 3 promises.

By the way, with help of bluebird library, You can write more readable JS code as I refactored show below, (I will explain more):

const bluebird = require('bluebird')
const User = require('./model/User')
bluebird.promisifyAll(User)
function asyncHandler(fn) {
 const promiseFn = bluebird.method(fn)
 return function(req, res) {
 let resultPromise = promiseFn.call(this, req,res)
 resultPromise.then(function(result) {
 res.json({
 success: true,
 response: result,
 })
 }).catch(function(err) {
 logHlpr.logThisError({stackTrace: err});
 res.json({
 success: false,
 })
 })
 }
}
exports.fetchUserInformation = asyncHandler(function(req, res) {
 if (!req.query.userId) {
 throw new Error("UserId is required")
 }
 let userId = req.query.userId
 let userDetails = User.getUserDetailsAsync({userId: userId})
 let userPosts = userDetails.then(result => User.fetchUserPostsAsync({userId: userId, result: result}))
 let userComments = User.fetchUserCommentsAsync({userId: userId, userType: 'user'})
 return bluebird.props({
 userId: userId,
 userDetails: userDetails,
 userPosts: userPosts,
 userComments: userComments,
 })
})

Most of the time, when you write JavaScript in Node.js, you need to work with callbacks, so convert all callback-style to promise-style by using bluebird.promisifyAll() or bluebird.promisify() will allow you to work with code easier, please see bluebird document.

After that most of handler function you write would deal with promise, so you can create higher-order function, like asyncHandler, which send success response in case that promise is fulfilled. You can also move asyncHandler function to shared-file and require() it later to use at multiple places.

For handler function, fetchUserInformation from your code sample, you want to return result as an object. This object contains result from many promises. You can use bluebird.props() which create a wrapper promise that wait all object's values to be fulfilled.

The last things, I would suggest you find a way to reimplement User.fetchUserPosts() that it wouldn't require userDetails as part of the parameter, so this method can be performed without waiting for User.getUserDetails() result and can make the code more elegant.

Answer 2

Writing three promises would be much better than the original code in the question, but further improvements might include :

• handling the case where if (req.query.userId) is falsy (or throws);
• flattening the promise chain
• passing response through from stage to stage adding properties as it goes
• having a single .catch() at the end.

For example, you could write :

exports.fetchUserInformation = function(req, res) {
 if(!(req.query && req.query.userId)) {
 return Promise.reject(new TypeError('request did not have the expected properties'));
 }
 // Stage 1 - Call to Model function to fetch user info
 return new Promise(function(resolve, reject) {
 var response = { 'userId': req.query.userId }; // embryonic response object
 User.getUserDetails(response, function(err, rows) {
 if(err) {
 reject(err);
 } else {
 response.userDetails = rows; // add .userDetails property to the response object ...
 resolve(response); // ... and pass to the next step in the chain
 }
 });
 })
 .then(function(response) {
 // Stage 2 - Call to Model function to fetch user timeline posts
 return new Promise((resolve, reject) {
 User.fetchUserPosts({ 'userId': response.userId, 'result': response.userDetails }, function(err, rows) {
 if(err) {
 reject(err);
 } else {
 response.userPosts = rows; // add .userPosts property to the response object ...
 resolve(reponse); // ... and pass to the next step in the chain
 }
 });
 });
 })
 .then(function(response) {
 // Stage 3 - Fetch user comments
 return new Promise(function(resolve, reject) {
 User.fetchUserComments({ 'userId': response.userId, 'userType': 'user' }, function(err, rows) {
 if(err) {
 reject(err);
 } else {
 response.userComments = rows; // add .userComments property to the response object ...
 resolve(response); // ... and pass to the next step in the chain
 }
 });
 });
 })
 .then(function(response) {
 // compose and deliver the full response
 res.json({
 'success': true,
 'response': response
 });
 })
 .catch(function(error) {
 logHlpr.logThisError({
 'stackTrace': error
 });
 res.json({ 'success': false });
 });
};

To simplify the code, you could avoid the need for new Promise() by using Bluebird promise lib's Promise.promisifyAll().

For example, with Bluebird installed you could write :

exports.fetchUserInformation = function(req, res) {
 if(!(req.query && req.query.userId)) {
 return Promise.reject(new TypeError('request did not have the expected properties'));
 }
 // Stage 1 - Fetch user info
 return User.getUserDetailsAsync({ 'userId': req.query.userId }) // call the promisified version of User.getUserDetails()
 .then(function(rows) {
 return { 'userId': req.query.userId, 'userDetails': rows }; // create embryonic response and pass to the next step in the chain
 });
 .then(function(response) {
 // Stage 2 - Fetch user timeline posts
 return User.fetchUserPostsAsync({ 'userId': response.userId, 'result': response.userDetails }) // call the promisified version of User.fetchUserPosts()
 .then(function(rows) {
 response.userPosts = rows; // add .userPosts property to the response object ...
 return reponse; // ... and pass to the next step in the chain
 });
 })
 .then(function(response) {
 // Stage 3 - Fetch user comments
 return User.fetchUserCommentsAsync({ 'userId': response.userId, 'userType': 'user' }) // call the promisified version of User.fetchUserComments()
 .then(function(rows) {
 response.userComments = rows; // add .userComments property to the response object ...
 return response; // ... and pass to the next step in the chain
 });
 })
 .then(function(response) {
 // compose and deliver the full response
 res.json({
 'success': true,
 'response': response
 });
 })
 .catch(function(error) {
 logHlpr.logThisError({
 'stackTrace': error
 });
 res.json({ 'success': false });
 });
};

Answer 3

Yes, you should implement three separate promises and chain them together for the desired effect.

You are triggering a new async request in the resolution callback of first and second promises. This creates the same callback hell, promises are designed to avoid.

You need to create three separate promises store them in variables or wrap them inside a named function and use chaining to trigger next promise after resolving first one.

Please refer this tutorial for the code samples.

Hi, John Doe, I saw your refactored code. It still is pyramidal and not flat. A more succinct code example, using vanilla javascript promises could be:

exports.fetchUserInformation = function (req, res) {
var response = {};
if (req.query.userId) {
var userId = req.query.userId;
response['userId'] = userId;
var getUserDetails = function(userId){ //returns first promise
 return new Promise(function (resolve, reject) {
 var param = {
 userId: userId
 };
 User.getUserDetails(param, function (err, rows) {
 if (err) {
 reject(err);
 } else {
 resolve(rows); //rows will be available for second promise to use as result1
 }
 });
 });
};
var getUserPosts = function(result1){ //returns second promise
 return new Promise(function (resolve, reject) {
 var result2;
 response['userDetails'] = result1;
 //Pass result from previous promise to fetch next result
 var params1 = {
 userId: userId,
 result: result1
 };
 //2 - Call to Model function to fetch user timeline posts
 User.fetchUserPosts(params1, function (err, rows) {
 if (err) {
 reject(err);
 } else {
 resolve(rows); //rows will be available for third promise to use as result2
 }
 });
 });
};
var fetchUserComments = function(result2){ //returns third promise
 return new Promise(function (resolve, reject) {
 response['userPosts'] = result2;
 var params2 = {
 userId: userId,
 userType: 'user'
 };
 User.fetchUserComments(params2, function (err, rows) {
 if (err) {
 reject(err);
 } else {
 response['userComments'] = rows;
 resolve(response); //response will be available for "resolve " function
 }
 });
 });
};
var getUserData = function(userId){
 getUserDetails(userId)
 .then(getUserPosts) // promise chaining
 .then(fetchUserComments) // previously resolved variables are available for this promise
 .then(function (response){
 res.json({
 success: true,
 response: response
 });
 })
 .catch( function (err){
 console.log(err);
 var logData = {
 stackTrace: err
 };
 logHlpr.logThisError(logData); //calling another function to log data
 //error logging ends
 res.json({
 success: false
 });
 });
}
getUserData(userId); // main function call which triggers all the promises
}
};

You can also use other promise libraries such as q or bluebird.