New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Backtraces truncated with 4.02 #6556
Comments
Comment author: @mshinwell As a further example, Core_kernel has a "result" type, with a [try_with] function thus: let try_with f = With 4.02 any caller of [Result.try_with] that wishes to propagate the exception by raising it, preserving the backtrace of [exn] from [f] through to [try_with], has to remember to use %reraise. |
Comment author: @mshinwell (This might be an argument for putting backtraces in exception values themselves, which would probably also solve the lack of precision in retrieval of backtraces mentioned in mantis #6554.) |
Comment author: @mshinwell Another case that is apparently not considered a re-raise: match expr with |
Comment author: @lpw25 Something that I've wanted in the past was to extend the notion of a reraise to include raising a different exception. For example,
could result in a backtrace like Raised at file "foo.ml", line 1, characters 18-22 This would then allow us to treat any raise that is syntactically within a This would fix the reported example, as well as make it easier to debug programs like the compiler, which wrap their exceptions multiple times as they escape. |
Comment author: @alainfrisch A first step would already be to export an explicit "reraise" function in Pervasives. The current heuristic used by the compiler to turn "raise" into "reraise" is mostly implemented to preserve backward-compatibility, for simple cases, but it seems fair to me to ask users to be explicit about a raise being actually a "reraise" (meaning that the stacktrace should be preserved, if possible). |
Comment author: @lpw25
Either way, I would like to be able to reraise a different exception and have it printed correctly in the backtrace. |
Comment author: @alainfrisch
Currently, "reraise" is considered as a simple "raise" if the exception is not physically equal to the previous one (see caml_stash_backtrace in byterun/backtrace.c and asmrun/backtrace.c). It would be easy to drop this test, with an increased risk of creating wrong traces (i.e. append to a previous, unrelated trace). This would not be enough to satisfy your request, because you also want to record all exception names in the trace. It should not be overly difficult, when the new exception isn't equal to the previous one, to record the name in the trace (in stash_backtrace). This would change the internal representation of traces, but luckily, the concrete representation is noy exposed in Printexc. In your example: try foo () with Not_found -> raise (Error "Not found") ---> Raised at file "foo.ml", line 1, characters 18-22 Called from file "foo.ml", line 4, characters 6-11 Re-raised as Error _ at file "foo.ml", line 7, characters 10-11 Called from file "foo.ml", line 9, characters 13-20 wouldn't it be more relevant to keep the old exception (Not_found) in the trace? |
Comment author: @gasche
There are many reasons for wrapping a failure into another exception which carries more information (some have been discussed in the coq-related PR; another concrete use-case I have is a parser that catches exceptions coming from semantic actions and rewrap them in a richer exception containing the location of the production). I think the suggestions to
are both good ideas. |
Comment author: @alainfrisch I was not arguing against the feature, but in Leo's example, the displayed trace shows "Error", which is the name of the uncaught exception, so the information is redundant. What is lost in the reraise is the name of the original exception (Not_found); my point is that it seems more useful to keep that name in the trace. |
Comment author: @gasche Sorry, I misunderstood indeed. We could display the name of the exception being raised on all "R(er-)?aised" lines -- preferably at the end to avoid disturbing tooling that parses raise locations, or even on its own line before. |
Comment author: @lpw25
Oh yes, I see what you mean. I guess I was thinking that the original exception would be displayed in the "Uncaught exception" message, but obviously that's incorrect. How about something like: Uncaught exception Error _ Raised as Not_found at ... with the "as Not_found" only appearing if there has been a changing reraise. |
Comment author: @lpw25 Related to some of the discussion of exceptions in Coq, I wonder whether it would also be a good idea to let users insert additional information into backtraces. Even if the information was just strings it might improve debugging in the presence of complicated control flow (e.g. Lwt and Async). For example try to give: Raised as Not_found at ... Perhaps it is not worth the effort, but it might be useful. |
Comment author: @alainfrisch
Would that subsume your proposal of keeping names of previous exceptions? This would require users to manually insert the old exception name as one of these extra information, but at least users that don't need this feature (but want to re-raise a different exception and keep the same trace) wouldn't have to pay the extra tiny cost of it. |
Comment author: @lpw25
I meant in addition to it. Reraising a different exception is the most common case, so it would be good to support it as easily as possible. I just thought that in more complex cases the additional power of arbitrary strings might be useful. |
Comment author: @lpw25
Since we already statically detect when syntactically the same exact exception value is used, this cost could easily be optimised away by the code for compiling the "%reraise" primitive. |
Comment author: @diml
I don't think that would help much for Lwt or Async. The main problem with backtraces and Lwt/Async is that the backtrace is global and will always be destroyed by another thread. |
Comment author: @lpw25
Note that this was fixed some time before 4.04.0 |
Original bug ID: 6556
Reporter: @diml
Assigned to: @gasche
Status: resolved (set by @diml on 2018-05-28T14:03:29Z)
Resolution: fixed
Priority: normal
Severity: minor
Version: 4.02.0
Category: standard library
Monitored by: @Drup @gasche @ygrek @hcarty @yakobowski
Bug description
In the following code, uncommenting the [let e = e in] produces a smaller backtrace:
let f2 () = raise Exit
let f1 f2 = f2 ()
let g f1 f2 =
try f1 f2
with e ->
(* let e = e in *)
raise e
let () = try g f1 f2 with _e -> Printf.eprintf "%s\n%!" (Printexc.get_backtrace ())
With [let e = e in] commented:
Raised at file "toto.ml", line 1, characters 18-22
Called from file "toto.ml", line 4, characters 6-11
Re-raised at file "toto.ml", line 7, characters 10-11
Called from file "toto.ml", line 9, characters 13-20
with [let e = e in] uncommented:
Raised at file "toto.ml", line 7, characters 10-11
Called from file "toto.ml", line 9, characters 13-20
Looking at the -dlambda it's easy to understand why: without the [let e = e in] the raise is turned into a reraise.
I remember there was a discussion about this, so it might have been said that this behavior was acceptable but I have a feeling it's going to make a lot of people unhappy.
The text was updated successfully, but these errors were encountered: