fredrover
fredrover

Reputation: 3025

Break out of Bluebird promise chain in Mongoose

I've studied several related questions & answers and still can't find the solution for what I'm trying to do. I'm using Mongoose with Bluebird for promises.

My promise chain involves 3 parts:

  1. Get user 1 by username

  2. If user 1 was found, get user 2 by username

  3. If both user 1 and user 2 were found, store a new record

If either step 1 or step 2 fail to return a user, I don't want to do step 3. Failing to return a user, however, does not cause a database error, so I need to check for a valid user manually.

I can use Promise.reject() in step 1 and it will skip step 2, but will still execute step 3. Other answers suggest using cancel(), but I can't seem to make that work either.

My code is below. (My function User.findByName() returns a promise.)

var fromU,toU;
User.findByName('robfake').then((doc)=>{
        if (doc){
            fromU = doc;
            return User.findByName('bobbyfake');
        } else {
            console.log('user1');
            return Promise.reject('user1 not found');
        }       
    },(err)=>{
        console.log(err);
    }).then((doc)=>{
        if (doc){
            toU = doc;
            var record = new LedgerRecord({
                transactionDate: Date.now(),
                fromUser: fromU,
                toUser: toU,
            });
            return record.save()
        } else {
            console.log('user2');
            return Promise.reject('user2 not found');
        }

    },(err)=>{
        console.log(err);
    }).then((doc)=>{
        if (doc){
            console.log('saved');
        } else {
            console.log('new record not saved')
        }

    },(err)=>{
        console.log(err);
    });

Upvotes: 1

Views: 299

Answers (4)

rsp
rsp

Reputation: 111296

Example

All you need to do is something like this:

let findUserOrFail = name =>
    User.findByName(name).then(v => v || Promise.reject('not found'));

Promise.all(['robfake', 'bobbyfake'].map(findUserOrFail)).then(users => {
    var record = new LedgerRecord({
        transactionDate: Date.now(),
        fromUser: users[0],
        toUser: users[1],
    });
    return record.save();
}).then(result => {
    // result of successful save
}).catch(err => {
    // handle errors - both for users and for save
});

More info

You can create a function:

let findUserOrFail = name =>
    User.findByName(name).then(v => v || Promise.reject('not found'));

and then you can use it like you want.

E.g. you can do:

Promise.all([user1, user1].map(findUserOrFail)).then(users => {
    // you have both users
}).catch(err => {
    // you don't have both users
});

That way will be faster because you don't have to wait for the first user to get the second one - both can be queried in parallel - and you can scale it to more users in the future:

let array = ['array', 'with', '20', 'users'];
Promise.all(array.map(findUserOrFail)).then(users => {
    // you have all users
}).catch(err => {
    // you don't have all users
});

No need to complicate it more than that.

Upvotes: 3

Bergi
Bergi

Reputation: 664307

Don't put error logging everywhere without actually handling the error - if you pass an error handler callback you'll get back a promise that will fulfill with undefined, which is not what you can need. Just use

User.findByName('robfake').then(fromUser => {
    if (fromUser) {
        return User.findByName('bobbyfake').then(toUser => {
            if (toUser) {
                var record = new LedgerRecord({
                    transactionDate: Date.now(),
                    fromUser,
                    toUser
                });
                return record.save()
            } else {
                console.log('user2 not found');
            }
        });
    } else {
        console.log('user1 not found');
    }
}).then(doc => {
    if (doc) {
        console.log('saved', doc);
    } else {
        console.log('saved nothing')
    }
}, err => {
    console.error("something really bad happened somewhere in the chain", err);
});

This will always log one of the "saved" or "something bad" messages, and possibly one of the "not found" messages before.

You can also use exceptions to achieve this, but it doesn't really get simpler:

var user1 = User.findByName('robfake').then(fromUser => {
    if (fromUser)
        return fromUser;
    else
        throw new Error('user1 not found');
});
var user2 = user1.then(() => // omit this if you want them to be searched in parallel
    User.findByName('bobbyfake').then(toUser => {
        if (toUser)
            return toUser;
        else
            throw new Error('user2 not found');
    })
);

Promise.all([user1, user2]).then([fromUser, toUser]) =>
    var record = new LedgerRecord({
        transactionDate: Date.now(),
        fromUser,
        toUser
    });
    return record.save();
}).then(doc => {
    if (doc) {
        console.log('saved', doc);
    } else {
        console.log('saved nothing')
    }
}, err => {
    console.error(err.message);
});

Upvotes: 0

Tamas Hegedus
Tamas Hegedus

Reputation: 29906

First, I would recommend using throw x; instead of return Promise.reject(x);, simply for readibility reasons. Second, your error logging functions catch all the errors, that's why your promise chain is continuing. Try rethrowing the errors:

console.log(err);
throw err;

Upvotes: 0

Johannes Merz
Johannes Merz

Reputation: 3342

move your error handling out of the inner chain to the place you want to actual catch/handle it. As i don't have mongo installed, here is some pseudocode that should do the trick:

function findUser1(){
  return Promise.resolve({
    user: 1
  });
}

function findUser2(){
  return Promise.resolve({
    user: 2
  });
}

function createRecord(user1, user2){
  return Promise.resolve({
    fromUser: user1,
    toUser: user2,
  });
}

findUser1()
  .then(user1 => findUser2()
      .then(user2 => createRecord(user1, user2))) // better nest your promises as having variables in your outside scope
  .then(record => console.log('record created'))
  .catch(err => console.log(err)); // error is passed to here, every then chain until here gets ignored

Try it by changing findUser1 to

return Promise.reject('not found 1');

Upvotes: 0

Related Questions