Error Stacktrace with Source Maps

[nicib83]

if i get an error within a promise this usually means:

• the stacktrace is wrapped within unhandled rejection and when.js calls
• Source maps don't work with browserify builds as a whole being displayed
• at least the error is shown, native promises (chrome) don't even log the error

this makes debugging not that easy, particularly when your whole bootstrapped code is somehow wrapped with a promise

however when i enwrap the promise callback with a setTimeout it works just fine, with full source map support.

new Promise(function(resolve) {
     throw new Error();     
});

vs.

new Promise(function(resolve) {
    setTimeout(function() {
        throw new Error();      
    })
});

[briancavalier]

Hey @nicib83, this is a very interesting issue, thanks for posting it!

Right now, we do our own formatting and logging of potentially unhandled errors for a few reasons. For example, so we can distinguish them visually from other synchronous errors, add an id so they can be "unlogged" later if they actually get handled (which may be much later due to the temporal nature of promises).

When you introduce the setTimeout, that moves the throw out of the current stack to a point where when.js cannot intercept it. Thus, it's being handled directly by the browser (ie window.error), and it is applying the source map transformation before logging the Error's stack. Unfortunately, using a setTimeout throw isn't a universal option for when.js in this case because it will outright crash Node.js, which is a bit extreme for an error that might actually get handled in the very next tick.

So, I wonder if we could check for window and then send the raw error object directly to window.error. That might work--it's certainly worth a try. We might still be able to include the "Potentially unhandled" tag and the id.

Any thoughts on that?

[nicib83]

I understand that due to the way promises can be used it may be better to encapsulate functionality and error handling.

I tried to experiment in the core codebase. If you skip the try and catch block all together the error gets logged like it isn't within a promise. Putting console.log(e.stack) into the catch outputs the promise Stacktrace.

        try {
            resolver(promiseResolve, promiseReject, promiseNotify);
        } catch (e) {
            promiseReject(e);
        }

Maybe an alternative debugging respectively development routine is necessary for this.

[briancavalier]

@nicib83 I think the best approach is to find the right way to log the error such that the browser applies its sourcemap transform (since only the browser's internal machinery can do that, unless we get crazy and load/interpret sourcemaps ourselves in user-land). Changing the way promises handle errors would not be good since it could actually change the runtime characteristics of the application. IOW, we want promises and the application to behave exactly the same, but the error output to be sourcemap-transformed.

So far, the only way I've found that causes the browser to apply the sourcemap transform is to force the error to hit window.error by doing a setTimeout throw. That might be a good option, but there at least a couple of tricks to work out:

1. We can't do that for Node, since it will crash the process. (and we don't need to anyway, since node doesn't need sourcemaps). It should be enough just to detect typeof window !== 'undefined' for this.
2. Using setTimeout introduces another async step in error reporting, which means we must account for it when unreporting rejections that become handled after being reported.

Like you said, having a flag to indicate an alternate debugging mode might be a good way to go. I'd like to try to make it work automatically, so it's worth doing a bit more prototyping to see if we can do better.

If you want to hack on it, the relevant code is in lib/decorators/unhandledRejection. Maybe we can compare notes and come with a good solution.

[briancavalier]

@nicib83 Here's a gist with a simple function that will get you minimal source map support. It overrides the builtin handling to force a next tick throw.

This should help your browserify case in the short term.

[nicib83]

@briancavalier since this doesn't modify the core and its possible to just include it in a dev environment its probably the most reasonable approach.

i agree one should not change the way promises handle errors. i think the issue seems to be in general that promises are being used as control flow structures of javascript but behave like libraries.

[rafaelchiti]

Hey!, thanks for the gist is very useful. Wondering what about the onFatalRejection?.

Lets say:

promise.done( () => somethingUndefined );

That for me logs an exception wihtout any stack trace just pointing to the file where it got thrown (lib/decorators/unhandledRejection)

I've patched that one too, but haven't had the chance to dig into the internals of when.js so I'm wondering if thats alright and my main question is if we get any side effect or issues by patching the whole unhandledRejection helper like you pointed out int he gist.

[danschumann]

I was about to ditch when, until I found this. TBH, this should be a much more televised fix, since many many people care very very much about meaningful stack traces.