2

I have constructed a function which iterates through a Generator containing both synchronous code and Promises:

module.exports = {


    isPromise (value) {
            return typeof value === 'object' && value !== null && 'then' in value;
        },

    runGen (generatorFunc, startValue) {

            let that = this,
                i = 0;

            function *iterator(resolve, reject) {

                let runGeneratorFunc = generatorFunc(startValue),
                    yieldedOut = {done: false},
                    yieldIn;

                while (!yieldedOut.done) {
                    console.log(i++, 'Ready for next iteration');
                    if (that.isPromise(yieldedOut.value)) {
                        console.log(i++, 'Pass promise to KeepIterating');
                        yieldIn = yield yieldedOut.value;
                        console.log(i++, 'Received value from promise');

                        if(yieldIn instanceof Error){
                            console.log(i++, 'Value was instance of Error');
                            try {
                                yieldedOut = runGeneratorFunc.throw(yieldIn)
                            }
                            catch(err){
                                console.log(i++, 'Throw Error');
                                throw(yieldIn);
                            }
                        } else {
                            yieldedOut = runGeneratorFunc.next(yieldIn);
                        }
                    } else {
                        try {
                            yieldIn = yieldedOut.value;
                            yieldedOut = runGeneratorFunc.next(yieldIn);
                        }
                        catch(err) {
                            runGeneratorFunc.throw(err);
                            reject(err);
                        }
                    }
                }
                resolve(yieldedOut.value);
            }

            return new Promise(function (resolve, reject) {
                var runIterator = iterator(resolve, reject);
                (function keepIterating(yieldIn) {
                    let yieldedOutPromise = runIterator.next(yieldIn);

                    if (!yieldedOutPromise.done) {

                        yieldedOutPromise.value.then(function (fulfilledValue) {
                            console.log('never gets here');
                            keepIterating(fulfilledValue);
                        });

                        yieldedOutPromise.value.catch(function (err) {
                            console.log(i++, 'Rejected promise catched');
                            if (err instanceof Error) {
                                try {
                                    console.log(i++, 'Rejected promise is instance of Error');
                                    let yieldedOut = runIterator.next(err);
                                    keepIterating(yieldedOut);
                                }
                                catch (err) {
                                    console.log(i++, 'Error propagated back out');
                                    yieldedOutPromise.value.catch(() => {})
                                    reject(err);
                                }
                            } else {
                                try {
                                    let yieldedOut = runIterator.next(new Error(err));
                                    keepIterating(yieldedOut);
                                }
                                catch (err) {
                                    reject(err);
                                }
                            }
                        })
                    }
                })();
            });
        }
    }

Now when I import and run it using this code:

const md = require('./module');

function* dummy () {
    yield Promise.reject(new Error('error1'));
}

md.runGen(dummy)
.catch(err => {
    console.log(9, 'Finished!');
})

I get this logged to the console:

0 'Ready for next iteration'
1 'Ready for next iteration'
2 'Promise yielded out'
3 'Rejected promise handled'
4 'Rejected promise instance of Error'
5 'Ready to handle promise value'
6 'Value was instance of Error'
7 'Throw Error'
8 'Error propagated back out'
9 'Finished!'
(node:9904) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 2): Error: error1
(node:9904) DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

This is all as expected except for the warning about UnhandledPromiseRejectionWarning. I am confused about why I receive this warning as the rejected Promise is handled in the code as far as I can tell.

What am I overlooking?

rabbitco
  • 2,790
  • 3
  • 16
  • 38

1 Answers1

3

What am I overlooking?

Your yieldedOutPromise.value.then call is creating a new promise, and if yieldedOutPromise.value rejects then it will be rejected as well. It doesn't matter that you handle the error via .catch on yieldedOutPromise.value, there's still a rejected promise around, and it's the one that will be reported.

You are basically splitting your promise chain, which leads to each end needing an error handler. However, you shouldn't be splitting anything at all. Instead of the

promise.then(onSuccess);
promise.catch(onError);

antipattern you should use

promise.then(onSuccess, onError).…

Oh, and while you're at it, avoid the Promise constructor antipattern. Just do

module.exports = function runGen (generatorFunc, startValue) {
    return Promise.resolve(startValue).then(generatorFunc).then(generator => {
        return keepIterating({done: false, value: undefined});
        function keepIterating({done, value}) {
            if (done) return value;
            return Promise.resolve(value).then(fulfilledValue =>
                generator.next(fulfilledValue)
            , err =>
                generator.throw(err)
            ).then(keepIterating);
        }
    });
};
Community
  • 1
  • 1
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • Your proposed solution does not return expected result. – guest271314 Dec 05 '16 at 00:00
  • @guest271314 What do you mean? What is the "expected result", the output log the OP was getting? What did it return instead for you? – Bergi Dec 05 '16 at 00:02
  • Your approach does not reach `console.log('never gets here');` – guest271314 Dec 05 '16 at 00:05
  • @guest271314 Of course it doesn't - it's not desirable to get there when you only `yield Promiser.reject('error1')`?! The OPs code doesn't get there either, and as he says that's expected. – Bergi Dec 05 '16 at 00:07
  • OP appears to be trying to reach `console.log('never gets here')` following `"Finished!"`. At least that is how interpreted expected result here. – guest271314 Dec 05 '16 at 00:08
  • @guest271314 No, please read the question again. Focus on "*This is all as expected except for …*". – Bergi Dec 05 '16 at 00:09
  • Yes, read that portion. Also noted that `.then()` where `console.log('never gets here')` is included at `javascript` at Question, and is called if chained to `.catch()`. There is no purpose for including that `.then()` if not intended to be called. In any event, one of the answers should provide a solution to Question. – guest271314 Dec 05 '16 at 00:12
  • @guest271314 Maybe the reason why your answer got a downvote is that it *shouldn't* be called… – Bergi Dec 05 '16 at 00:16
  • @guest271314 The purpose of including the success handler is to handle the case when the generator in normal programs `yield`s something that is not a rejected promise… which just doesn't happen in the minimal example necessary to reproduce the unhandled rejection warning. – Bergi Dec 05 '16 at 00:17
  • If that is case, chaining `.catch()` to `.then()` and using `javascript` at Question returns expected result. The `yieldedOutPromise.value.then()`, `yieldedOutPromise.value.catch()`, unchained, is issue, as you alluded to. fwiw, updated post to include both options. – guest271314 Dec 05 '16 at 00:28
  • 1
    @guest271314 [Except that's not the same](http://stackoverflow.com/a/24663315/1048572). Chaining `value.then(…).catch(…)` is not what we want here. – Bergi Dec 05 '16 at 00:32
  • What is difference between `.then(onFulfilled, onRejected)` and `.then().catch()`? Appears to return expected result at second stacksnippets. – guest271314 Dec 05 '16 at 00:51
  • 1
    @guest271314 As the link explains, you need to consider the case that `onFulfilled` throws. Admittedly, that doesn't happen in your code, but only because you still employ the promise constructor antipattern. – Bergi Dec 05 '16 at 00:54
  • @Bergi The [Promise anti patterns](https://github.com/petkaantonov/bluebird/wiki/Promise-anti-patterns) post by Gorgi Kosev that the answer about anti-patterns that you linked to is based on, actually lists the `promise.then(onSuccess, onError)` that you're recommending as [**the second anti-pattern**](https://github.com/petkaantonov/bluebird/wiki/Promise-anti-patterns#the-thensuccess-fail-anti-pattern). I'm not saying I necessarily agree with it or not but I wanted to let you know that the original sources of your references actually contradict your own recommendations in this answer. – rsp Dec 05 '16 at 10:12
  • @rsp Have a look at [this answer](http://stackoverflow.com/a/24663315/1048572) where I explained when `promise.then(onSuccess, onError)` is not an antipattern. You just have to be aware of the difference, and most people will want to use `.then(dosomething).catch(handleAllErrors)`, but not here. – Bergi Dec 05 '16 at 15:03
  • @Bergi: This might be contrary to Stackoverflow protocol but I spent hours trying to crack this problem and when finally getting the solution I just feel like in s good old-fashioned way to express my gratitude: thank you! I have hard time agreeing though that I am using the anti-pattern. `resolve` and `reject` is passed around because the `runGen` will settle different places dependent on the content of the `Generator` passed to it. – rabbitco Dec 07 '16 at 19:29
  • @mlntdrv [`promise.then(onSuccess, onError)` is not an antipattern](http://stackoverflow.com/a/24663315/1048572) – Bergi Nov 10 '21 at 13:01