Debugging strategy about native-promise-only

[getify]

[Editor's note: I have edited this thread with @benjamingr and moved discussion of asynquence to another thread on that repo.]

That's a good question. IMO, it's a design flaw in promises that requires either:

1. the browser's devtools have to infer unhandled rejection
2. bastardizing promise design with hacks like done(..) to say "I promise I'm done now"

IMO, since promises themselves can't be fixed in this way (ship has already sailed), neither can a faithful polyfill be fixed.

Bluebird promises track unhandled rejection approximately (in a way that practically always works).

I'm not familiar with this. Can you elaborate?

[getify]

Note: there were discussions at one point of some sort of explicit "API" for promise implementations to make themselves observable by devtools. If that ever comes about, I would certainly extend NPO so it automatically did that to whatever extent possible.

[benjamingr]

Bluebird promises track unhandled rejection approximately (in a way that practically always works).

I'm not familiar with this. Can you elaborate? Perhaps it's similar to what I'm about to discuss immediately below?

Bluebird will check when the .catch handler is attached, and if the promise rejects in a reasonable time, and unhandled rejection was observed - it will output an "onPossiblyUnhandledRejection" - adding .catch asynchronously in a distant time in the future is very rare so this catches most (read all) practical uncaught rejections. I've used it on over 500K LoC by one worked on by multiple developers and haven't had a single miss. It even caught an error by an API who threw a primitive (much better than Firefox does).

[benjamingr]

Though I feel like I'm getting slightly off topic here, what do you think about implementing unhandled rejection detection with a rather huge delay (let's say - no handler in 4 seconds) without the ability to override it so the library pretty much does a "less clever" version of native Firefox promises?

[getify]

Though I feel like I'm getting slightly off topic here

Actually, I think this message is entirely on topic. Your question is if I feel it's appropriate to add such a feature to what is otherwise a bare-bones polyfill only.

The answer is: I don't think it's appropriate. The reason: the behavior you define from Bluebird, and indeed even the behavior you talk about with Firefox + native promises, is not standardized. I don't think this polyfill should deviate significantly from the spec, especially heavily into evolving territory. If there's ever some firm specification for handling it, this polyfill will of course do its best to conform.

I still believe that the issue of solving uncatchable errors is something that the spec (and polyfills) can't fix because they didn't fix it already, and now the ship has sailed. That (unfortunately) has left it to promise extensions (like bluebird) and promise abstractions (like asynquence).

What the native browsers can or cannot do with respect to GC is in a whole other field, but it's even less something that I think is appropriate, since it's basically impossible to do in user-land JS. As soon as we pick an arbitrary 4s to pretend that's the same as GC, others will get bitten or surprised, and we'll be potentially chasing forever something that's just not possible.

If promise error handling bothers you (and I agree it should), then that's one of the reasons to use an extension or abstraction library. Sounds like you have plenty of experience with Bluebird, so my guess is you've already come up with various reasons to use it instead of native promises.

of native Firefox promises?

Wanted to address this point specifically. Neither Firefox nor Chrome seem to handle these scenarios, at least not all of them, as is claimed. I hear they may, eventually, but right now, these two nightlies yield:

As you can see from the image, the native promises still leak silent errors past the GC in at least some circumstances. I don't think it's a good idea to rely on browser dev tools to solve this "pit of failure" problem for you. I understand a lot of people are in favor of it, but I think it will be like chasing your tail. Just my 2 cents.

[benjamingr]

I meant "off topic" about the previous message (about asynquence) :)

is not standardized. I don't think this polyfill should deviate significantly from the spec, especially into evolving territory.

Well, you're right in that it's not standardized, but Firefox already does it and Chrome is doing it soon enough from what I understand.

GC based tracking is something that is impossible to solve from user land until ES7, at which point people hopefully won't need polyfills for ES6 features :)

As for it not working - it might not work from the developer console directly in cases like that. Go to http://jsfiddle.net/WXL7L/ and wait for a few seconds, it should appear in your console.

[petkaantonov]

GC based tracking can give false negatives, choosing a delay like 4 second (or even just end of the turn) can give false positives. Neither gives false anything in practice for normal use cases, but how can you reasonably prefer false positives over false negatives?

[benjamingr]

Of course, the question of "will this code throw at some point" in the general case is undecidable. I think tracking still can and will be better as more heuristics come along but like Petka said in several huge code bases and over a year I've seen the system miss exactly once (and there was one library bug at a point). Otherwise it is working perfectly fine.

[getify]

It's really irrelevant to NPO what I do or do not think is the right default behavior for error handling. I have made my case for how I think error handling should work over in the asynquence discussion. I stand by that opinion, even if you disagree.

This thread however is about NPO, which is striving to be as trimmed down and as close to native-only implementation as possible. It's not appropriate for this project to have extensions to behavior beyond the core specs.

If you like Bluebird's model of error handling better than core promises, then use Bluebird. NPO polyfill is just intended to be as good as possible at fitting in the slot of "I have no promises built-in and I want just promises, nothing else". Of course, everyone's entitled to their own opinions as to whether that's useful or not.

[benjamingr]

I have no promises built-in and I want just promises, nothing else

I think we both agree that:

1. The error handling model of promises without tracking unhandled rejections or .done is broken. (We even kind of agree on the solution, although not on the abstraction to use it on)
2. Debugging in older browsers is hard, the older the browser the harder debugging is.

So I still think that it would be useful to track unhandled rejections (like you do in asynquence) since it's the only solution I can come up with that doesn't change native promises in any way.

[getify]

To reduce the false positives, I had to invent defer() in asynquence. Which means to reduce false positives in NPO, I'd have to do something similar.

There were plenty of cases where "deferral" can be inferred, like when you pass a promise into another promise's chain. But there were also plenty of cases where you did things like Promise.reject(..). So, should that one automatically defer? You could make that argument. But what about this case? new Promise(function(resolve,reject){ reject(..); }). Is that automatic deferral? Or this case:

function ajax(url) {
   return new Promise(function(resolve,reject){
      makeAjaxCall(url,function(err,data){
         if (err) reject(err);
         else resolve(data);
      });
   });
}

var p = ajax("some.url");

document.addEventListener("DOMContentLoaded",function(){
   p.then(.., ..);
});

Here, it's kind of a race condition as to whether the error will be globally reported or not. But is there anyway to reasonably infer the "deferral"? I doubt it.

I think it's these cases which, if you use my strategy, require you to opt-out of global error reporting to reduce unnecessary false positives.

Is better error handling important enough to diverge from the spec to add a customized extension like defer()? Or would it be better for the polyfill-only implementation to simply "suffer" the extra false positives in the cases where "deferral" can't be inferred?

I admit I don't have a conclusion on those questions. The first one feels like "no" and the second one feels like "yes". But... shrugs.

[benjamingr]

I don't have strict conclusions about it either - otherwise I would be a lot more argumentative about it :p

Is better error handling important enough to diverge from the spec to add a customized extension like defer()?

I believe that the answer is no (agreeing with you), since this means that the library is no longer swappable for native promises which would kind of miss the point.

There were plenty of cases where "deferral" can be inferred, like when you pass a promise into another promise's chain.

Once the promise gets chained, it is no longer possibly unhandled.

Here, it's kind of a race condition as to whether the error will be globally reported or not. But is there anyway to reasonably infer the "deferral"? I doubt it.

That's actually a really good example of where the sort of error showing Bluebird does can be problematic, I'm surprised that Bluebird hasn't messed up on this even once on different browsers in large code bases and consistently - but it hasn't in practice. Maybe @petkaantonov can shed some light on the subject?

I tend to agree with you about the second one feeling like a yes.

[getify]

I'm surprised that Bluebird hasn't messed up on this even once on different browsers in large code bases and consistently - but it hasn't in practice

I'm not here to debate the merits of Bluebird. But in all fairness, it might be "messing up" more than you realize, but it's not something that people squawk about enough to rise to the level of high visibility. Shrugs. Also, race conditions.

[petkaantonov]

That's actually a really good example of where the sort of error showing Bluebird does can be problematic, I'm surprised that Bluebird hasn't messed up on this even once on different browsers in large code bases and consistently - but it hasn't in practice. Maybe @petkaantonov can shed some light on the subject?

If you chose to report them after 4 seconds, it would give false positive if dom loaded took 4 seconds - so never in development.

Secondly, of course you can construct infinite examples where error handler is attached asynchronously but does it make sense? Just like you can learn GC internals and construct examples where GC based error handling results in false negatives. Except how do you know about false negatives so you can refactor your code?

Thirdly, this is only a sane default, not the end of all. Bluebird provides hooks so you can implement any kind of scheme you want, be it GC based, live list of unhandled rejections, manual .done() and so on. However, there is no question what is the most sane default, the only one that cannot give false negatives.

[benjamingr]

Oh, that ping was just me wondering why it always worked in similar conditions. Then again I'd never let an error slide like that. My code is probably more like:

initialBootstrap().then(function(){
    return [onUiReady(), fetchAjax()];
}).spread(function(ui,data){
    ui.feed(data);
   //...
}).catch(ServerError,function(el){
     // handle ajax failures up to this point together, by updating the dom, in a promise.
});

[benjamingr]

I'm not here to debate the merits of Bluebird. But in all fairness, it might be "messing up" more than you realize, but it's not something that people squawk about enough to rise to the level of high visibility. Shrugs. Also, race conditions.

Ok, that's fair - I did however read every single promise question in Stack Overflow in the past year and I've answered over 150 of them. I've spent probably over a hundred hours in the Stack Overflow JavaScript chat talking to people about their experiences, and a similar amount of time on IRC.
I think that if someone had an issue with the way unhandled rejection is handled in Bluebird or in other libraries that implement it like When I'd hear about it. Considering it, you might be right though, and people might just not be very vocal about it - after all libraries make it very easy to turn off.

Have you had anyone experience issues with it in asynquence? (Any issues/etc)

Hmm.... what about an additional script that you can require before/after NPO that turns unhandled rejection tracking on/off? That way people can opt into it so we get debuggability but they don't have to use it.

[getify]

I don't think it would be out of the question to provide a debug build of NPO with such error tracking bits (and a defer()) in it.