Open jguyon opened 6 years ago
Hi @jguyon, thank you for your interest in contributing to the project, I really appreciate it.
Exception handling is indeed a missing feature. However, adding reject
to Future kind of makes it a promise, which is something I'd like to avoid if possible, mostly to avoid complicating the implementation and type signature. But there needs to be some solution for JavaScript exceptions, so I'm glad you're bringing the issue to light.
You or someone else might have a better idea, but I was thinking of an API to catch the error as soon as possible. Something like...
// Instead of...
Future.make(resolve => dangerousJs() |. resolve)
// You use...
FutureJs.make(
resolve => dangerousJs() |. resolve,
err => ...
)
where err
is perhaps a Js.Exn.t
. But like you said, JavaScript can throw anything... and a library might also throw different types of errors (SyntaxError
, MyCustomError
, etc.). But the main idea & feature is to force the err => ...
function to return the same type as the resolve =>
function, unifying things from the very start.
I named this particular example FutureJs
because it would only exist to catch JS-specific behavior. Normal Reason/OCaml exceptions already have a handy syntax that I think is sufficient for this use case:
Future.make( resolve => switch(dangerous()) {
| result => resolve(result)
| exception Not_found => resolve(...)
})
Thank you for the detailed answer!
After reading it I'm realizing that I was thinking too much in terms of js promises. The use cases I had in mind would indeed be better solved by explictly handling exceptions on each callback like in your last example.
Otherwise, I like your idea for handling exceptions from js, I think it's simple and clear.
Feel free to close this PR if you think discussions around this would be better placed in issues, I don't think much of the changes I made could be reused to implement what you've detailed.
This PR allows to reject a future by calling
Future.error
or by raising in a callback.After our small exchange yesterday on discord, I got interested and started experimenting with handling exceptions, and this is the result :)
One thing to consider before merging this is that catching exceptions doesn't seem to be sound in bucklescript, at least to my understanding, because js can throw whatever, not just
Error
objects. I don't know if this is a showstopper for you, but I thought a PR could at least serve as a reference if it is.Otherwise, I was not sure what to name the equivalent of
catch
because naming itflatMapError
was out of the question, so I named it...catch
, but you might have a better idea?