Converting Exceptions to Rejections in Initializer

[ghost]

Apologies if I'm retreading old ground, but under Promise(resolver), step 10 indicates that exceptions thrown by the initializer function must be converted to a rejection of the new promise. This seems surprising to me:

var promise = new Promise((resolve, reject) => {
    throw new Error("boom");
});

console.log("Should I see this?");

The initializer executes synchronously, and really just provides a closure around the resolve and reject parameters. It's more akin to a block than a promise transform. What are the arguments in favor of converting exceptions?

Thanks!

[annevk]

Making throwing reject the promise would be nicely consistent with new platform methods that return a promise and could synchronously throw for some input, but would instead reject the promise, so that the whole programming paradigm remains asynchronous.

[ghost]

Can you provide an example of such a new platform method?

[domenic]

I do think it's a bit weird that we capture user-thrown exceptions but let argument validation exceptions to leak out, per #10.

I believe one motivation for capturing user-thrown exceptions is the almost-equivalence between

new Promise((resolve, reject) => ...)

and

Promise.resolve({ then(resolve, reject) { ... } })

[annevk]

E.g. createImageBitmap() in HTML returns a promise. If the first argument is not of the correct type, the promise should be rejected and no exception should be thrown, I think. @domenic at least argued for that and it makes sense to me.

[domenic]

Yes, for APIs like createImageBitmap() that's definitely the right choice, otherwise you have to add not only asynchronous error handling code but a try-catch as well.

[ghost]

So if I understand correctly, the underlying principle here is: functions which return promises should never throw. I think I can buy that, with the caveat that this rule should be consistently applied (across HTML, ES standard library, etc.).

But should the Promise constructor obey this rule? I need to think about that some more.

[annevk]

Yeah, for the swath of platform APIs this is easy to enforce through IDL. ES standard library through good review I suppose. (Which all APIs should receive, but not everyone knows about everything...)

And new Promise does return a promise :-)

[domenic]

@annevk @erights I think @zenparsing's essential point is that we are being inconsistent, saying that it's OK for new Promise to throw for argument/context errors, but not for other functions which create promises. Do either of you feel like it's worth revisiting that allowance, perhaps taking into account my arguments over at https://github.com/domenic/promises-unwrapping/issues/24#issuecomment-23979547 into account? As I said originally in #10, I can see both sides of the argument.

[annevk]

I thought only Promise() threw, not new Promise()?

So I guess in particular this is about new Promise(2) and such as 2 is not callable?

I think the consistency is that callbacks should be treated identically, but that fundamental misuse of the constructor can throw early. That's a clear typo, the callback throwing could have any number of reasons.

[domenic]

So I guess in particular this is about new Promise(2) and such as 2 is not callable?

Right, that's it.

That's a clear typo, the callback throwing could have any number of reasons.

Hmm OK, that has me more convinced. In particular, new Promise(resolve => resolve(document.creatElement("div"))) is a clear typo, which made me feel like it was analogous, but then again new Promise(() => throw new Error("I am purposefully creating a rejected promise")) or new Promise(resolve => resolve(callAFunctionWhichMightThrowAnError())) are definitely not typos. So "any number of reasons" was helpful. Thanks.

[ghost]

I would think that there are many APIs that one could argue should throw early because of "clear typos" (e.g. parameter type validation).

I haven't really formed an opinion on this yet...

[domenic]

Let's close this and leave as-is. I admit it's not a clear-cut answer, but I think the current situation is pretty good. I am open to compelling arguments in favor always-reject (even for argument/context validation errors), or even the OP's never-trap, but they would have to be made quickly and compellingly since we need to lock this down.

[ghost]

Just continuing this inquiry...

The rule that promise-returning functions should never throw exceptions is going to be difficult for user code to maintain. Consider this:

function getPromise({ x, y, z }) { /*...*/ }
var p = getPromise({}); // Boom - synchronous exception

Destructuring is refutable and can therefore throw. A corollary of the rule that promise-returning functions can never throw is that promise-returning functions cannot use destructuring (edit: in their argument list). I wouldn't want to commit anyone to that restriction.

So if I throw out the rule that promise returning functions can never throw, I'm left with the possibility of synchronous and asynchronous errors. Per @domenic, we don't want to bifurcate our error handling logic. What, then, is the solution? One solution is to mark a code block such that all errors (synchronous or otherwise) are routed asynchronously:

PROMISE_BLOCK(_ => {
    function getPromise({ x, y, z }) { /*...*/ }
    var p = getPromise({}); // Exception converted to a rejection of this block
});

Obviously, the name isn't meant to be an actual API suggestion : )

With this strategy, functions do not protect synchronous callers from exceptions. Synchronous callers protect themselves, if they choose. I believe that this is a more feasible and scalable approach.

If so, then the primary motivation for catching exceptions thrown by the initializer disappears.

[domenic]

That's a pretty compelling argument, although I still maintain well-designed functions should not have dual failure modes. In Q we do this by var promiseReturningFunction = Q.fbind(possibleThrowingOrReturningFunction). In ES7 you can imagine an async keyword (or perhaps a sigil, function! or function^ or similar), which has the same effect.

In the meantime, I am not sure what the encouraged pattern should be for writing promise-returning functions, besides "be very careful" and even "don't use destructuring." Shifting the burden to the consumer, as you suggest, is definitely possible (Q.try, a.k.a. Q.fcall) but is unfortunate. @erights?

[erights]

Sigh. I also find this argument compelling. In fact, it has me considering changing my mind on promise initializer behavior, so I am reopening this issue. (I say "sigh" only because I hope to close all significant open issues here before the upcoming TC39 meeting; but if a question is open, it is open.)

For promise initialization, the observable difference between sync and async errors could be taken as a useful signal rather than just an annoyance. But it is essential that promise.then and its brethren do provide the no-sync-errors guarantee directly, as the integrity of much sync code will depend on that. (See the "aborting the wrong plan" attack in my thesis and presentations.) To keep the resulting annoyance tolerable we eventually need two things:

• a wrapping function such as @zenparsing's PROMISE_BLOCK, where the callback argument is still called synchronously. I love using .try for this except for precedent: The existing Q.try calls this callback asynchronously, which is also a valuable abstraction, but doesn't fill the current need.
• a convention for promise-returning functions, such that we can anticipate when they should guarantee no synchronous errors. For system-provided APIs, this "should" is "must", but I don't imagine a convention with any enforcement mechanism for user-provided APIs.

@annevk already provided one criteria we've agreed to: If the detected problem indicates a static bug which the programmer should simply correct, then a sync error is much better, as it will be noticed by any test that covers the code. A problem that is conditional on dynamic data is not covered by Anne's criteria by itself.

With the difference between promise initialization and .then in mind, I propose the following additional criteria: If the method operates on non-this arguments to this function synchronously, then sync errors thrown by these interactions, and only these interactions, are propagated as sync errors. Specifically, if the operation calls callback arguments synchronously, then throws by these callbacks are propagated as throws. By this convention, promise initialization would be expected to propagate initialization throws and .then would still be required to protect against internal thenable-testing errors. Given some PROMISE_BLOCK operation, the nice thing about this convention is that PROMISE_BLOCK around either the callback body itself or the operation as a whole would generally have the same effect.

@domenic @kriskowal How disruptive would it be to current Q users to repurpose .try to have this synchronous PROMISE_BLOCK meaning?

I emphasize that we eventually need these two things because we should not try to resolve precisely how these two things are provided here, as long as we're confident they can be provided in a compatible superset of the current proposal.

@domenic Regarding

I believe one motivation for capturing user-thrown exceptions is the almost-equivalence between

new Promise((resolve, reject) => ...)

and

Promise.resolve({ then(resolve, reject) { ... } })

these are already inequivalent in a bigger way -- when does the "..." code run? In promise initialization, it runs during the initialization, with the caller needing to cope with all the sync plan interference hazards. With .resolve, it happens later, without any user stack frames underneath it, avoiding plan interference hazards but need to cope with all the world-may-have-changed-in-the-meantime hazards. Whatever we do with errors, this difference is a bigger deal than whether exceptions are reported sync or async. Given this, perhaps it is nice for this error handling difference to align with this difference in when callbacks are called?

[annevk]

Do PROMISE_BLOCK errors go to the console?

Is the proposal that thrown exceptions would no longer reject a promise? That is incompatible with A+. We could do it for the constructor I suppose, but then you would get surprised about the behavior with then().

Destructing errors seem like a static bug at the language level. It seems fine for those to throw early.

[erights]

Do PROMISE_BLOCK errors go to the console?

No, they result in rejected promises. PROMISE_BLOCK(...).done() errors would go to the console.

I imagine a self-hosted implementation of PROMISE_BLOCK would be something like:

function PROMISE_BLOCK(callback) {
    try {
        return callback(); 
        // or perhaps: return Promise.cast(callback());
    } catch (er) {
        return new Promise((_,r) => r(er));
    }
}

Is the proposal that thrown exceptions would no longer reject a promise? That is incompatible with A+. We could do it for the constructor I suppose, but then you would get surprised about the behavior with then().

No, that's not what I had in mind at all. A thrown exception would either be reported sync or async. If the error is thrown by a sync-called callback, as in the initializer, the throw propagates. If the error is thrown by an async called callback, as in .then, it results in a rejection. The suggestion would not in any way change the current behavior of .then.

However, I withdraw both the suggestion about .try and the above PROMISE_BLOCK. I no longer think either is needed, and that the current behavior of Q.try is exactly what it should be. The idea is that we should distinguish two kinds of promise returning functions:

• Those like the initializer, where argument provided code may run synchronously during the call. For these, the caller is not protected against sync plan interference, and so should not expect to be protected from thrown exceptions
• Those like .then, that guarantee that all argument provided code is only run async after the call, if at all, and therefore reliably protects the caller against synchronous plan interference. The "aborting the wrong plan" attack should be regarded as a form of sync plan interference, and so these operations should protect against that as well -- except in the special case you identified, where the error likely indicates a static mistake that the programmer should simply fix.

Destructing errors seem like a static bug at the language level. It seems fine for those to throw early.

Yes, that would be an example of this special case.

Given this understanding, I don't see a need to provide an abstraction that calls the callback synchronously but still protects only against thrown errors.

[domenic]

OK. Is this an accurate representation of your ideas, @erights?

• Change the promise constructor to not trap thrown errors; treat them as static failures of the promise-creation process. (E.g. new Promise(r => getEventEmtter().on("success", r)) would throw, because of the typo.)

• Add to our list of nice-to-have-later additions a Promise.try(func), which is essentially defined as

  Promise.try = func => new Promise((resolve, reject) => {
    process.nextTick(() => {
      try { resolve(func()); } catch (e) { reject(e); }
    });
  });

  Then people can use this on any function to ensure that the function runs async and returns a promise.

• Change the guidance in the accompanying promise-API-writing-guide to say that argument validation errors should be sync exceptions, not rejections.

I am not sure I like this last point, as I found my arguments over in #24 fairly compelling. But I am not sure there is a good consistent story, especially in the face of refutable argument destructuring.

[mhofman]

This is quite a departure from some of the current implementation, such as the current DOM Promises proposal / polyfill, as well as Q.promise. My concern is that the synchronous code executed in the initializer is, as the name says, used to kick of an asynchronous operation. There will be a lot of promise wrappers built around existing callback based APIs and I'm sure some will throw synchronously. This would require to wrap manually all those in a try / catch statement or force the initialization to happen on the next tick with that Promise.try. As others have pointed out, the destructuring case is a developer mistake that can be caught by static analysis and I have no issues with it throwing synchronously. To me it's akin to the Promise constructor throwing if the initializer is not a function. I don't see why we should change how exceptions are handled inside the initializer.

[erights]

@mhofman I am hopeful that this argument from precedent lets us resolve this to the status quo. Even after elaborating the argument on the other side, history aside, I am only weakly in favor of it. The way of the web is to avoid breaking a strong precedent without an extremely strong reason, which we do not have here. We respected this, for example, by not changing the signature of .all without changing the name, and only changing the name back when we changed the signature back.

On the promise constructor, we got the idea from WinJS. Anyone know what the WinJS promise constructor does with thrown exceptions? http://msdn.microsoft.com/en-us/library/windows/apps/br211866.aspx doesn't say.

[erights]

OK. Is this an accurate representation of your ideas, @erights?

@domenic Yes, that accurately represents the position I stated.

I am not sure I like this last point, as I found my arguments over in #24 fairly compelling. But I am not sure there is a good consistent story, especially in the face of refutable argument destructuring.

I'm not sure either. After having given my best shot at trying to believe and argue for this change, I'm still unsure. I like @mhofman's point about precedent. If it holds up, I'm inclined to close this with the status quo. We may anyway.

[ghost]

As others have pointed out, the destructuring case is a developer mistake that can be caught by static analysis and I have no issues with it throwing synchronously.

Just pointing out that this is not really true. You can construct all kinds of cases where a function call throws based on refutation of argument destructuring, which are not statically knowable.

[ghost]

Change the guidance in the accompanying promise-API-writing-guide to say that argument validation errors should be sync exceptions, not rejections.

I think an appropriate thing to advise is that, to put it into HTTP terms, 40x errors should manifest as exceptions, and 50x errors should manifest as rejections. If you get a promise back from a function, then that should indicate that you provided the function with enough valid information to at least "start" the operation, whatever that may be. Without language-level support for "async" functions, that's probably the best we can do.

[ghost]

Actually, I'm not sure if I believe what I wrote just above without constructing some code examples...

[erights]

Having made the decision to reopen this, mulling over the arguments since then, and in light of the short time till the TC39 meeting, I'm closing this as the status quo. If there are strong objections we can reopen, but from the arguments so far, the status quo seems good enough even if possibly not ideal. Objections?

[domenic]

@erights I guess my only objection is that the status quo is weird. We don't have any real guidance we can give to authors, and from one perspective, we are inconsistent with our own current guidance (never throw) for the promise constructor, since we throw on non-functions or bad this.

I actually just had an extended discussion yesterday with @tomdale (he works on Ember, which uses RSVP) which ended up orbiting around this point, I believe. We were discussing the usability issues around "swallowed" errors, and in addition to arguing for some crazy things like onFulfilled and onRejected not converting SyntaxErrors/ReferenceErrors into rejections, I think he was also suggesting that promise constructor behavior pass through thrown exceptions, because they mean that "something went wrong" in the non-promise-async to promise-async conversion steps (my new Promise(r => getEventEmtter().on("success", r)) typo example).

So I am even less sure.

[erights]

I am also unsure that this is the right decision, but I am confident that it is good enough. The status quo at least satisfies all the hard criteria we are already confident of.

DOM needs to proceed with something asap. TC39 is about to meet. If a clear decision emerges from that meeting to close this issue differently, fine. Otherwise, by leaving this issue closed, it remains the clear fallback position if not overturned this week -- unblocking the DOM with something that is clearly good enough.