Documents API changes in #256

[benjamingr]

Documents the new unhandled rejection detection API.

Awesome work on that PR everyone!

#256 (comment)

[vkurchatkin]

rejectionHandled?

[benjamingr]

@vkurchatkin Yes, I wrote this before the name changed - nice catch

[domenic]

I disagree with the framing here and think it is biased. I can comment more when back at my computer (which will be later tonight).

[benjamingr]

@domenic biased against what? I tried to keep it as simple as possible and using the least promise terminology. Is your issue with the terminology or are you looking for a more 'pro-promise' sentiment?

[petkaantonov]

I think it should simply cover how every philosophy is implemented and their pros and cons in an unbiased way, for example not saying that something is rare but saying that in some programming styles this will result in false positives.

[petkaantonov]

There doesn't seem to be a way to document either event in isolation without being biased actually.

Any usage example of unhandledRejection event in isolation is biased towards instant logging/throwing philosophy, because what else can you do in there if you use that event alone?

And rejectionHandled is only needed if you buy into the growing and shrinking list of rejections philosophy so documenting its usage is going to be biased towards that obviously..

[benjamingr]

@petkaantonov which is why this PR after the update due to @domenic 's comment shows both patterns. The unhandledRejection event is demonstrated in isolation and then in the rejectionHandled event the shrinking list idea is explained (paraphrasing @domenic's own words) and demonstrated. I think the current balance is ok - anything you'd change?

[petkaantonov]

Your update doesn't include any example usage of the 'unhandledRejection' event, it just says that an event will be fired but you don't show example code handling it.

[benjamingr]

@petkaantonov from the doc:

---

Here is an example that logs every unhandled rejection to the console:

process.on('unhandledRejection', function(reason, p){
    console.log("Possibly Unhandled Rejection at: Promise ", p, " reason: ", reason);
    // application specific logging, throwing an error, or other logic here
});

[domenic]

I think "rejection handler" would be better than "error handler" but nbd.

[benjamingr]

@domenic I think rejection handler is more concise promise terminology but I chose error handler since I found it more approachable for people without a background with promises - if you feel strongly about this I can change it though.

[domenic]

OK. I've left extensive comments on ways to improve this, because I am passionate about this subject getting good docs treatment. HOWEVER, @benjamingr's edits since my original comment have addressed the bias I perceived, and so I think if there's a release imminent it's fine to merge this as-is without necessarily implementing my changes. We should do them eventually though since they're good changes :)

[sam-github]

"will trigger" not "will be trigger"

[sam-github]

Events in markdown should use single quotes, for example 'rejectionHandled'.

The example uses an odd mix of double and single quotes for strings, I'd suggest single quotes everywhere.

Markdown should be wrapped, like all other text lines in io.js.

[benjamingr]

Thanks @domenic and @sam-github I've updated the PR with all the advice and the formatting issues included except for the FileHandle example since I've found it a bit controversial (the API format itself) and I think we can come up with a more concise one. I've pretty much merged all the proposed changes by domenic (good terminology remarks) - the only thing I didn't change there explicitly was "error handler" to "rejection handler" because I find the terminology more approachable to people who are new to promises.

[benjamingr]

Rebased.

[vkurchatkin]

Maybe it is better to say: after after 'unhandledRejection' was emitted?

[benjamingr]

@vkurchatkin I considered it but figured it's better to phrase it containing when it emits as a whole (after a promise was rejected and no handler was attached in time). I wanted to add a 'this happens when an "unhanldedRejection" event was emitted' but figured that it's better to give an example of using them together.

[Fishrock123]

I agree with @vkurchatkin

Emitted whenever a Promise was rejected and an error handler was attached to it later than after an event loop turn.

Sounds like "emitted when the 'unhandledRejection' has a listener added".

[benjamingr]

@Fishrock123 that's what I'm trying to avoid - you don't have to add a listener to the "unhandledRejection" event in order for this event to fire. You might be interested in using this event for purposes that are not related to that other one. Even if you are - I think the semantics of this event should stand up for themselves.

Maybe adding it as an extra? How would you phrase it?

[Fishrock123]

Maybe more like:

"Emitted whenever a Promise was previously rejected and an error handler (such as .catch()) was attached to it"

[rvagg]

OK, I want to get a release out today and this is going to get merged to make that happen. If anybody disagrees with the language please suggest changes, otherwise perhaps reserve them for a future PR.