Exceptions silently absorbed in `thennable` processing.

[l-cornelius-dol]

I have posted in detail on StackOverflow (http://stackoverflow.com/q/26007475/8946), but the short story is that when I make a typo in promise chain code a ReferenceError is thrown, but Mithril silently absorbs it at line 719 (for version 0.1.21) in this code:

function fire() {
    ...    
    thennable(ref, function () {
        state = 1
        fire()
    }, function () {
        state = 2
        fire()
    }, function () {
        try {
            if (state == 1 && typeof fn == 'function') {
                val = fn(val)
            }

            else if (state == 2 && typeof er == 'function') {
                val = er(val)
                state = 1
            }
        } catch (e) {
            /**/console.log("[**] Caught in fire.thennable: %o",e);
            val = e
            return finish()
        }
    ...
}

An example of the mistake in my code is:

function getListData(ctl) {
    ctl.data([]);
    ctl.loading(true);
    return m.request({ method: "GET", url: apiBase, background: true }).then(done,fail);

    function done(rspdta) {
        xctl.loading(false);
        ctl.data(rspdta.Customer);
        m.redraw();
        };

    function fail(rspdta) {
        ctl.loading(false);
        ajaxError(ctl);
        m.redraw();
        throw rspdta;                                                                               // continue error condition
        };
    }

Notice the deliberate xctl.loading(false) in the done function -- the script just appears to stop there, but a ReferenceError is thrown. However, nothing is logged.

[Zolmeister]

This is expected behavior, repost of my answer on stackoverflow:

---

Errors thrown inside an onFulfilled handler result in a rejected promise. Spec: http://promisesaplus.com/#point-42

Here is the solution:

    abc.then(thrower, fail1).then(null, fail2)

It will be caught by fail2

[l-cornelius-dol]

I strenuously disagree with this "solution", at least for the JavaScript engine errors, EvalError, InternalError, RangeError, ReferenceError, SyntaxError, TypeError, URIError.

At the very, very least, InternalError, RangeError, ReferenceError, SyntaxError, and TypeError should percolate and not be injected into the promise chain.

I should not have to anticipate runtime errors and explicitly log them; if I was good enough to reliably do that, then I wouldn't make these errors to begin with.

Alternately, if the promise chain ends with an exception and no handler, it should be rethrown. Please reopen this ticket for Leo to comment on.

[Raynos]

@lawrence-dol you strenuously disagree with how promises work. This is how promises work, all thrown errors get cast into rejected promises.

The solution is to not use promises. I suggest you implement an alternative m.request or make a pull request on mithril to make promises optional in m.request. This is one of the advantages of a simple callback based approach.

[Zolmeister]

This has nothing to do with the type of error, it has to do with where it is thrown from. And more importantly, it is part of the Promises/A+ spec.

There is no way to tell if a rejected promise will not be dealt with later, though ES6 Promises let you re-throw if they are garbage collected.

[l-cornelius-dol]

Yes, but still, what happens if the promise chain ends with an exception? Currently it appears to just silently swallow it. Why should it not rethrow in that case?

[Zolmeister]

@lawrence-dol You can't, it's not possible

[Raynos]

@lawrence-dol this is an issue for the Promises/A+ spec.

Short answers is that promises are eager.

What if you do

var p = m.request({ ... });

setTimeout(function () {
  p.then(null, handleError);
}, 0);

You never know who might read your value in the future so you never know when to rethrow in the "my error never got handled" edgecase.

This is why @Zolmeister mentioned that ES6 promises might rethrow when the promise gets garbage collected, that is the only time it's save to throw.

You cannot fix this without making promises lazy, at which point they are no longer promises.

[Raynos]

@lhorie

you've had to deal with a lot of recent issues and complexity because you ship with a promise implementation in mithril.

You might want to consider removing the promises feature at some point and replacing it with a simple function (err, result) {} interface.

By moving the promises feature into userland or into an optional plugin / extension, you will remove an entire range of issues to answer and an entire lump of code that you have to maintain.

It will make mithril simpler.

[jmventura]

+1

[l-cornelius-dol]

There just needs to be some way to add a default "rejected" handler to the promise when it's created, and/or better yet to always log the exception if it's not handled. _Silently_ stopping when an exception occurs is _never_ a useful thing to do.

If I understand Leo correctly, Mithril 0.1.22 will do that.

[chexxor]

I believe Q's "done" method a solution to this problem. If a chain ends with an error, and the promise had "done" applied, it will be thrown as a normal javascript exception. https://github.com/kriskowal/q/wiki/API-Reference On Sep 24, 2014 4:59 PM, "Lawrence Dol" notifications@github.com wrote:

There just needs to be some way to add a default "rejected" handler to the promise when it's created, and/or better yet to always log the exception if it's not handled. Silently stopping when an exception occurs is never a useful thing to do.

If I understand Leo correctly, Mithril 0.1.22 will do that.

— Reply to this email directly or view it on GitHub https://github.com/lhorie/mithril.js/issues/279#issuecomment-56745695.

[l-cornelius-dol]

@Raynos: I take your point about there being no "end" to the promise chain until the promise object is GC'd, making this more complex than I originally intuited.

Even so it seems like (at least some of) the runtime errors I listed should always be logged. There's no intelligent handling of a ReferenceError that was thrown because I mistyped a variable name.

[Raynos]

@lawrence-dol can't be fixed.

Either:

• don't use promises
• never forget to add error handlers to your promises. i.e. always .then(null, handleError)

[lhorie]

@lawrence-dol So yeah, as I wrote in the stack overflow question, Mithril 0.1.22 will ship with an exception monitor (basically the same workaround Ember is using). The docs will be updated in the site to explain it, but there's a preview of it here: https://github.com/lhorie/mithril.js/blob/next/docs/mithril.deferred.md#unchecked-error-handling

I'm going to be releasing it soon (hopefully this week), pending some improvements on the docs re: the mailing list thread about controller nesting confusion

Longer term, once ES6 Promises throw on GC'ed unhandled rejections, I'm planning to move Mithril promises to use them.

@chexxor .done was discussed by the A+ folks and the general feeling was that it's a hack that doesn't really alleviate the issue of forgetting to add to tack something at the end

@Raynos removing promises from Mithril seems a bit like throwing the baby out with the bath water. Error binding is much more straightforward with promises and doing the equivalent of Promise.all, for example (or m.sync, as it's called in Mithril), is a major pain in the butt with callbacks. Maybe it's just me, but I have some Angular code that uses callbacks and with non-trivial async, it gets nasty fast because of callback hell.

[ekanna]

@lhorie @Raynos

you've had to deal with a lot of recent issues and complexity because you ship with a promise implementation in mithril.

You might want to consider removing the promises feature at some point and replacing it with a simple function (err, result) {} interface.

By moving the promises feature into userland or into an optional plugin / extension, you will remove an entire range of issues to answer and an entire lump of code that you have to maintain.

It will make mithril simpler.

+1

I love mithriljs a lot. But when it comes to fetching data from server, i am thinking of using plain ajax instead of m.request.

The problem for me with r.request is it pretends as if it is a synchronous code but in reality it is asynchronous code. In my opinion, synchronous code should look like synchronous and asynchronous code should look like asynchronous.

After all callbacks are not that bad. Entire nodejs on the serverside is built around the concept of callbacks. We may have to refer to thoughts of Isaac, the maintainer of nodejs. http://callbackhell.com/ http://blog.izs.me/post/59142742143/designing-apis-for-asynchrony

These are just my 2 cents.

Thanks

[lhorie]

Hi @ekanna

I think that same link has already been posted a couple of times here before. The gist is of the argument is basically "under the hood async stuff is just callbacks, so why not just use callbacks". The counter argument to that is that promises, etc let you organize code in better ways. The meta conclusion is that it's hard to see the benefits of the abstraction layer unless you've reached a point where you have enough complexity to benefit from these abstractions.

The thing with abstraction arguments is that people often think they're arguing for simplicity when they're really arguing for familiarity.

For example, with callbacks, it looks perfectly reasonable to do this:

request(..., function(data) {
  ctrl.things = data
})

I just got rid of the need for promises AND m.prop by using plain js. But this is not necessarily simpler. Once the code starts growing, it becomes harder and harder to find what things exist in ctrl when and it becomes harder to understand when ctrl.things will be available for other async ctrl members (and vice-versa). Managing code like that is really hard.

It's hard to see why you would need to do relational stuff if you work in a young startup and you always fetch everything by primary key, but once you have an older codebase, relational questions start to show up everywhere and the last thing you want is for every variable in your relational algebra algorithm to be maybe-nullable-depending-on-network-latency.

Promises give you the ability to organize code in a way that makes the availability of data a lot more obvious, easier to understand and refactor. So in that sense, promises are simple because they decrease complexity as code grows, whereas callbacks are only air-quotes "simple" because it's familiar but you haven't been burned by the complexity of code growth yet.

[ekanna]

@lhorie Thanks for the enlightenment. May be you are right. I am a newbie to this concept of promises. I will try to get familiarity by extensively using promises. At this point of time, i will simply follow your advise. Thanks a lot.