Promise error handling

[Lupus]

I'm opening this issue for discussion on the promise error handling.

The Promise module does not include support for rejections so far:

 module Promise = {
  type t('a) = Js.Promise.t('a);
  let return = Js.Promise.resolve;
  let map = (value, ~f) => value |> Js.Promise.then_(res => Js.Promise.resolve(f(res)));
  let bind = (value, ~f) => value |> Js.Promise.then_(f);
  let consume = (value, ~f) => value |> Js.Promise.then_(res => {f(res); Js.Promise.resolve(0)}) |> ignore;
  let join2 = (a, b) => Js.Promise.all([|map(a, ~f=(r => Left(r))), map(b, ~f=(r => Right(r)))|]) |> Js.Promise.then_(items => Js.Promise.resolve((leftForce(items[0]), rightForce(items[1]))));
};

Yet the NodeContinuation module returns Js.Result.t based on what was passed to the callback. If we promisify such callback, we'll get promise resolution or rejection. I'm not sure if it's node specific or general JS pattern. Actually then supports 2 callbacks and it's recommended to use second one in some articles:

p.then(onFulfilled[, onRejected]);

p.then(function(value) {
  // fulfillment
}, function(reason) {
  // rejection
});

May be that should be the way for Promise module, so that it returns Js.Return.t based on what callback was invoked akin to NodeContinuation? Probably reject type should be exn.

ES7 await just seems to throw rejection as exception, future.js (bundled with node-fibers) is also following the same pattern.

I've got a recommendation on the Discord to never reject promises unless it's fatal and just resolve them with result type of value. This works fine if you own all the code, but the benefit of using node ecosystem is leveraging large number of "batteries", and those do reject as a way of indicating errors. It makes sense to follow ecosystem conventions instead of fighting them and thus I'm thinking about a way to do that within reason_async.

P.S.: I've tried out this cool project recently and I really like the way it sugared promise code looks, very clean! Thanks for your work!

[jaredly]

Glad you like it! As it currently stands, exceptions act the same way as in js's async/await, that is they are captured in the promise that's returned from the containing function (or the expression in the case of reason). So

let f = () => {
  [%await let x = somePromise()];
  x + 2
};
let z = f(); /* if somePromise was rejected, z is now rejected, and we can `Js.Promise.catch()` it */

If you wanted to handle it with a default earlier, you could do

let f = () => {
  [%await let x = somePromise() |> Js.Promise.catch((err) => 5)];
  x + 2
};
let z = f();

Is that enough? Or can you think of some syntax that would make it clearer?

[Lupus]

As it currently stands, exceptions act the same way as in js's async/await, that is they are captured in the promise that's returned from the containing function (or the expression in the case of reason).

Hm, according to the documentation [1] "If the Promise is rejected, the await expression throws the rejected value".

We had discussion with colleagues recently, studied our codebase (in other language) that uses manual error propagation (Golang nil,err style) and decided that exceptions would make it cleaner for us, as most of the time we're just doing manual error propagation, and being manual makes it error prone (pun intended :) ).

To match JS await behavior one could probably define another Let_syntax module which throws OCaml exception from Js.Promise.catch handler.

[1] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/await

[jaredly]

Actually it does currently match js await behavior -- because you can only await within an async function, which catches all thrown exceptions and rejects the wrapping promise.

async function x() {
  throw "beep";
}
x() /* is now rejected with "beep", does not throw an exception */

[Lupus]

Probably I'm not clear in my explanations/terminology. Let me try one more time :)

An example taken from an article describing relation between promises and async/await:

let hn = require('@datafire/hacker_news').create();

// Old code with promises:
Promise.resolve()
  .then(_ => hn.getStories({storyType: 'top'}))
  .then(storyIDs => hn.getItem({itemID: storyIDs[0]))
  .then(topStory => console.log(topStory))
  .catch(e => console.error(e))

// New code with async / await:
(async () => {
  try {
    let storyIDs = await hn.getStories({storyType: 'top'});
    let topStory = await hn.getItem({itemID: storyIDs[0]});
    console.log(topStory);
  }  catch (e) {
    console.error(e);
  }
})();

What I'm trying to stress is that await code within async function does throw rejections as exceptions. If the "new code" from the above snippet didn't do try/catch, anonymous function would itself return rejected promise, I agree. But the error will start bubbling up as long as await gets rejection and fail fast without additional boilerplat per each await site.

Given that reason + this ppx does not have a notion of async function, exception (if thrown automatically) will not be caught by async function boundary and will bubble up further. May be that's a bad thing to do unless there are async functions which implicitly do try/catch and return rejected promises to follow JS async/await behavior.

[jaredly]

Ahhh yes now I understand. try / catch doesn't interact the same way, is what you're saying. I wonder if something like

[%async try {
  ... something promisy
} catch (e) {
  ... handle the error, return a value
}]

would do the trick?

[Lupus]

Hm, what would be the semantics of this construct once de-sugared? Shouldn't catch just return rejected promise? Is it possible to annotate a function like this:

let f = [@async] () => { 42 };

So that its body is being wrapped in a try/catch and changed to return a promise (which either resolves in case of no exceptions or rejects in case something was thrown down the lines)?

[jaredly]

.catch in promise land returns a value that then resolves the resulting promise. hmm yeah I guess you could do that wrapping