modernator modernator - 1 month ago 6
Node.js Question

How to stop promise chain after resolve?

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.

Answer

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(); // ???
            });
        });
    }
}