exn: preserve backtraces

[tatchi]

This PR changes Exn.fail to preserve backtraces

[tatchi]

I think we could also improveExn_lwt version. Here's what the doc says about Lwt.fail_with

val fail_with : string -> _ t
(** [Lwt.fail_with s] is an abbreviation for

{[
Lwt.fail (Stdlib.Failure s)
]}

    In most cases, it is better to use [failwith s] from the standard library.
    See {!Lwt.fail} for an explanation. *)

And Lwt.fail

val fail : exn -> _ t
(** [Lwt.fail exn] is like {!Lwt.return}, except the new {{!t} promise} that is
    {e already rejected} with [exn].

    Whenever possible, it is recommended to use [raise exn] instead, as [raise]
    captures a backtrace, while [Lwt.fail] does not. If you call [raise exn] in
    a callback that is expected by Lwt to return a promise, Lwt will
    automatically wrap [exn] in a rejected promise, but the backtrace will have
    been recorded by the OCaml runtime.

    For example, [bind]'s second argument is a callback which returns a promise.
    And so it is recommended to use [raise] in the body of that callback. This
    applies to the aliases of [bind] as well: [( >>= )] and [( let* )].

    Use [Lwt.fail] only when you specifically want to create a rejected promise,
    to pass to another function, or store in a data structure. *)

Given the following example code:

let main_lwt () =
  let fn_fail () = raise (Failure "inner fail") in
  let use f = 
    try%lwt f ()
    with exn -> Exn_lwt.fail ~exn "enriched" in
  use fn_fail

let () = Lwt_main.run (main_lwt ())

Here's the result with the current Exn_lwt.fail version:

me@spawnbox-febox-eu-corentinleruth ~/w/monorepo (develop) [2]> OCAMLRUNPARAM="b" dune exec experimental/coco/main.exe
Fatal error: exception Failure("enriched : Failure(\"inner fail\")")
Raised at Lwt.Miscellaneous.poll in file "src/core/lwt.ml", line 3123, characters 20-29
Called from Lwt_main.run.run_loop in file "src/unix/lwt_main.ml", line 27, characters 10-20
Called from Lwt_main.run in file "src/unix/lwt_main.ml", line 106, characters 8-13
Re-raised at Lwt_main.run in file "src/unix/lwt_main.ml", line 112, characters 4-13
Called from Dune__exe__Main in file "experimental/coco/main.ml", line 10, characters 9-35

We don't get much info from the stacktrace, it's almost all about lwt internals. Line 10 is the last line.

If we replace Lwt.fail_with with failwith:

me@spawnbox-febox-eu-corentinleruth ~/w/monorepo (develop) [2]> OCAMLRUNPARAM="b" dune exec experimental/coco/main.exe
Fatal error: exception Failure("enriched : Failure(\"inner fail\")")
Raised at Stdlib.failwith in file "stdlib.ml", line 29, characters 17-33
Called from Dune__exe__Main in file "experimental/coco/main.ml", line 10, characters 22-35

Not much better, lwt internals are removed.

Now using Exn.fail version from this PR

me@spawnbox-febox-eu-corentinleruth ~/w/monorepo (develop) [2]> OCAMLRUNPARAM="b" dune exec experimental/coco/main.exe
Fatal error: exception Failure("enriched : Failure(\"inner fail\")")
Raised at Dune__exe__Main.main_lwt.fn_fail in file "experimental/coco/main.ml", line 4, characters 19-47
Called from Lwt.Sequential_composition.backtrace_catch in file "src/core/lwt.ml", line 2077, characters 10-14
Re-raised at Dune__exe__Main.main_lwt.use.(fun) in file "experimental/coco/main.ml", line 6, characters 4-57
Re-raised at Devkit_core__Exn.fail.fails in file "vendor/devkit/exn.ml", line 42, characters 6-47
Called from Dune__exe__Main in file "experimental/coco/main.ml", line 10, characters 22-35

We get better stacktrace. So I'd be in favour of doing:

let fail = Exn.fail

[tatchi]

ping @rr0gi

[rr0gi]

Exn_lwt.fail change makes sense too

[Khady]

is it correct? What if exn wasn't the last exn raised?

[rr0gi]

you are right, it is not hard to come up with the case when it is not correct, thought i would argue most of real-world usecases should be ok. Did you see a specific case when this was wrong?

[Khady]

nothing specific. I know this is something we have faced over the years. But not particularly since this change landed.

[rr0gi]

but yes, it deserves a comment, @tatchi please

[tatchi]

#31