CODEWITHSUNDEEP
node.jspromisesequelize.js
franciscofrr
franciscofrr

Reputation: 57

Sequelize nested promise on model create inside a loop

I have an async function (createObjects) that needs to create some models into the database, so, until all the objects are created (inside a forEach loop), the function needs to wait. After the last model is created, then it should return the "Data Synchronized!" string, but the "Data Synchronized!" message never waits for createObjects() to finish. I think I need to return all the model.create promises to the mainPromise, like an array of promises, but I don't know the best way to do that. Any suggestions?

PS: The calculateCost that is called inside createObjects is async and is working fine.

mainPromise()
.then( (data) => {
    return proccessData(data); //this is a sync function
})
.then( (newData) => {
    createObjects(newData); // this is a async function
})
.then( () => {
    return "Data Synchronized!";
})

//this needs to be an async function
function createObjects(newData){
newData.forEach((bills) => {
    //if the object bills has the Groups array attributes...
    if (bills.Groups.length !== 0) {
        //iterate through groups
        bills.Groups.forEach( (group) => {
        var uid = group.id;
        var usage = group.Metric.UsageAmount;
        var cost = calculateCost(usage, uid); //async function

        //the problem is here
        cost.then((result) => {
            models.Billing.create({
                group_id: uid,
                cost: result,
                usage: usage          
            });
        });
        })
    }
});
}

var calculateCost = (usage, uid) => {
    var cost;

    return models.ObjectA.findOne({
        where: { id: uid }
    }).then((data) => {
        if (data.type == "Interactive") {
            cost = usage * 0.44;
        } else if (data.type == "Automated") {
            cost = usage * 0.11;
        }
        return cost;
    });
}

Upvotes: 2

Views: 1445

Answers (1)

JLRishe
JLRishe

Reputation: 101778

There is nothing in your code watching the result of cost().then(...), so that bit of code is fire-and-forget. The same is true for your call to models.Billing.create and one of the thens toward the top of your code. That's why you're seeing the outcome you are. When using Promises, be on the lookout for places where you are creating promises and not returning them to a higher caller. This often suggests the creation of a promise that isn't being watched.

To fix this:

First of all, fix the then towards the top of the code so that the result of createObjects is actually being returned:

.then( (newData) => {
    return createObjects(newData); // this is a async function
})

Better yet:

.then(createObjects)

After that's remedied...

Option 1 - use reduce instead of forEach

Use this approach if you want to assure that the queries are executed one at a time (in sequence), instead of all at once:

function processBillGroups(groups) {
    return groups.reduce((last, group) => {
        var group_id = group.id;
        var usage = group.Metric.UsageAmount;

        return last
            .then(() => calculateCost(usage, group_id))
            .then((cost) => models.Billing.create({ group_id, cost, usage }))
    }, Promise.resolve());
}

function createObjects(newData) {
    return newData.reduce(
        (last, { Groups }) => last.then(() => processBillGroups(Groups)),
        Promise.resolve(),
    );
}

Option 1.1 Use async/await

This will also carry out the actions in sequence, but uses the async/await syntax instead of direct promise manipulation.

async function processBillGroups(groups) {
    for (group of groups) {
        let group_id = group.id;
        let usage = group.Metric.UsageAmount;

        let cost = await calculateCost(usage, group_id);

        await models.Billing.create({ group_id, cost, usage });
    }
}

async function createObjects(newData) {
    for ({ Groups } of newData) {
        await processBillGroups(Groups);
    }
}

Option 2 - use map and Promise.all instead of forEach

Use this if you don't mind all of the actions executing at the same time (in parallel), or even prefer that they execute in parallel. createObjects will return a single promise that will resolve when all of the actions have completed:

function processBillGroups(groups) {
    return Promise.all(groups.map((group) => {
        var group_id = group.id;
        var usage = group.Metric.UsageAmount;

        return calculateCost(usage, group_id)
            .then((cost) => models.Billing.create({ group_id, cost, usage }));
    }));
}

function createObjects(newData) {
    return Promise.all(
        newData.map(({ Groups }) => processBillGroups(Groups))
    );
}

Option 2.1 - Use map and Promise.all with a little bit of async/await:

Acts just like option 2, but the syntax is a little nicer.

function processBillGroups(groups) {
    return Promise.all(groups.map(async (group) => {
        let group_id = group.id;
        let usage = group.Metric.UsageAmount;

        let cost = await calculateCost(usage, group_id);

        await models.Billing.create({ group_id, cost, usage });
    }));
}

function createObjects(newData) {
    return Promise.all(
        newData.map(({ Groups }) => processBillGroups(Groups))
    );
}

Upvotes: 4

Related Questions