rsvp.js

rsvp.js copied to clipboard

Hey, I've written the following proposal here: https://gist.github.com/benjamingr/0237932cee84712951a2

I'd love your feedback.

it is exposed. RSVP.on('error'

@stefanpenner right, and I'm asking you expose it on "process" in addition like the proposal says, Please consider reading it :p

Sounds good. I'll add it to rsvp and es6promise this weekend

Awesome - thanks a ton :)

thanks for investing in the promise community :)

please, don't emit this event on process object. It could end up conflicting with native node.js/io.js implementation or other libs. If the purpose is to share emitter between dupes, the event should be namespaced somehow (e.g. process.on('rsvp:possiblyUnhandledRejection'))

@benjamingr thoughts?

@stefanpenner

The whole idea here is to is to fire the same events as native promises and with other libraries so you get a global place to handle every promise error from any library (or native promises) in one place - please see the design document (at the start of the issue) on what problems it solves.

What @vkurchatkin is describing is why we want to do this.

@benjamingr the idea is good, but there is a reason why developing a standard takes time. There is no consensus on how this should be done.

@vkurchatkin I believe there is, and several libraries have already implemented this. This standard is just a minimal first stage as a preparation for more interesting standards and ideas which is why all it takes is exposing two events on process and nothing more.

If you have any interesting feedback I would greatly value it here: https://gist.github.com/benjamingr/0237932cee84712951a2

You can also find me regularly in the Stack Overflow JavaScript chatroom and I tend to frequent #promises too. You can also contact me by email if you'd prefer (benji at tipranks) (dot com)

@benjamingr i believe the best thing todo is to namespace the event for all implementations. This way it wont conflict with any current or future built-ins.

After-which we can bug standards/language folks to consider this proposal.

something like: process.on('promise:possiblyUnhandledRejection')

@stefanpenner there are no standards/language folks. This is just NodeJS/io.js and the libraries. The name "possiblyUnhandledRejection" doesn't make much sense outside of promises anyway so prefixing it doesn't make sense IMO.

On the other hand if the need ever arises you can always give it a different name - "possiblyUnhandledRejection" as a name was chosen just because it made sense - there is no obligation to support it in the future but hopefully it will be used and people will find it useful.

The idea is again - to create a global point all promise libraries and native promises report unhandled rejections to so the user can choose what to do on an unhandled rejection (report it, restart the server, special handling, ignore it or whatever they want).

@stefanpenner note the events have been renamed to unhandledRejection and unhandledRejectionHandled as Domenic and Petka suggested in various threads and as when and bluebird implement.

Where did I suggest that? I liked possiblyUnhandledRejection... no big deal though.

@Domenic To be fair - Petka told me that you suggested these names in a spec and encourages implementing them with this name in bluebird as real events. If you care I can ask him where (cc @petkaantonov) - apologies if I misrepresented your opinion.

@domenic In the original email you suggested unhandledrejection and rejectionhandled and the camelCased versions as per node convention are unhandledRejection and rejectionHandled. I went with unhandledRejection because it is in better symmetry with rejectionHandled.

@petkaantonov I suggested error and rejectionhandled actually, then at the bottom had

We may be better off with a separate unhandledrejection event (or, more accurately and as popular libraries call it, possiblyunhandledrejection)

I just want to mark the time and date - @domenic said something nice about bluebird :D (that it's popular)

@stefanpenner this is now implemented in io.js in the 1.4 release. Please consider adding it to RSVP.

@stefanpenner this is now implemented in io.js in the 1.4 release. Please consider adding it to RSVP.

yup I believe I am on board. Unfortunately pretty busy now, will likely look into it further post ember-conf (which is next week)

Q now has this, you're pretty much the last ones left.

Ping @stefanpenner

So, it's been like half a year and native node does this. Would be nice to finally have it in.

Pr welcome? :D

Sure.

Basically

    if (typeof process === "object" && typeof process.emit === "function") {
        try { 
             process.emit("unhandledRejection", promise._result, promise);
        } catch (e) { /* don't throw for rejection hooks failing in hostile environment */ }
   }

In https://github.com/tildeio/rsvp.js/blob/7684756417cb69ce91033c7951fcf5ba92f879aa/lib/rsvp/promise.js#L179

Yup exactly

This just cost me like 4 hours stef :(

@benjamingr why didn't you use RSVP.on('error'? emitting events from objects you don't own is generally a bad idea

Because I worked on a code base I did not author that used es6-promise - I just assumed implementations don't swallow exceptions anymore :/