Chaining more than one promise function in Node.js for mongoDB

Question

There is a requirement, that review can be added only when product and user both are found. So i have written code in order to implement this scenario.

User.findById(req.params.userId).exec()
        .then(response => {
            console.log("response", response);
            if (response) {
                return Product.findById(req.params.productId).exec();
            }
            else {
                return res.status(404).json({
                    message: 'User not found'
                })
            }
        })
        .then(response => {
            console.log("response", response);
            if (!response) {
                return res.status(404).json({
                    message: 'Product not found'
                })
            }

            const review = new Review({
                _id: mongoose.Types.ObjectId(),
                rev: req.body.rev,
                productId: req.params.productId,
                userId: req.params.userId
            });

            return review.save();

        })
        .then(response => {
            console.log("responseeeeeeee", response);
            res.status(201).json({
                response
            })
        })
        .catch(error => {
            console.log("error", error);
            res.status(500).json({
                error
            })
        })

This is working fine, but as soon as product or user is missing it is throwing a warning as such :

(node:17276) UnhandledPromiseRejectionWarning: Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client
    at ServerResponse.setHeader (_http_outgoing.js:470:11)
    at ServerResponse.header (D:\backup-learning\project-shop-always\node_modules\express\lib\response.js:771:10)
    at ServerResponse.send (D:\backup-learning\project-shop-always\node_modules\express\lib\response.js:170:12)
    at ServerResponse.json (D:\backup-learning\project-shop-always\node_modules\express\lib\response.js:267:15)
    at User.findById.exec.then.then.then.catch.error (D:\backup-learning\project-shop-always\api\controllers\review-controller.js:58:29)
    at process._tickCallback (internal/process/next_tick.js:68:7)

Answer 1

The problem is that the chain continues with the return value of res.status as the fulfillment value when you do return res.status(/*...*/) from within the then callback.

You can't use a single chain for this. Also, since you need to locate both the user and the product, probably best to do that bit in parallel. See comments:

// *** Start both the product and user search in parallel
Promise.all([
    User.findById(req.params.userId).exec(),
    Product.findById(req.params.productId).exec()
])
.then(([user, product]) => {
    // *** Handle it if either isn't found
    if (!user) {
        res.status(404).json({
            message: 'User not found'
        });
    } else if (!product) {
        res.status(404).json({
            message: 'Product not found'
        });
    } else {
        // *** Both found, do the review
        const review = new Review({
            _id: mongoose.Types.ObjectId(),
            rev: req.body.rev,
            productId: req.params.productId,
            userId: req.params.userId
        });
        // *** Return the result of the save operation
        return review.save()
        .then(response => {
            console.log("responseeeeeeee", response);
            res.status(201).json({
                response
            });
        }
    }
    // *** Implicit return of `undefined` here fulfills the promise with `undefined`, which is fine
})
.catch(error => {
    // *** An error occurred finding the user, the product, or saving the review
    console.log("error", error);
    res.status(500).json({
        error
    })
});

If you're doing this in any modern version of Node.js, you can use an async function and await:

// *** In an `async` function
try {
    const [user, product] = await Promise.all([
        User.findById(req.params.userId).exec(),
        Product.findById(req.params.productId).exec()
    ]);
    if (!user) {
        res.status(404).json({
            message: 'User not found'
        });
    } else if (!product) {
        res.status(404).json({
            message: 'Product not found'
        });
    } else {
        // *** Both found, do the review
        const review = new Review({
            _id: mongoose.Types.ObjectId(),
            rev: req.body.rev,
            productId: req.params.productId,
            userId: req.params.userId
        });
        const response = await review.save();
        console.log("responseeeeeeee", response);
        res.status(201).json({
            response
        });
    }
} catch (error) {
    console.log("error", error);
    res.status(500).json({
        error
    })
}

Note that the entire code is wrapped in a try/catch, so the async function this is in will never reject (unless the console.log or res.send in the catch block raised errors), so it won't result in an unhandled rejection warning if you just make your Express endpoint handler async (whereas normally passing an async function into something that isn't expecting to receive a promise is an anti-pattern). (If you want to be a bit paranoid, wrap the contents of the catch block in another try/catch.)