Research: a debug build of NPO that includes better error tracking about native-promise-only

[benjamingr]

Just found this - that would be awesome.

What sort of tracking did you have in mind? I can take a shot at implementing

[getify]

@benjamingr Thanks for raising a hand. Would love some help if you're interested in taking a stab at it!

See this explanation of how asynquence handles errors, which is basically the opposite approach of how native promises do it.

Basically, the idea is that the promise object should default to reporting any error to the console (console.error(..)) rather than defaulting to silence. IOW, basically we're changing error handling/reporting from being an opt-in mechanism to being an opt-out mechanism.

But, since this will be very verbose (lots of "duplicate" errors reported there which you're already correctly handling in the app), we need some heuristics of when this global error console reporting can be omitted for a promise object:

1. If a promise object already has a valid error handler (not just the default empty one) registered at the moment a promise gets a rejection/error, then you implicitly opt-out: don't worry about reporting to console.
2. To handle the case where you the developer know for sure that a promise will get an error handler attached later, but it may get the error triggering now (and so you don't want the unnecessary reporting), you need a way to explicitly opt-out. My plan is to have an extra method on the promise object called defer() which suppresses the error console reporting. You must call defer() on a promise before it gets rejected. Well... sorta (see (2) below).

---

Some design considerations:

1. I don't want this behavior to be present in the production build of NPO. It's unnecessary weight, and most importantly defer() is a deviation from the spec, which is against the main design principle of NPO. So, this must be a separate dev-only debug build.

   How I want this to work is to have the debug bits in the original source code package, but have comment markers at the beginning and end of each debug section of code, and then modify the production ./build.js tool to find these debug section markers via regex and remove that code, before minification.

   This takes some careful planning of how the code is written. I've done this sort of thing a few times before, with LABjs (script loader) and grips (templating engine). Examples:

   • LABjs L41 - L48
   • LABjs L137
   • grips L219 -- Pay particular attention to how this line works, it's tricky. ;-)

2. One tricky detail (which isn't an issue in asynquence) is immediate promise rejection, such as new Promise(function(resolve,reject){ reject("immediately"); }) (which is exactly what Promise.reject(..) does too). In these cases, you wouldn't have a chance to call defer() before the rejection, but you almost certainly wouldn't want the message reported to the console.

   TBH, I'm not exactly sure how to do this. I was going to experiment. It's pretty tricky, I believe.

   I think the way to do this is basically to have a separate internal queue (*not the same as the queue that already is there, I don't think) of these global error reporting tasks, somehow indexed by each promise instance (like by some internal ID). Whenever a promise rejection occurs and an error *should be console reported, instead add a scheduled (aka async) task to the queue to report it. If defer() gets called on a promise object, we should see if its already a rejected promise and then look up by internal ID if there's still a queued task to report its error to the console, and if so, clear that item from the queue.

   Like I said, this is going to be kinda tricky to get right, especially without violating the other standard behaviors of promises. We'll have to make sure to regularly run the test suite to make sure we aren't creating a regression.

[benjamingr]

If a promise object already has a valid error handler (not just the default empty one) registered at the moment a promise gets a rejection/error, then you implicitly opt-out: don't worry about reporting to console.

How do you feel about a 2 second timeout here? I don't think Promise.reject(3) should log to the console.

Since this is only for the debug build anyway we can:

• Keep a "rejected but unhandled" set.
• Whenever a promise is rejected - add it to the set.
• If it has been in the set for 2 seconds - log it to the console as a default and remove it from the set, allow this functionality to be overridden and to "no op" in production. for example - by assigning to Promise.onPossiblyUnhandledRejection rather than keeping it a function in the production build could just be assigning to undefined or something similar.
• If a rejection handler is added to the promise - it is removed from the unhandled queue.

What do you think?

[getify]

I really, strongly dislike arbitrary-length timeouts. They're a crazy hack for this case. I believe they encourage race condition coding.

Promise.reject(3) is what my whole discussion above in design considerations (2) was about, regarding the second internal queue, etc.

You should be able to do Promise.reject(3).defer(), and it wouldn't log to the console. But to accomplish that, I believe we have to "defer" the console reporting until the end of the current event turn, just like promise resolutions do.

So, in other words, var p = Promise.reject(3) is fine and will stay silent as long as somewhere later in the same event turn you call p.defer(). But if you don't, at the end of the event loop turn, it'll report.

[benjamingr]

The problem with requiring users to .defer() is that it effectively makes migrating to actual native promises and using the library only to shim them impossible - so I believe any solution that allow ES6 promises to work is problematic.

While I dislike the concept of an arbitrary timeout I'm not sure what other options we have in ES5 - the only race condition I could think of is in the order of unhandled rejections - what if we don't allow onPossiblyUnhandledRejection to begin with and just always log it? That way we know there will be no race conditions possible (or at least they'd require back flips like overriding console.error). What do you think?

[getify]

it effectively makes migrating to actual native promises and using the library only to shim them impossible

Which is exactly why I'm making this a debug-build feature only and not just extending the production API or behavior. This whole feature is purely for debugging code in development. Just like with console.log(..) and debugger code, you'll need to strip out any defer() calls when going to production. Since we won't be doing any error-log reporting in production, there won't be any need to defer() to silence it.

The invariant of the production build of this library is that it sticks strictly to the API and semantics/behavior of the spec. The only exception to that is the already-documented lack of subclassability, and I extensively documented why that extraordinary deviation was necessary.

It's a given that the spec is bad/limited in this error-handling way, but it's not the job of a polyfill to solve that. If you want a production solution to this error-handling, you need to use an abstraction library.

[benjamingr]

It's a given that the spec is bad/limited in this error-handling way, but it's not the job of a polyfill to solve that. If you want a production solution to this error-handling, you need to use an abstraction library.

Meh, if it's gonna get to this again I'm out of here. I'm willing to help with making exception handling easier with NPO. Given how our last argument ended up on promises I don't feel like exploring the "abstraction library" will be very constructive.

Thanks for your time :)

[getify]

I'm willing to help with making exception handling easier with NPO

It's really quite simple in my view: polyfills don't add/change functionality, or they're not polyfills. To me, it matters to stick to that principle. The debug build option I've suggested is a way to compromise in recognition that debugging lost errors is hard. But if you want something more than the spec in production, a polyfill is not (or should not be!) the appropriate thing.

I'm out of here

I'm sorry you feel that way, but thanks for your time and participation.