Exception in callback functions
Opened this issue · 2 comments
scull7 commented
Currently, when an exception occurs within a callback function; handling that exception is very difficult.
Here are several examples listed (imho) in order of severity (greatest -> least)
- An exception occurs in a callback handler after transforming a Promise to a Future.t. This is particularly insidious because it effectively squelches the error; unless the user has the unhandled-rejection process handler defined.
testAsync("fromPromise (exception in callback)", done_ => {
let expected = "TestFutureJs.TestError,2,confused!";
let nodeFn = (callback) => callback(TestError("confused!"));
let promise = Js.Promise.make((~resolve as _, ~reject) => {
nodeFn((err) => reject(. err));
});
let future = () => FutureJs.fromPromise(promise, errorTransformer);
Future.value(Belt.Result.Ok("ignored"))
|. Future.tap(_ => Js.log("huh? - 1"))
|. Future.mapOk(_ => raise(TestError("oh the noes!")))
|. Future.tap(_ => Js.log("huh? - 2"))
|. Future.get(r => {
switch(r) {
| Belt.Result.Ok(_) => raise(TestError("shouldn't be possible"))
| Belt.Result.Error((s)) => s |. equals(expected)
};
done_();
});
});
- An exception occurs in a callback handler after an asynchronous operation. This causes problems because the exception cannot be caught by the traditional try/catch mechanism. Wrapping the call in a Promise may be a solution, though probably a bad one.
testAsync("mapOk (async exception)", done_ => {
delay(25, () => Belt.Result.Ok("ignored"))
|. Future.tap(_ => Js.log("mapOk-async-exception-1"))
|. Future.mapOk(_ => raise(TestError("boom, goes the dynamite!")))
|. Future.tap(_ => Js.log("mapOk-async-exception-2"))
|. Future.get(r => {
Js.log("mapOk-async-exception-3");
switch (r) {
| Ok(_) => raise(TestError("shouldn't be possible"));
| Error(TestError(s)) =>
Js.log("mapOk-async-exception-4");
s |. equals("boom, goes the dynamite!");
done_();
| Error(e) => raise(TestError(Js.String.make @@ e));
}
})
});
- An exception occurs in a callback handler after a synchronous operation. This exception can be caught by a traditional try/catch, however, the behavior is surprising, at least it was to me.
test("mapOk (sync exception)", () => {
Belt.Result.Ok("ignored")
|. Future.value
|. Future.mapOk(_ => raise(TestError("boom, goes the dynamite!")))
|. Future.get(r => switch (r) {
| Ok(_) => raise(TestError("shouldn't be possible"))
| Error(TestError(s)) => s |. equals("boom, goes the dynamite!")
| Error(e) => raise(TestError(Js.String.make @@ e))
})
});
gilbert commented
For (1), newer versions of node should report uncaught rejections (for the record; in chat you mentioned how jest / ospec were squelching it somehow).
(2) and (3) are due to Future's decision to delegate error handling to the user. The solution is to wrap your code in a try/catch but within the mapOk
itself. That way you can convert the error to a more appropriate type.
At least, that's what I think. Are these suggestions missing anything?
scull7 commented
I think that covers why the current behavior exists. Perhaps it is best solved with documentation then? The behavior (especially (1)) was very surprising to me.