promise chain not executing in order

Question

I'm having a weird problem, I call "createProductLine" in the following code :

module.exports.postProductLine = (req, res) => {
    DAL.createProductLine(req.body.category, req.body.type, req.body.name, req.body.description, req.body.tags)
    .then(() => {
        res.sendStatus(200)
        console.log("SUCCESS")
    })
    .catch(err => {
        res.status(500).send(err.message)
        console.error(err)
    })
}

and here is the "createProductLine" function :

module.exports.createProductLine = async (category, type, name, description, tags) => {
    const [pool, dbErr] = await tryCatch(await database.pool)
    if(dbErr) return Promise.reject()
    const [connection, connectErr] = await tryCatch(pool.getConnection())
    if(connectErr) return Promise.reject()
    connection.beginTransaction()
    .then(() => {
        const query = `INSERT INTO product_lines (category,type,name,description,last_update,creation_date) VALUES (?,?,?,?,NOW(),NOW())`
        return connection.query(query, [category, type, name, description])
    })
    .then(({insertId: productLineId} = {}) => Promise.all([
        Promise.resolve(productLineId),
        ...tags.map(productLineTag => new Promise(async (resolve, reject) => {
            let query = "SELECT id FROM tags WHERE name=?"
            const [existingTag, err1] = await tryCatch(connection.query(query, [productLineTag]))
            if(err1) return reject(`could not get tag (${productLineTag}) id`)
            if(existingTag.length === 0) {
                query = "INSERT INTO tags (name) VALUES (?)"
                const [{insertId}, err2] = await tryCatch(connection.query(query, [productLineTag]))
                if(err2) return reject(`could not insert tag (${productLineTag})`)
                return resolve(insertId)
            }
            return resolve(existingTag[0].id)
        }))
    ]))
    // CHANGE OF ORDER HAPPENS HERE
    .then(ids => Promise.all(
        ids.reduce((acc, cur, i, arr) => {
            if(i === 0) return acc
            acc[i - 1] = connection.query(
                `INSERT INTO product_lines_tags (product_line_id, tag_id) VALUES (${arr[0]}, ${cur})`
            )
            return acc
        }, new Array(ids.length - 1))
    ))
    .then(async () => {
        console.log("THEN")
        await connection.commit()
        return connection.release()
    })
    .catch(async err => {
        console.log("CATCH", err)
        await connection.rollback()
        await connection.release()
        return err
    })
}

For some reason the execution order of the all the .then doesn't go as expected, after finishing the second one (inside "createProductLine") it executes the .then in the first block of code (postProductLine function) and then comes back to execute the rest.

I don't understand why does it behave like this, do anyone have any idea on how to fix this please ?

This is how async code works. If you use `.then()` the callback will be added to a "list" of callbacks to execute, and executed later. If you are using an `async` function, just use `await` — yaakov, Jul 08 '21 at 19:36
First - try not to mix `async`/`await` and `.then()`/`.catch()`. It's just easier to stick to one. Second, see [Is it an anti-pattern to use async/await inside of a new Promise() constructor?](https://stackoverflow.com/q/43036229) and [What is the explicit promise construction antipattern and how do I avoid it?](https://stackoverflow.com/q/23803743) — VLAZ, Jul 08 '21 at 19:52
So res.sendStatus is called before "THEN" is printed. But after all the other callbacks? It is a little difficult to understand with your question . Try labelling different then blocks, with which one is run in what order. Just a sidenote: you do not have any console.log in your first 2 then blocks in the 2nd file. You can add them to check ghe order. — Tushar Shahi, Jul 08 '21 at 19:55
What's with this horrible `const […, err] = await tryCatch(somePromise); if(err) return Promise.reject()` pattern? Just write simply `const … = await somePromise`. Use `const … = await somePromise.catch(err => { throw new Error("better message"); })` if you need to hide the original error — Bergi, Jul 08 '21 at 21:07
@VLAZ thanks I didn't know about that, but in that case how would you return an array of promises with .then, should you nest them ? — ne3zy, Jul 08 '21 at 21:49
@TusharShahi yes, it reads 1st two .then, then sends the response, prints SUCCESS and then comes back to the 3rd then and prints THEN on the 4th. I removed most of the console.logs but I had them before. — ne3zy, Jul 08 '21 at 21:50
@Bergi but in case you have no catch and an error happens won't the error be put inside the const and break your code ? I saw that on youtube and thought it was good, is it not ? — ne3zy, Jul 08 '21 at 21:50
@all Thanks guys, I guess a fix is to recode it with all await but do you have any idea why this happens ? — ne3zy, Jul 08 '21 at 21:50
@ne3zy The error object would only end up in the `const` if you were to `return` it from the promise callback, not when you `throw` it. — Bergi, Jul 08 '21 at 21:51
@Bergi you are right I should be throwing inside the promise and catch it... thanks I'll be fixing this trash pattern — ne3zy, Jul 08 '21 at 21:54

score 0 · Accepted Answer · answered Jul 10 '21 at 09:32

module.exports.createProductLine = async (category, type, name, description, tags) => {
    const pool = await database.pool
    const connection = await pool.getConnection()
    await connection.beginTransaction()
    try {
        const {insertId: productLineId} = await connection.query(
            "INSERT INTO product_lines (category,type,name,description) VALUES (?,?,?,?)", 
            [category, type, name, description]
        )
        const tagIds = await Promise.all(tags.map(async tag => {
            const existingTag = await connection.query(
                "SELECT id FROM tags WHERE name=?", [tag]
            )
            if(existingTag.length === 0) { 
                const {insertId} = await connection.query(
                    "INSERT INTO tags (name) VALUES (?)", [tag]
                )
                return insertId
            }
            return existingTag[0].id
        }))
        await Promise.all(tagIds.map(tagId => connection.query(
            `INSERT INTO product_lines_tags (product_line_id, tag_id) VALUES (${productLineId}, ${tagId})`
        )))
        await connection.commit()
        await connection.release()
        return true
    }
    catch(err) {
        await connection.rollback()
        await connection.release()
        return Promise.reject(err)
    }
}

I remade the code like this, following the advice of the comments on the original post. It works great and looks much cleaner thanks all! — ne3zy, Jul 10 '21 at 09:33

promise chain not executing in order

1 Answers1