Question about caml_exn_with_js_backtrace special-casing some exceptions.

[Enoumy]

Hi!

I have a question about the way exceptions are handled in jslib.js here.

Specifically, I am hoping to understand the purpose of || exn[0] === 248.

function caml_exn_with_js_backtrace(exn, force) {
  //never reraise for constant exn
  if (!exn.js_error || force || exn[0] === 248)
    exn.js_error = new globalThis.Error("Js exception containing backtrace");
  return exn;
}

My understanding is that we are unconditionally setting js_error to a new error for OCaml exceptions whose first entry/tag is 248. My rough understanding is that these “248” exceptions are the “constant” exceptions (e.g. Stack_overflow,Not_found).

I think that this overrides the previous js_error field. In some situations, I think this loses the backtrace/information from where the exception was originally raised. Is this overriding behavior intentional/am I misunderstanding the boolean condition here? What is the purpose of always assigning a new js_error when the exception has a tag of 248?

I have a prototype benchmark that compares the impact of removing || exn[0] === 248 below. The benchmark raises a bunch of Not_found’s. The benchmark assumes that the libraries base, core, core_bench_js, and stdio exist and was compiled into a jsoo executable.

open! Core

module Benches = struct
  let table = Stdlib.Hashtbl.create 100

  let table_not_found =
    Core_bench_js.Test.create ~name:"Exception_benches.table_not_found" (fun () ->
      Fn.apply_n_times
        ~n:1_000
        (fun () ->
          let _ : _ =
            Fn.ignore
              (try Some (Stdlib.Hashtbl.find table 10_000) with
               | Stdlib.Not_found -> None)
          in
          ())
        ())
  ;;
end

let run_benches suites =
  List.iter suites ~f:(fun (name, benches) ->
    Stdio.print_endline ("running: " ^ name);
    Core_bench_js.bench benches)
;;

let () =
  run_benches [ "Exception benches table is not found", [ Benches.table_not_found ] ]
;;

The results of the benchmark before the change:

$ node benchmark_base_in_javascript.bc.js
running: Exception benches table is not found
Estimated testing time 10s (1 benchmarks x 10s). Change using '-quota'.
┌───────────────────────────────────┬──────────┬────────────┐
│ Name                              │ Time/Run │ Percentage │
├───────────────────────────────────┼──────────┼────────────┤
│ Exception_benches.table_not_found │   7.28ms │    100.00% │
└───────────────────────────────────┴──────────┴────────────┘

The results of the benchmark after removing || exn[0] === 248.

$ node benchmark_base_in_javascript.bc.js
running: Exception benches table is not found
Estimated testing time 10s (1 benchmarks x 10s). Change using '-quota'.
┌───────────────────────────────────┬──────────┬────────────┐
│ Name                              │ Time/Run │ Percentage │
├───────────────────────────────────┼──────────┼────────────┤
│ Exception_benches.table_not_found │   1.36ms │    100.00% │
└───────────────────────────────────┴──────────┴────────────┘

My rough understanding is that it would be safe to not override js_error in this scenario by not creating a new JavaScript error+stack trace, but I would first like to make sure that I am not misunderstanding something. Is there some purpose for treating 248 exceptions differently that - if the special casing were gone - might break something else? - some other purpose that I am not thinking about? I am also happy to provide additional details/info if helpful. Thanks!

Best,
Jose

[hhugo]

My understanding is that we are unconditionally setting js_error to a new error for OCaml exceptions whose first entry/tag is 248. My rough understanding is that these “248” exceptions are the “constant” exceptions (e.g. Stack_overflow,Not_found).

Correct

I think that this overrides the previous js_error field. In some situations, I think this loses the backtrace/information from where the exception was originally raised. Is this overriding behavior intentional/am I misunderstanding the boolean condition here? What is the purpose of always assigning a new js_error when the exception has a tag of 248?

The aim was to attached a js error to an ocaml exception if there was no previous js_error attached.
This logic didn't work well for constant exception because we would attach a js_error the first time and keep this js error even when calling raise_notrace.

We could try to remove this logic and remove the js_error if calling raise_notrace

[hhugo]

Also note that the next release won't enabled this feature by default because creating js errors is too expensive. See #1637

[Enoumy]

My apologies for the delayed response! Thanks for the quick response! I think that this sounds reasonable to me. I was not thinking about the risk of showing the same/stale js_error for constant exceptions. Thanks for the additional context. I think that my question is answered. I will follow up if I have any other questions.

Thanks,
Jose