Search code examples
javascriptnode.jses6-promise

Issue with Nodejs promise


I have a sequence of function calls, connected with ES6 promises. Apparently, there is something wrong with this implementation, as API calls to the endpoint are not returning anything and the browser is stuck waiting for a response.

Please advise.

module.exports.insertTreatmentDetails = function (req, res) {
    var doctorId = 10000
    var departmentId = 10000
    var procedureid = 10000
    var hospitalSchema = new hospitalModel();

    var p = new Promise(function (resolve, reject) {
        counterSchema.getNext('Treatment.doctor.doctorId', collection, function (doctorId) {
            doctorId = doctorId;
        })
        counterSchema.getNext('Treatment.departmentId', collection, function (departmentId) {
            departmentId = departmentId
        })
        counterSchema.getNext('Treatment.procedureid', collection, function (procedureid) {
            procedureid = procedureid
        })
    }).then(function () {
        setData()
    }).then(function (){
        hospitalSchema.save(function (error, data) {
            if (error) {
                logger.error("Error while inserting record : - " + error)
                return res.json({ "Message": error.message.split(":")[2].trim() });
            }
            else {
                return res.json({ "Message": "Data got inserted successfully" });
            }
        });
    });
};

Solution

  • The short answer is that you aren't calling resolve or reject inside the first promise in your chain. The promise remains in a pending state. Mozilla has a good basic explanation of promises.

    How to Fix

    It appears that you want to retrieve doctorId, departmentId, and procedureId before calling setData. You could try to wrap all three calls in one promise, checking whether all three have returned something in each callback, but the ideal is to have one promise per asynchronous task.

    If it's feasible to alter counterSchema.getNext, you could have that function return a promise instead of accepting a callback. If not, I would recommend wrapping each call in its own promise. To keep most true to what your code currently looks like, that could look like this:

    const doctorPromise = new Promise((resolve, reject) => 
        counterSchema.getNext('Treatment.doctor.doctorId', collection, id => {
            doctorId = id;
            resolve();
        }));
    

    Then you could replace the first promise with a call to Promise.all:

    var p = Promise.all([doctorPromise, departmentPromise, procedurePromise])
        .then(setData)
        .then(/* ... */);
    

    Promises allow you to pass a value through to the next step, so if you wanted to get rid of your broadly-scoped variables (or set them in the same step where you call setData), you could just pass resolve as your callback to counterSchema.getNext and collect the values in the next step (also how you'd want to do it if you have counterSchema.getNext return a promise:

    Promise.all([/* ... */])
        .then(([doctorID, departmentID, procedureID]) => {
            // If you aren't changing `setData`
            doctorId = doctorID;
            departmentId = departmentID;
            procedureid = procedureID;
            setData();
            // If you are changing `setData`
            setData(doctorID, departmentID, procedureID);
        }).then(/* ... */).catch(/* I would recommend adding error handling */);