6

I am starting with E6 Promises. I like them very much, but there is a crucial concept around error handling that I don't understand and would love some clarification on.

Let's assume the following simple function that returns a promise:

    function promiseString(str, timeout, doResolve) {
        return new Promise((resolve, reject) => {
            setTimeout(() => {
                if (doResolve) {
                    resolve(str);
                } else {
                    reject(new Error("Rejecting " + str));
                }
            }, timeout);
        });
    }

It is pretty straightforward, just returns a promise for the string that was passed to it, and causes that promise to be resolved or rejected (based on the third argument) in "timeout" milliseconds.

I can consume this completely as expected as follows:

            promiseString("One", 100, true)
                .then((str) => { console.log("First then is " + str); return promiseString(str + " two", 100, true); })
                .then((str) => { console.log("Second then is " + str); return promiseString(str + " three", 100, true); })
                .then((str) => console.log(str))
                .catch((err) => console.error(err));

If alter the third argument to from "true" to "false" in any of the calls in this chain, my error is caught as expected and send to console.error().

However, now imagine the following (similarly silly) function for constructing a promising object:

    function DoublePromiser(str1, str2, doResolve) {
        this.promise = new Promise((resolve, reject) => {
            promiseString(str1, 100, doResolve)
                .then((s1) => promiseString(s1 + str2, 100, doResolve))
                .then((s2) => resolve(s2));
        });
    }

Imagine now that I consume this code as follows, with everything resolving and nothing rejecting, (doResolve is set to true):

            var dp = new DoublePromiser("Big", "Promise", true);
            dp.promise
                .then((s) => console.log("DoublePromise: " + s))
                .catch((err)=>console.log("I did catch: ", err.message));

As would be expected, I see the following in the console:

DoublePromise: BigPromise

However, now I alter the consuming code, setting doResolve to "false" (which causes my promise routine to reject):

            var dp = new DoublePromiser("Big", "Promise", false);
            dp.promise
                .then((s) => console.log("DoublePromise: " + s))
                .catch((err)=>console.log("I did catch: ", err.message));

Because of my understanding of how errors should "bubble up", I would expect the console to log as follows:

I did catch: Rejecting Big

But it does not. Instead, the console shows an uncaught error:

Uncaught (in promise) Error: Rejecting Big

I only get what I expect (and desire) if I add a catch to the end of the chain in the DoublePromiser, like this:

    function DoublePromiser(str1, str2, doResolve) {
        this.promise = new Promise((resolve, reject) => {
            promiseString(str1, 100, doResolve)
                .then((s1) => promiseString(s1 + str2, 100, doResolve))
                .then((s2) => resolve(s2))
                .catch((err) => reject(err)); // ADDING THIS TO MAKE IT WORK
        });
    }

Now I get what I expect, the error is not uncaught. But this seems counter to the whole idea that errors bubble up, and it seems weird to catch an error just to re-reject the same error.

Am I missing a way of getting this to work simply?

Am I missing some fundamental concept?

jfriend00
  • 683,504
  • 96
  • 985
  • 979
Stephan G
  • 3,289
  • 4
  • 30
  • 49
  • 1
    These 100% theoretical questions using made up code that don't state a real problem just don't work very well on stack overflow. – jfriend00 Jul 28 '16 at 00:37
  • Also, you are using the promise constructor anti-pattern: https://github.com/petkaantonov/bluebird/wiki/Promise-anti-patterns – jfriend00 Jul 28 '16 at 00:38
  • @estus - I edited their title. – jfriend00 Jul 28 '16 at 00:50
  • @jfriend00 Thanks. Nice answer btw, it nails this down. – Estus Flask Jul 28 '16 at 00:53
  • Reopening because, though that other answer contains info that can be used to answer this question, they are not even close to the same actual question. Nobody who read that other title or question would think that is the same question as this question. – jfriend00 Jul 28 '16 at 02:35

1 Answers1

10

You are using a promise constructor anti-pattern. Don't wrap an existing promise in another promise you make yourself as that just makes you do lots of extra work to make things work properly and since most people don't do that extra work properly, it's also very prone to programming mistakes. Just return the promise you already have.

Change this:

function DoublePromiser(str1, str2, doResolve) {
    this.promise = new Promise((resolve, reject) => {
        promiseString(str1, 100, doResolve)
            .then((s1) => promiseString(s1 + str2, 100, doResolve))
            .then((s2) => resolve(s2))
            .catch((err) => reject(err)); // ADDING THIS TO MAKE IT WORK
    });
}

to this:

function DoublePromiser(str1, str2, doResolve) {
    return promiseString(str1, 100, doResolve)
       .then((s1) => promiseString(s1 + str2, 100, doResolve));
}

And, then just use it as a function:

DoublePromiser("Big", "Promise", false).then(...);

Recap: You nearly always want to return inner promises from within .then() handlers because this allows nested errors to propagate upwards and also properly chains/sequences async operations.

And, you want to avoid wrapping new promises around existing promises nearly always because you can just chain to and/or return the existing promise you already have.

Also, be aware that if you do a .catch(), that will "handle" a rejected promise and return a new non-rejected promise, continuing the promise chain from there unless inside the .catch() handler you return a rejected promise or throw an exception. So, this:

p.catch((err) => console.log(err)).then(() => console.log("chain continues"))

will happily do both console.log() statements because the .catch() "handled" the promise so the promise chain happily continues.


As I said in my earlier comments, these 100% theoretical discussions are hard to get to exactly what you really want to accomplish (we have to guess what the real problem is) without a 20 page tutorial on how promises work that covers a lot of stuff. It's a lot better if you post a real-world problem you're trying to solve with this technique and we can show/explain the best way to do that in a few lines of code and a few paragraphs of explanation.

jfriend00
  • 683,504
  • 96
  • 985
  • 979
  • Well thanks for your work on this. I'll be working through your answer tomorrow. The "real world" version of this is so large and squirrely it is hard to imagine describing succinctly for StackOverflow. It has to do with creating a caching system using IndexedDb for a SAAS system, but a pretty complicated caching system. I spent quite some time boiling down the "real world" problem to this "fake world" excerpt. Our current design does require that we create an object through a constructor, but I'll try to use your concepts not to next promises and see where I get. – Stephan G Jul 28 '16 at 03:34
  • 1
    And.... THIS is why I love StackOverflow. After reading many spec documents and tutorials and getting this "mostly" right, the penny finally dropped on the "anti-pattern" and nesting. And it works fine to do the same thing in a constructor. Perfect. Thank you. – Stephan G Jul 28 '16 at 04:30
  • @StephanGolux - Great. The issue with a constructor is that it can't return a promise. But, you can do what you were doing which is stick the promise in instance data and reach in to get it after calling the constructor. Though that seems odd to me, but perhaps because you don't disclose any other purpose for have it be an object/constructor. – jfriend00 Jul 28 '16 at 04:33