2

After having read pages and pages about promises, I still cannot figure out the right way to return a promise (ES6 spec) inside a class function.

Here are the various options I've tried. utils.getMyPhotos() returns a promise.

1) Return the promise AND it's values

profilePictures(){
    return utils.getMyPhotos().then(function(result){
            var photosArray = result;
            photosArray.forEach(function(element) {
                this._list.push(new Picture(
                    element.src,
                    element.caption
                    ));
            }, this);
            return this._list;

        },function(error){
            console.log(error);
            return error;
        });
        }

2) Only return the promise' values

 profilePictures(){
        utils.getMyPhotos().then(function(result){
            var photosArray = result;
            photosArray.forEach(function(element) {
                this._list.push(new Picture(
                    element.src,
                    element.caption
                    ));
            }, this);
            return this._list;

        },function(error){
            console.log(error);
            return error;
        });
        }

3)Create a new Promise and return it

 profilePictures(){
 return new Promise(function(fulfill,reject){
 utils.getMyPhotos().then(function(result){
            var photosArray = result;
            photosArray.forEach(function(element) {
                this._list.push(new Picture(
                    element.src,
                    element.caption
                    ));
            }, this);
            fulfill(this._list);

        },function(error){
            console.log(error);
            reject(error);
        });
        }
 }

I try to use the above function as following :

pictures.profilePictures().then(function(result){
        console.log("all good");
        console.dump(result);
    },function(error){
        console.log("errors encountered");
        console.log(error);
    });

Hoever in the CLI I only see "errors encountered" followed by an empty error object.

What am I doing wrong?

Aaron Ullal
  • 4,855
  • 8
  • 35
  • 63
  • _"Only return the promise' values"_ A `Promise` contains exactly one value and you can't unwrap it (unless the function passed to `then` uses side effects), because `then` automatically returns a `Promise` –  Jul 08 '16 at 13:07

2 Answers2

2
  1. You are doing the right thing in here, but your error handler will not catch errors from its sibling success handler. I also would recommend that you don't mutate the instance variable this._list and return the result of doing this, as it is not clear from the function name that this is what it will do. Or maybe I'm being nit-picky. Anyway, see below for a recommended refactor.
  2. You are not returning anything. (See @Bergi's answer for why it is important to return!)
  3. Is a Promise anti-pattern. Watch out for those.

______

Refactor of (1)

I have moved the error handler so that it will catch errors in all previous handlers. And we throw the error after logging it so we can catch errors on consumption of the API, rather than handling them internally. Though this may not be what you want to do, it means errors don't get swallowed mysteriously.

profilePictures () {
    return utils.getMyPhotos()
        .then(function (result) {
            return result.map(function (element) {
                return new Picture(element.src, element.caption);
            });
        })
        .then(null, function (error){
            console.log(error);
            throw err;
        });
}

Consume it:

instance.profilePictures()
    .then(function (pics) {
        pics.forEach(function (pic) {
            // Do something with the Picture instance
        });
    })
    .then(null, function (err) {
        // Handle the error
    });
sdgluck
  • 24,894
  • 8
  • 75
  • 90
  • He is using native es6 promises, not bluebird – baao Jul 08 '16 at 13:05
  • @mmm I don't think I am using any of Bluebird in the refactor? Edit: ah, the link about anti-patterns? That's not Bluebird-specific I don't think. – sdgluck Jul 08 '16 at 13:09
  • 1
    Is there a particular reason that you don't use `catch` instead of `then(null,...)`? –  Jul 08 '16 at 13:20
  • 1
    @LUH3417 To sort of demonstrate that a `then` error handler doesn't catch errors from its sibling success handler. – sdgluck Jul 08 '16 at 13:22
  • Amazing answer! Could you share some url containing info about `then(null,function(error)` ? I couldn't understand that very much – Aaron Ullal Jul 08 '16 at 13:40
  • 1
    @AaronUllal: It's the same as `.catch(function(error) { … })` if that helps. See also [here](http://stackoverflow.com/q/24662289/1048572) for what he was refering to. – Bergi Jul 08 '16 at 13:49
1

1) Return the promise AND it's values

Yes! You always need to return promises from asynchronous functions, and from then callbacks you need to return values (or promises for them) to make them available to the next function in the chain.

2) Only return the promise' values

That doesn't work.

3) Create a new Promise and return it

That can work but is an antipattern. Avoid it.

Community
  • 1
  • 1
Bergi
  • 630,263
  • 148
  • 957
  • 1,375