promises-aplus / unhandled-rejections-spec

Discussion and drafts of possible ways to deal with unhandled rejections, across implementations
6 stars 0 forks source link

Environment hooks #2

Open domenic opened 11 years ago

domenic commented 11 years ago

This idea involves establishing a set of standard "environment hooks" which, if present, are called upon an unhandled rejection and upon it being handled later.

The essential idea would be something like two globally-known methods, e.g. console.unhandledRejection, console.rejectionHandled, which implementations call upon those two events---if they exist (and are callable). Presumably the former would give out a handle which the latter then accepts.

By itself this does nothing, but if people build tools that tap into these (e.g. browser extensions), they could provide a very nice debugging experience.

ForbesLindesay commented 11 years ago

I think this is my favourite of the options presented thus far. Although it seems a shame to introduce a 'global', I'm not convinced it's truly avoidable. I think the Library hooks option would leave us having to maintain a list of compliant promises libraries.

briancavalier commented 11 years ago

This feels like the right path to me, even if it involves a global. As I've said before, I'm not a fan of done (I'll elaborate more in #5). Having a debug/monitor API that promise implementations can use puts more of the burden on promise and promise-related tool implementers than it does on application developers.

It also allows promise-implementation-agnostic tooling to be created. So, if your app mixes when, Q, and dojo (possibly because you are using other libs that depend on them), and each of those supports the API, your favorite debugging tool will work with all the promises in the app. The tool and its output will be familiar (tho possibly not identical) to you no matter which promises are being reported as unhandled.

Of course, it means that tools need to be created :)

Although, I could see promise implementations providing their own simple "debug mode" that makes use of these same APIs when no other debugging tool is present.

novemberborn commented 11 years ago

Dojo already implements something akin to this, though in a somewhat broken form that was introduced too quickly.

In other words, I'm in favor.

novemberborn commented 11 years ago

Obviously I should read all tickets before responding… Dojo 1.8 has what #3 proposes, but I'd prefer environment hooks to enable generic tooling. Conversely, if the hooks in #3 are consistent then the tooling could just bind to whatever implementation it may find in the environment.

domenic commented 11 years ago

I just realized we should probably include #3 as well, since if multiple promise libraries are active on a page, it might be useful to distinguish where they came from.

ForbesLindesay commented 11 years ago

Could just use:

console.unhandledRejection(rejection, 'Q');
console.unhandledRejection(rejection, 'when.js');
Raynos commented 11 years ago

What does this do that throw Error and process.uncaughtException / window.onerror don't already do for you?

domenic commented 11 years ago

@Raynos read the background thread in #1 and also note point 3.2.6.2 of the spec.

Raynos commented 11 years ago

@domenic it's not obvouis what 3.2.6.2 is?

domenic commented 11 years ago

@Raynos sorry, the numbers are in the gh-pages version

Raynos commented 11 years ago

@domenic we both know those points should be linkable ;)

Raynos commented 11 years ago

There is a general issue of an eventual value is an error. Either it's never handled and the environment never sees it or it as some point marked as "error should have been handled" and then throws.

For example with gozala/reducers a Reducible is a representation of a list over time on which you can do lazy transformations and consuming transformations. For it's version 3 implementation the rules are if you fold (consume) a reducible containing an error and you have no error handler then the implementation throws and the environments uncaughtException handler is invoked if it exists.

However if you just wrap or transform the reducible then it will be a no-op or an empty pass through if it's an error.

Which means you solve the issue by having two sets of operations, one which allow errors to be handled later at any time and another which will throw the error if it's not handled. Of course this would still cause the same problem if you never use the latter type of operation (the consuming operation which throws errors). That issue is solved by having the wrapping operation be lazy. i.e. if it's never consumed it does nothing, forcing someone (the last person that write the eventual value somewhere) to consume it.

Under the above approach I see no need for environment hooks.

domenic commented 11 years ago

Which means you solve the issue by having two sets of operations, one which allow errors to be handled later at any time and another which will throw the error if it's not handled.

This is exactly the done idea of #5. The then operation allows errors to be handled later; the done operation doesn't.

Of course this would still cause the same problem if you never use the latter type of operation (the consuming operation which throws errors).

This is exactly why we are trying to find other solutions, such as this one of environment hooks.

That issue is solved by having the wrapping operation be lazy. i.e. if it's never consumed it does nothing, forcing someone (the last person that write the eventual value somewhere) to consume it.

I'm not sure I understand this approach, so feel free to explain it in more detail. But I think the problem is async-ness. How do you know an error will never be consumed, in an async world?

Raynos commented 11 years ago

@domenic let's imagine that an Eventual representation is backed by a asynchronous source.

And unless you consume (.done()) the eventual the asynchronous source never "starts reading"

This allows you to lazily wrap an Eventual and return a new Eventual that when consumed will consume the source and apply some kind of transformation.

You avoid the issue basically by saying "the asynchronous operation only starts when you call .done()"

Raynos commented 11 years ago

What I'm saying is basically that .then() should be lazy and that if someone returns a promise they should not do their asynchronous action until someone signals them to start by calling .done() on the promise.

So when you create a promise you should do something like:

var p = Promise(function (promise) {
  // do async thing
  // function only called if someone calls p.done()

  return promise
})
ForbesLindesay commented 11 years ago

That's a really really interesting idea and ties in nicely with https://github.com/promises-aplus/resolvers-spec/issues/8 (Hot and Cold Promises). It would be a pretty radical change, and would promote the use of done to the same level as then but it would ensure that errors were never silenced. Personally I'm +1 for that idea.

ForbesLindesay commented 11 years ago

We can continue discussing that idea in #8 as it does not really relate to Environment hooks.

briancavalier commented 11 years ago

The concept is interesting, indeed. I'm not caught up yet on the Hot and Cold promises thread, but my initial reaction is that lazy promises are something that could easily be built on top of a current Promises/A+ promise. It feels weird to me initially to have this integrated this directly into Promises/A+ promises as they exist today.

domenic commented 11 years ago

Everyone likes the environment hooks idea. I think we should standardize on it, but before we do, I think we should write a quick Chrome extension to show that it works.

I admit that this has been on my weekend task list for a couple months now, without having been accomplished, so, anyone else have more free time to prototype such things? :)

novemberborn commented 11 years ago

Haven't prototyped a Chrome extension, but did extract some debugging utilities from Legendary into its own package: https://github.com/novemberborn/legendary-debug.

Internally Legendary's Promises will call legendary.unhandledRejection whenever it rejects a promise. This method is a no-op normally, but with legendary-debug it returns a method that should be called when the rejection is handled. See https://github.com/novemberborn/legendary-debug/blob/master/lib/unhandled.js for the unhandledRejection implementation, and https://github.com/novemberborn/legendary-debug/blob/master/index.js#L58 for an example of how this can be used to log to the console when unhandled rejections occur.

ricardobeat commented 11 years ago

Has an unhandledRejection emitter been implement in when or any other library?

novemberborn commented 11 years ago

Legendary has something, see https://github.com/novemberborn/legendary/blob/master/lib/legendary.js and https://github.com/novemberborn/legendary-debug. I haven't given it much love though.

briancavalier commented 11 years ago

@ricardobeat We're currently prototyping support for an unhandledRejection-like API. It's pretty rough, right now, so be gentle :) The prototype relies on APIs added to console (as suggested in this issue), but they could easily reside somewhere else. Normally, I'm anti-global, but the ease of being able to rendezvous with any available "promise debugger" that publishes the API is kinda tempting.

briancavalier commented 11 years ago

@ricardobeat when.js >= 2.2.0 has unhandled rejection monitoring without using done.

I don't think the API is perfect, but I think it could be a good basis for discussion here. Basically it relies on a single PromiseStatus constructor that a promise can be used to report its state. It doesn't matter how a library acquires a reference to the PromiseStatus constructor (i.e. it's allowed to be library specific: sniff for global, library property, library config method, etc.).

I'll make some time to do a quick writeup on the PromiseStatus API to see if it's something that might be worth adopting here, even if in some tweaked/improved form.

domenic commented 11 years ago

@briancavalier looking forward to the PromiseStatus API. After doing some tentative work on my own unhandled rejection monitor with @stefanpenner, we realized console wasn't such a good place to put the APIs because in IE9 and below that object doesn't exist until the developer tools open -_-.

briancavalier commented 11 years ago

re: console in IE < 9. Yeah, it's a pain. Right now, when carefully sniffs for console.PromiseStatus and falls back to when.PromiseStatus.

Here's a quick and dirty dump of the PromiseStatus API:

// Sorry for mixing JS-like and IDL, it's just easier
function PromiseStatus(); // constructor

interface PromiseStatus {
    PromiseStatus observed();
    void fulfilled(optional any value);
    void rejected(optional any reason);
}

A few interesting things:

  1. The intention is that each promise is given its own PromiseStatus instance, and the promise should call the appropriate PromiseStatus method when its state changes.
  2. A promise need not expose any information, except it's status, to the API. For example, it never passes itself to any PromiseStatus method, and may elect not to provide it's fulfillment value and/or rejection reason.
  3. observed() returns a new PromiseStatus instance, which parallels then returning a new promise. The intention here is that promise.then will call observed and hand the returned PromiseStatus to the new promise it intends to return. So, the PromiseStatus API can be used to build a graph that mimics the actual promise graph (if the particular PromiseStatus implementation wants to do that--when.js's monitor only builds a partial graph)
  4. Given these three methods, it's been possible in when.js to detect unhandled rejections and to detect when those rejections later become handled. For example: An unobserved+rejected promise can be considered unhandled (when it is deemed worthy to report to the developer is a different matter), and a previously unobserved+rejected promise which becomes observed can be considered handled.

when.js's particular implementation is here (slightly cleaned up in the dev branch as opposed to master)