0

I call storage.createTask and use BlueBird's promises to get the returned value. This works fine when I do new Promise() and resolve the promise using either resolve(something) or reject(error). However, Promise.reject(new Error('some error')) causes cannot read property 'then' of undefined.

According to the docs, Promise.reject

Creates a promise that is rejected with the given reason

. Isn't this similar to reject(error), which works fine?

What's the difference between Promise.resolve/Promise.reject and doing new Promise? When should we use one over the other?

//server.js

// returning Promise.reject causes
// Cannot read property 'then' of undefined
    storage.createTask(task).then(function(id) {
        task.id = id;
        reply(task);
    }, function(error) {
        console.log(error);
        reply(error);
    });

// storage.js
function _create(task) {
    return new Promise(function(resolve, reject) {
        var id = shortid.generate();
        Task.create({
            id: id,
            content: task.content,
            deadline: task.deadline,
            milestone_id: task.milestone_id,
        }).catch(function (error) {
            reject(error); // works ok
        }).then(function() {
            resolve(id); //works ok
        });
    });
}

module.exports = {
    createTask: function(task) {
        if (task.milestone_id != null ) {
            Milestone.isExist(task.milestone_id).then(function(exists) {
                if (!exists) {
                    return Promise.reject(new Error('some error')); 
                }
                return _create(task);
            });
        } else {
        return _create(task);
        }
    }   
Yan Yi
  • 763
  • 1
  • 10
  • 29
  • 1
    http://stackoverflow.com/questions/23803743/what-is-the-explicit-promise-construction-antipattern-and-how-do-i-avoid-it – Benjamin Gruenbaum Sep 23 '15 at 16:53
  • Is your `createTask()` function supposed to return a promise because there is nothing that can even receive your rejected promise in that function so as you have it now, it's kind of pointless. – jfriend00 Sep 23 '15 at 16:59

1 Answers1

1

Your createTask() function does not return anything; you need to return the promise created by Milestone.isExist().

Update

Here's how I would rewrite the function:

createTask: function(task) {
    if (task.milestone_id == null ) {
        return Promise.reject(new Error('null id'));
    }

    return Milestone.isExist(task.milestone_id).then(function(exists) {
        if (!exists) {
            return Promise.reject(new Error('some error')); 
        }
        return _create(task);
    });
}

With this rewrite, you are always returning a promise from createTask(), so you can safely chain off of it.

joemfb
  • 3,056
  • 20
  • 19
  • thanks, I accidentally left out one `return _create(task);` line when typing this question. However, that was already in my code and `return Promise.reject` doesn't seem to return a valid Promise object – Yan Yi Sep 23 '15 at 17:02
  • @YanYi: Right. But you are only returning from the `else` block, not the `if` block. If `task.milestone_id != null` is `true` you are not returning anything from `createTask`. That's where the `undefined` comes from. It has nothing to do with `Promise.reject` (and it can't because of the way how `.then` works. `.then` always returns a promise, not matter what the callback returns). – Felix Kling Sep 23 '15 at 17:07