-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
What happens when the factory throws an error? #21
Comments
Def tricky. It seems to me that the promise's fate has been sealed by the time
Same? Fate is sealed when
Hmmm. Maybe our new fulfillment is the goal ("fulfillment is possible" vs. "fulfillment is impossible") thinking can help here? Should an exception in the factory be taken to mean that fulfillment has become impossible? It seems intuitive to me that it should, so it seem ok to me to trap exceptions and ignore return values, despite the fact that |
So you're thinking a hybrid approach, wherein |
Errors should always be caught and rejected. It's easier to implement and think about:
|
@juandopazo that code would silence |
Ah yes, good question. Seems problematic to reject, since it would mean that Seems like ensuring a consistent behavior for
|
Consider the case where the resolve(x);
nonExistentFunction(); // oops I made a coding mistake If we silence this implicit |
@domenic damn I was thinking too much about
@briancavalier yes, that's what I want to avoid too. |
Same-turn and crash. Assuming promises are constructed inside |
I would much prefer to always treat We really can't go with throwing synchronously because that leads to people having to wrap calls to async methods in Crashing entire applications on thrown exceptions is pretty much never acceptable outside of a web-browser. So I think we should avoid that at all costs. We already silence double rejections and rejections after resolution, so I see no real problem with silencing The other alternative is to make var promise = Q.promise(function (resolve, reject) {
resolve(x);
nonExistentFunction(); // oops I made a coding mistake
});
//promise is rejected with the exception resulting
//from calling undefined function The exception rejects the promise before the call to |
Yes, but without due research you wouldn't know whether the async method you call could crash before the promise is even constructed. It strikes me that most code is already executing inside a then-callback, so thrown exceptions won't crash the program. People must assume that any function they call, even if it's supposed to return a promise, may throw instead. For example due to bad inputs. Next-turn/crash will require (non-promise) library authors to carefully guard their inputs as to not crash from the factory. Capture will require authors to carefully guard their inputs as to be able to throw useful errors, because otherwise they may be hidden by the promise library. Same-turn/crash means the (non-promise) library authors can deal with exceptions thrown by the factory as they see fit. Either by crashing the program, returning a rejected promise, etc. |
Capture shouldn't impose any such requirements. If errors are being hidden by the promise library, then the promise library is being misused. Most promise libraries provide a People actually rarely have to assume that functions they call may throw even if they're supposed to return promises. Even if people should be, they rarely do. It's mostly just much too complicated to think about something that could throw both synchronously and asynchronously. Pick one, and stick to it. Either you return synchronously and throw synchronously or you return asynchronously and throw asynchronously. |
In light of promises-aplus/promises-spec#76, I think it should be capture, not crash. |
In #18, I propose letting the factory run in a new turn per #20, and not catching any thrown errors (i.e. they should just crash the program).
The alternative is to catch any errors and transform them into rejections. This gets a bit complicated, though:
resolve(x); throw y;
behave?reject(x); throw y;
behave?Plus, it creates a somewhat false parallel between
factory
and the callbacks tothen
. The callbacks tothen
check both return values and thrown errors, while checking return values is not possible forfactory
.This is related to what is decided in #20, but in theory any combination could be decided on (same-turn, capture; same-turn, crash; next-turn, capture; next-turn, crash).
The text was updated successfully, but these errors were encountered: