reazen / relude

FP-inspired prelude/standard library for ReasonML projects
https://reazen.github.io/relude
MIT License
267 stars 41 forks source link

Ways to better integrate JS and OCaml exceptions into Relude IO #259

Closed esbullington closed 2 years ago

esbullington commented 4 years ago

I've been thinking about ways to better integrate legacy code and code that wraps JS APIs, both of which can throw OCaml and JS exceptions, into code using Relude IO.

If there's a good way right now to do so, pls let me know, but if not: one possibility, although the API is a bit complex, is to offer an IO function that takes a function capable of converting a JS or OCaml exception, its parameter(s), and a function that converts the exception(s) into a custom IO error type.

It's just an expansion on the existing IO.tries and IO.triesJS, but can be integrated into other Relude IO since the type is Relude.IO.t('a, 'e) instead of Relude.IO.t('a, exn). So you'd end up with something like this:

// These two functions could be in Relude.IO module (open to ideas for better names):
let fromExn1: 'a 'b 'e. ('a => 'b) => 'a => (exn => 'e) => IO.t('b, 'e) =
  fn => v => errorFn =>
    IO.SuspendIO(
      () =>
        switch(fn(v)) {
        | v => IO.Pure(v)
        | exception exn => IO.Throw(errorFn(exn))
        },
    );

let fromExn2: 'a 'b 'c 'e. ('a => 'b => 'c) => 'a => 'b => (exn => 'e) => IO.t('c, 'e) =
  fn => v1 => v2 => errorFn =>
    IO.SuspendIO(
      () =>
        switch(fn(v1, v2)) {
        | v => IO.Pure(v)
        | exception exn => IO.Throw(errorFn(exn))
        },
    );
// End Relude IO

// Example usage:
module ErrorIO =  {
  type t =
    | MainApplicationError
    | InvalidArgumentOCamlException(string)
    | OCamlException(string)
    | JSException(option(string))
    ; 

  let fromExn = fun
    | Js.Exn.Error(jsExn) => JSException(Js.Exn.message(jsExn))
    | Invalid_argument(s) => InvalidArgumentOCamlException(s)
    | exn => OCamlException(Printexc.to_string(exn))
  ;

  let show = fun
    | MainApplicationError => "Main error"
    | InvalidArgumentOCamlException(s) => "OCamlException: Invalid argument: " ++ s
    | OCamlException(s) => "OCaml exception: " ++ s
    | JSException(Some(s)) => "JS exception: " ++ s"
    | JSException(None) => "Unspecified JS exception"
  ;
};

module IO = Relude.IO;
module IOE = IO.WithError(ErrorIO);

// this could also be a function raising any JS exception
let f1 = (c) =>
  if (c > 1)
    raise(Invalid_argument("too big"))
  else
    c + 4

let mainIO = () => {
  fromExn1(f1, 0, ErrorIO.fromExn);
};

mainIO()
  |> IO.unsafeRunAsync(fun
    | Ok(v) => Js.log2("OK: ", v)
    | Error(e) => Js.log2("Error: ", ErrorIO.show(e))
  );
esbullington commented 4 years ago

I'm not even 100% this would be worth it, since right now I'm just wrapping functions like this in a try block inside IO.async, and then converting caught exceptions into IO Errors using onDone.

This approach would just let me centralize the error logic in one place instead of scattered in throughout the code base.

But not sure, just throwing out ideas.

EDIT: Actually, having tried this out in our codebase, this is pretty useful. It allows me to centralize the error handling in one place in all our IO code, instead of having a bunch of try expressions in our IO code. And it can be as granular as I want. I can either continue to catch all JS and/or OCaml exceptions and convert them to just JSException(option(string)) and OCamlException(string) or I can create individuals variants for them if I need more granularity.

It's possible I'm misusing, or not understanding, the existing Relude.IO.tries and Relude.IO.triesJS, but those were not helpful for me since they couldn't be integrated into my other Relude IO code. triesJS returns Relude.IO.t('a, Js.Exn.t) type, and for whatever reason, BuckleScript made BuckleScript JavaScript exceptions a private extension to OCaml extensible variants, so I can't extend Js.Exn.t. Plus, I prefer to use a custom error type for Relude IO errors, anyway, rather than extending OCaml extensible exceptions, even if that were possible with Js.Exn.t.

I'll definitely use the approach above in our IO code, unless I'm misunderstanding how to integrate tries and triesJS into other IO code.

I haven't looked closely at the WithError functor in Relude.IO, so I'm not sure, but if you don't like the fromExn1 and fromExn2 functions above having to include the function call each time to convert exceptions into custom errors, you might could also make a WithExtensibleError functor for Relude.IO that's similar to WithError. It could take a extensible variant as an argument, with that module already including the JsException and OCamlException base constructors in the base extensible variant, and then you could include that logic in the library rather than requiring the user to pass the conversion function (although you'd want to still offer the option to pass a conversion function in case the user wants greater granularity).

andywhite37 commented 4 years ago

I think the general pattern for synchronous calls would be to use Result.tries, then Result.mapError, rather than going to IO, because IO brings async semantics that you might not want/need.

Result.tries(() => someThrowingFunction(whatever, args, etc))
|> Result.mapError(e => makeSenseOfError(e))

For async code, there is IO.tries, but this basically just wraps the call like above, but forces you into the async IO context, which might make sense if you're already operating in IO, but maybe not if you just want a synchronous call.

I'm not sure if we need the different arities of tries, because you can just wrap the whole thing in a unit => 'a function.

I could see an argument for a version of tries (for both Result and IO) that has the error mapping built-in, like:

let triesMapError: (exn => 'e, unit => 'a) => result('e, 'a)
let triesMapErrorJs: (Js.Exn.t => 'e, unit => 'a) => result('e, 'a)

These would provide an error handling function rather than having to do the mapError.

Is this what you're thinking?

andywhite37 commented 2 years ago

I'm going to close this for inactivity. I'm not sure if anything should be done here, or if it's more of a discussion. Feel free to re-open if more discussion is desirable.