How to stop promise chain after resolve?

Question

I want to stop promise chain after it resolved via some conditions. Below code is might useful to understand what am I saying.

function update(id, data) {
    return new Promise((resolve, reject) => {
        let conn;

        pool.get()
        .then((db) => {
            conn = db;

            if(Object.keys(data).length === 0) {
                return resolve({ updated: 0 });
            }
            else {
                return generateHash(data.password);
            }
        })
        .then((hash) => {
            conn.query("UPDATE ... ", (err, queryResult) => {
                if(err) {
                    throw err;
                }

                resolve({ updated: queryResult.affectedRows });
            });
        })
        .catch((err) => { ... })
    });
}

Note that pool.get() is promise wrapped API for getting connection pool from MySQL module that I made.

What I'm trying to do is updating user data. And for save server resources, I avoided to update if no data to update(Object.keys(data).length === 0).

When I tried this code, second then(updating db) is always happening even if no data to update!

I read this post, but it didn't worked. Why the promise chain wasn't stopped when I called "return resolve();"? And how to I stop it properly? I really like using Promises, but sometimes, this kind of things make me crazy. It will be very appreciate to help me this problem. Thanks!

P.S. I'm using node v6.2.2 anyway.

firstly, you're using the Promise constructor antipattern ... secondly, resolving in one part of the chain guarantees to fall through to the next .then - that's how promise chains work! — Jaromanda X, Oct 10 '16 at 09:59
You can take help here http://stackoverflow.com/questions/20714460/break-promise-chain-and-call-a-function-based-on-the-step-in-the-chain-where-it — abdulbarik, Oct 10 '16 at 09:59
Why would you wait until **after** you `pool.get` before checking if the incoming `data` has no keys? — Jaromanda X, Oct 10 '16 at 10:05
@torazaburo - that would be the value of `hash` in the next `.then` callback — Jaromanda X, Oct 10 '16 at 10:09
First of all, avoid the [`Promise` constructor antipattern](http://stackoverflow.com/q/23803743/1048572)! — Bergi, Oct 10 '16 at 10:26
@Bergi - going to need a Promise constructor eventually in that code :p — Jaromanda X, Oct 10 '16 at 10:28

score 5 · Accepted Answer · edited May 23 '17 at 10:34

Why the promise chain wasn't stopped when I called "return resolve();"?

You've returned from the current then callback and fulfilled the outer promise. But that doesn't "stop" anything, then then chain still will continue by resolving with the return value of the callback.

And how to I stop it properly?

You need to put the then call inside the if to have the condition apply to it:

pool.get()
.then((db) => {
    …
    if (Object.keys(data).length === 0) {
        …({ updated: 0 });
    } else {
        return generateHash(data.password)
        .then((hash) => {
            conn.query("UPDATE ... ", (err, queryResult) => {
                …
            });
        })
    }
})
.catch((err) => { ... })

And in any case, you should avoid the Promise constructor antipattern! You should only promisify the query method:

function query(conn, cmd) {
    return new Promise((resolve, reject) => {
        conn.query(cmd, (err, queryResult) => {
            if (err) reject(err); // Don't throw!
            else resolve(queryResult);
        });
    });
}

and then use that:

function update(id, data) {
    return pool.get()
    .then(conn => {
        if (Object.keys(data).length === 0) {
            conn.close(); // ???
            return { updated: 0 };
        } else {
            return generateHash(data.password)
            .then(hash => {
                return query(conn, "UPDATE ... ")
            }).then(queryResult => {
                conn.close(); // ???
                return { updated: queryResult.affectedRows };
            }, err => {
                …
                conn.close(); // ???
            });
        }
    });
}

Notice that it might not make sense to get a connection from the pool if you can know beforehand that no query will be made, so probably you should put the if on the top level:

function update(id, data) {
   if (Object.keys(data).length === 0) {
       return Promise.resolve({ updated: 0 });
    } else {
       return pool.get()
       .then(conn => {
            return generateHash(data.password)
            .then(hash => {
                return query(conn, "UPDATE ... ")
            }).then(queryResult => {
                conn.close(); // ???
                return { updated: queryResult.affectedRows };
            }, err => {
                …
                conn.close(); // ???
            });
        });
    }
}

hmmm `return resolve({ updated: 0 });` ... Promise.resolve perhaps? — Jaromanda X, Oct 10 '16 at 10:34
@JaromandaX Dunno, that's the non-cleaned-up version where I only focused on the `if`/`then` nesting — Bergi, Oct 10 '16 at 10:42
you missed the => in `conn.query(cmd, (err, queryResult) => {` — Jaromanda X, Oct 10 '16 at 10:44
there's a stray `;` after `return generateHash(data.password)` - ... and this also assumes that `generateHash` returns a promise, doesn't it? — Jaromanda X, Oct 10 '16 at 11:01
@JaromandaX Yes, that's the assumption - otherwise OP wouldn't have to use a `then` chain with it, would he? Btw, feel free to fix any typos with edits directly — Bergi, Oct 10 '16 at 11:19

JLRishe · Answer 2 · 2016-10-10T10:12:26.647

0

This would be a good situation to use an if statement:

function update(id, data) {
    if (Object.keys(data).length === 0) {
        return Promise.resolve({ updated: 0 }); 
    }

    let conn;

    return pool.get()
        .then((db) => {
            conn = db;

            return generateHash(data.password);
        })
        .then((hash) => {
            return new Promise(function (resolve, reject) {
                conn.query("UPDATE ... ", (err, queryResult) => {
                    if(err) {
                        reject(err);
                    }

                    resolve({ updated: queryResult.affectedRows });
                });
            });
        })
        .catch((err) => { ... })
}

edited Oct 10 '16 at 10:12

answered Oct 10 '16 at 10:09

JLRishe

99,490
19
131
169

1

replace `$q.when` with `Promise.resolve` and `$q(...` with `new Promise(...` - because there's no indication that this `$q` library is being used in the question – Jaromanda X Oct 10 '16 at 10:12
@JaromandaX Thank you. For some reason I thought it was an Angular question. – JLRishe Oct 10 '16 at 10:13

How to stop promise chain after resolve?

2 Answers2

Linked