How to structure result lwt code to avoid nesting hell

I have an issue - I haven’t found an easy way to structure code in a simple, readable way.

I’m working with APIs that return Lwt results, and I need to handle both the Ok and Error cases, so I can’t just map them.

I also have some conditions (if/else) that can’t be included in the match ‘when’

In an imperative language I would use early exits, so the code would look like (this is a simplified version):

const a = await myService.getA();
if(!a.isOk) {
  errorService.log('error with a', a.err);
  return Error('a');
}

const b = await myService.getB(a.value);
if(!b.isOk) {
  errorService.log('error with b', b.err);
  return Error('b');
}

if(b.length > 5) {
  return Error('z');
}

const c = await myService.getC(b.value);
if(!c.isOk) {
  errorService.log('error with c', c.err);
  return Error('c');
}

return c.value * 5;

but the same in OCaml gets very nested, with many brackets, so it gets difficult to follow (and that’s a simple, contrived example):

let* a = MyService.get_a () in
match a with
| Error err ->
    ErrorService.log "error with a" err
    Lwt.return (Error A)
| Ok value -> (
    let* b = MyService.get_b value in
    (match b with
      | Error err ->
          ErrorService.log "error with b" err
          Lwt.return (Error B)
      | Ok value ->
          if List.length value > 5
          then Error Z
          else (
            let* c = MyService.get_c value in
            (match c with
              | Error err ->
                  ErrorService.log "error with c" err
                  Lwt.return (Error C)
              | Ok value ->  value * 5)
         )
    )
)

note: I don’t want to move each handling to a separate function, because it then gets even more annoying to read - you have to jump to separate definitions instead of reading the code top-down

How do you deal with early exits, deeply nested code and handling of both Ok and Result in OCaml?

6 Likes

Short-circuiting failure can be abstracted away with the result monad ('a, _) result:

let ( let* ) x f = match x with
  | Ok x -> f x
  | Error _ as e -> e

let () =
  let* a = attempt_to_get_an_alpha () in
  let* b = attempt_to_get_a_beta () in
  a + b

When using this pattern in Lwt code, one ends up wanting to compose the result monad with the Lwt monad[^1], giving a monad of type ('a, _) result Lwt.t:

let ( let* ) x f =
  Lwt.bind x (function
    | Ok x -> f x
    | Error _ as e -> Lwt.return e)

Fortunately, Lwt already provides this in Lwt_result. Aside from use of the Lwt_result monad, your code has another pattern related to logging the error value and returning the error context, which I’d probably pull out into a utility function. Putting these together:

open Lwt_result.Syntax

let or_log_error ctx =
  Lwt_result.map_err (fun err_val ->
      let err_msg = "error with " ^ (match ctx with A -> "a" | B -> "b" | C -> "c") in
      ErrorService.log err_msg err_val;
      ctx)

let () =
  let* a = MyService.get_a () |> or_log_error A in
  let* b = MyService.get_b a |> or_log_error B in
  match List.compare_length_with b 5 with
  | n when n > 0 -> Lwt_result.fail Z
  | _ ->
      let+ c = MyService.get_c b |> or_log_error C in
      c * 5

You could further un-nest by pulling the list length assertion out to a separate function, which may or may not improve readability – as you say, this is sometimes not worth it.


[^1]: The pedant in me must add that Lwt.t isn’t technically a monad.

13 Likes

[^1]: The pedant in me must add that Lwt.t isn’t technically a monad.

Maybe this is well known, but would you mind explaining why Lwt.t is not a monad?

2 Likes

thanks @CraigFe !

I generally avoid piping |>, but in this case the code does become more readable with less nesting

In the ‘real world’ use case I have a mixture of ('a, 'b) Lwt_result.t, 'a Lwt.t and ('a, 'b) result so I ended up having to open the different modules on different lines let open Lwt.Syntax and let open Lwt_result.Syntax, which made the code a lot less nice and made it more confusing - let* ... is now doing different things on the different lines, and you have to look up to see what is opened

The additional complication is that I have multiple nested match on the happy path as well, which becomes more and more indented

let open Lwt_result.Syntax in
let* a = get_a () |> or_log_error in
match a with
| A1 -> Lwt.return (Error ())
| A2 -> 
    let* b = get_b () |> or_log_error in
    let open Lwt.Syntax in
    let* x = get_x () in
    match b with
    | B1 -> Lwt.return (Error ())
    | B2 ->
      let open Lwt_result.Syntax in
      ...

And the final issue is I suppose with my mental model
In the imperative code

if(x) {
  return ...
}

I handle edge cases and ‘forget’ about them in the rest of the code.
In the ‘functional’ code with monads, bind and syntax extensions the types feel more confusing. Though that’s possible because I’m not used to this style of code, don’t know.

I don’t think those brackets are actually necessary. You could write it without them, e.g.

MyService.get_a () >>= function
| Error err -> ErrorService.log "error with a" err; Lwt.return (Error A)
| Ok value ->
MyService.get_b value >> function
| Error err -> ErrorService.log "error with b" err; Lwt.return (Error B)
| Ok value when List.length value > 5 -> Error Z
| Ok value ->
MyService.get_c value >>= function
| Error err -> ErrorService.log "error with c" err; Lwt.return (Error C)
| Ok value -> value * 5  (* Missing Lwt.return here? *)
6 Likes

To add onto talex’s point, I’d highly recommend your process your code with ocamlformat: it will format your code just like he did, in the optimal non-nested way most of us use. It can also reassure you about the fact you don’t need those brackets by removing them for you when they are superfluous.

Additionally, I personally prefer sticking to the let* monadic bindings operator by using the ones for the most general monad (in your example, Lwt_result) and wrapping other values in it. For instance if your code deals with Lwt_result.t values, and you want to insert some Result.t in your bindings chain, just Lwt.return it:

let* a = some_lwt_result_value () in
let* b = Lwt.return (some_result_value ()) in
...

Likewise if you have a Lwt.t value, you can Lwt.map ~f:Result.return it to make it compatible. If you have too many heterogeneous monads there’s no harm in changing your let* definition in a scope to match the correct one too.

4 Likes

We face the same difficulty in the Tezos code-base. I don’t think there is a well known, very satisfying solution to that. We have syntactically distinct operators (bind, return, fail) for the different monads so our problem is slightly different.

I’d recommend that you define a few additions to Stdlib.Result to help with moving from one monad to the other. E.g., Result.recover : ('b -> 'a) -> ('a, 'b) result -> 'a will help you go from Lwt+result into Lwt only.

Also, whilst @CraigFe has given a very good answer already, there are some things he did not mention in text. So here are a few comments about his code.

  • Pulling out the common pattern into a separate function is useful for readability, but you end up also pulling an error literal (A, B, C) out of a branching pattern. If you are in a more realistic case where the error is actually an evaluated expression you will evaluate it even in the success path. You can either (a) avoid pulling this into its own function or (b) wrap it in a lazy or a closure (fun () -> …).

  • The Lwt_result.map_err is doing the heavy lifting here. It is actually sufficient to avoid nesting altogether. (You could also use Result.iter_error to separate the logging from the erroring.)

    let () =
      let* a = MyService.get_a () in
      let* a = Lwt_result.map_err (fun e -> ErrorService.log "error with a" e; Error A) in
      <same for b and c>
    

    One of the issue you might encounter is that Result and Lwt_result have a different set of functions (e.g., Result.iter_error does not have a a counterpart in Lwt_result and Result.map_error's counterpart is spelled Lwt_result.map_err). If you start relying on these modules a lot, I’d recommend recoding your own to have a uniform view of results.

  • Do use List.compare_length_with when comparing lengths: it returns early if the list is longer than the integer.

4 Likes

thanks all, I tried all suggestions, adding the simple code for completeness
2 clarifications:

  • I am using ocamlformat with conventional formatting, I tried it with default and compact but I still get indentations
  • List.length was an arbitrary if check to showcase nesting

Setup

open Base

module Error_service = struct
  let log m s =
    Stdio.print_endline m;
    Stdio.print_endline s
end

module My_service = struct
  type a_error = A1 | A2

  let string_of_a_error = function A1 -> "a1" | A2 -> "a2"

  let get_a () : (int, a_error) Result.t Lwt.t = Lwt.return (Ok 5)

  type b_error = B1

  let string_of_b_error = function B1 -> "b1"

  let get_b x : (int list, b_error) Result.t Lwt.t = Lwt.return (Ok [ 2 * x ])

  type c_error = C1 | C2

  let string_of_c_error = function C1 -> "c1" | C2 -> "c2"

  let get_c x : (int, c_error) Result.t Lwt.t = Lwt.return (Ok (List.length x))
end

original version - lots of nesting

let nested_fn () : (int, unit) Result.t Lwt.t =
  let open Lwt.Syntax in
  let* a = My_service.get_a () in
  match a with
  | Error err ->
      Error_service.log "error with a" (My_service.string_of_a_error err);
      Lwt.return (Error ())
  | Ok value -> (
      let* b = My_service.get_b value in
      match b with
      | Error err ->
          Error_service.log "error with b" (My_service.string_of_b_error err);
          Lwt.return (Error ())
      | Ok value -> (
          if List.length value > 5 then Lwt.return (Error ())
          else
            let+ c = My_service.get_c value in
            match c with
            | Error err ->
                Error_service.log "error with c"
                  (My_service.string_of_c_error err);
                Error ()
            | Ok value -> Ok (value * 5) ) )

using Lwt_result.Syntax

let or_log_error m to_string ctx =
  Lwt_result.map_err (fun e -> Error_service.log m (to_string e)) ctx

let pipes_fn () : (int, unit) Result.t Lwt.t =
  let open Lwt_result.Syntax in
  let* a =
    My_service.get_a ()
    |> or_log_error "error with a" My_service.string_of_a_error
  in
  let* b =
    My_service.get_b a
    |> or_log_error "error with b" My_service.string_of_b_error
  in
  if List.length b > 5 then Lwt.return_error ()
  else
    let* c =
      My_service.get_c b
      |> or_log_error "error with c" My_service.string_of_c_error
    in
    Lwt.return_ok (c * 5)

same, but without pipes and handling the error inline

let map_err t f = Lwt_result.map_err f t

let no_pipes_fn () : (int, unit) Result.t Lwt.t =
  let open Lwt_result.Syntax in
  let a = My_service.get_a () in
  let* a =
    map_err a (fun e ->
        Error_service.log "error with a" (My_service.string_of_a_error e))
  in
  let b = My_service.get_b a in
  let* b =
    map_err b (fun e ->
        Error_service.log "error with b" (My_service.string_of_b_error e))
  in
  if List.length b > 5 then Lwt.return_error ()
  else
    let c = My_service.get_c b in
    let* c =
      map_err c (fun e ->
          Error_service.log "error with c" (My_service.string_of_c_error e))
    in
    Lwt.return_ok (c * 5)

infix operators - with ocamlformat I still got nested code, but even though I find infix operators less readable compared to syntax extensions - the use of function made it easy to follow

let log_error e m to_string =
  Error_service.log m (to_string e);
  Error ()

let infix_fn () : (int, unit) Result.t Lwt.t =
  let open Lwt.Infix in
  My_service.get_a () >>= function
  | Error e ->
      Lwt.return (log_error e "error with a" My_service.string_of_a_error)
  | Ok a -> (
      My_service.get_b a >>= function
      | Error e ->
          Lwt.return (log_error e "error with b" My_service.string_of_b_error)
      | Ok b -> (
          if List.length b > 5 then Lwt.return_error ()
          else
            My_service.get_c b >|= function
            | Error e -> log_error e "error with c" My_service.string_of_c_error
            | Ok c -> Ok (c * 5) ) )

summary of different types solutions:

  • map all monads to one type ('a Lwt.t -> ('a, ..) Result.t Lwt.t)
  • define different operators to different monads
  • use Result.map and Lwt_result.map, but define a common to the codebase naming convention (and argument order, I for example have a strong preference to JaneStreet’s t-first functions)

I might have misunderstood, do you mean that the map_error evaluation would be executed even in the success path?

The body of the function passed to map_error will not be evaluated in the success path. However, the argument you pass to the wrapper around map_error will be. So when you write MyService.get_a () |> or_log_error A, the A is evaluated. When it’s a literal (A, 0, …) it’s not an issue. But you might have more complex expressions there. E.g.,

  • maybe you need to allocated a structured error: or_log_error (A {attempts = !counter; max_attempt = limit; location = __LOC__; message = "Attempting to A but got an error})
  • maybe you need to pretty-print: or_log_error (AError (Printf.asprintf "Error whilst attempting A (attempt %d or %d)" !counter limit),
  • maybe you need to have side-effects (such as increasing a counter of errors) count the number of errors,
  • maybe you need to call into a function that may or may not do some of the above: or_log_error (new_a_error ()) (what does this function do??)

In these cases, you may want to consider delaying the evaluation of this parameter to avoid the computational cost and or the side-effects.

2 Likes