janestreet / base

Standard library for OCaml
MIT License
848 stars 124 forks source link

Please don't auto-register printers for exceptions defined outside of Base #146

Closed mjambon closed 1 year ago

mjambon commented 1 year ago

Problem

The Base library registers printers for exceptions from the standard library, including the Failure exception raised by failwith, without our consent. We don't want that. We need to control how we print exceptions. We don't use Base directly but it's an indirect dependency of our application.

How to reproduce

utop # failwith "x";;
Exception: Failure "x".

utop # #require "base";;

utop # failwith "x";;
Exception: (Failure x)
Raised at Stdlib.failwith in file "stdlib.ml", line 29, characters 17-33
Called from Stdlib__Fun.protect in file "fun.ml", line 33, characters 8-15
Re-raised at Stdlib__Fun.protect in file "fun.ml", line 38, characters 6-52
Called from Topeval.load_lambda in file "toplevel/byte/topeval.ml", line 89, characters 4-150

🙄

Similarly, if we happen to register our own exception printer too early, it gets clobbered by Base:

utop # Printexc.register_printer (function Failure msg -> Some ("Error: " ^ msg) | _ -> None);;
- : unit = ()

utop # failwith "x";;
Exception: Error: x

utop # #require "base";;

utop # failwith "x";;
Exception: (Failure x)
Raised at Stdlib.failwith in file "stdlib.ml", line 29, characters 17-33
Called from Stdlib__Fun.protect in file "fun.ml", line 33, characters 8-15
Re-raised at Stdlib__Fun.protect in file "fun.ml", line 38, characters 6-52
Called from Topeval.load_lambda in file "toplevel/byte/topeval.ml", line 89, characters 4-150

Desired behavior

Loading Base or any library that depends on it should not change how standard library exceptions are printed. Let application authors control how they print non-Base exceptions by having them run Base.Application.init () explicitly. More generally, don't alter the behavior of other libraries unless explicitly requested by the application author.

Current workaround

  1. We redefine exception handlers for Failure and such.
  2. We run our own init () function at the last moment to ensure that we register our exception printers after Base.
tdelvecchio-jsc commented 1 year ago

I tried reproducing this locally, but wasn't able to see that behavior. Could you give a more specific set-up / export and share the switch you are using with me to reproduce?

mjambon commented 1 year ago

This is with base.v0.14.3 (the result of some version constraints on some of our dependencies I don't know how to tell which package is holding us back actually we should be able to use base.v0.15.1 according to the actions required by opam install base.v0.15.1). I see it's fixed in base.v0.16.0 and base.v0.15.1. Sorry for the noise.