Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0006556OCamlstandard librarypublic2014-09-11 17:372018-05-29 10:59
Reporterdim 
Assigned Togasche 
PrioritynormalSeverityminorReproducibilityalways
StatusresolvedResolutionfixed 
PlatformOSOS Version
Product Version4.02.0 
Target VersionFixed in Version 
Summary0006556: Backtraces truncated with 4.02
DescriptionIn 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.


TagsNo tags attached.
Attached Files

- Relationships

-  Notes
(0012116)
shinwell (developer)
2014-09-11 17:45
edited on: 2014-09-11 17:56

As a further example, Core_kernel has a "result" type, with a [try_with] function thus:

let try_with f =
  try Ok (f ())
  with exn -> Error exn

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.

(0012117)
shinwell (developer)
2014-09-11 17:54
edited on: 2017-02-24 13:48

(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 0006554.)

(0012118)
shinwell (developer)
2014-09-11 20:32

Another case that is apparently not considered a re-raise:

match expr with
| v -> v
| exception exn -> raise exn
(0012629)
lpw25 (developer)
2014-12-03 16:55

Something that I've wanted in the past was to extend the notion of a reraise to include raising a different exception. For example,

    try
      foo ()
    with Not_found -> raise (Error "Not found")

could result in a backtrace like

  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

This would then allow us to treat any raise that is syntactically within a `with` clause as a reraise.

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.
(0012635)
frisch (developer)
2014-12-03 18:04

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).
(0012642)
lpw25 (developer)
2014-12-03 18:56

> 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).

Either way, I would like to be able to reraise a different exception and have it printed correctly in the backtrace.
(0012648)
frisch (developer)
2014-12-04 11:27

> Either way, I would like to be able to reraise a different exception and have it printed correctly in the backtrace.

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?
(0012650)
gasche (administrator)
2014-12-04 12:06

> wouldn't it be more relevant to keep the old exception (Not_found) in the trace?

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

- allow to explicitly require a reraise
- in that case, append to the backtrace even when the exception differs

are both good ideas.
(0012651)
frisch (developer)
2014-12-04 12:14

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.
(0012652)
gasche (administrator)
2014-12-04 13:12
edited on: 2014-12-04 13:13

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.

(0012653)
lpw25 (developer)
2014-12-04 13:53

> wouldn't it be more relevant to keep the old exception (Not_found) in the trace?

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 ...
  Called from ....
  Rerasied as Error _ at ...
  Called from ...

with the "as Not_found" only appearing if there has been a changing reraise.
(0012654)
lpw25 (developer)
2014-12-04 14:04

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
    spawn_thread ()
   with exn ->
     Printexc.attachf "Spawned from thread %s" thread_name;
     reraise exn

to give:

  Raised as Not_found at ...
  Called from ....
  Spawned from thread foo
  Called from ...

Perhaps it is not worth the effort, but it might be useful.
(0012656)
frisch (developer)
2014-12-04 14:12

> be a good idea to let users insert additional information into backtraces.

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.
(0012657)
lpw25 (developer)
2014-12-04 14:15

> Would that subsume your proposal of keeping names of previous exceptions?

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.
(0012658)
lpw25 (developer)
2014-12-04 14:18

> 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.

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.
(0012660)
dim (developer)
2014-12-04 15:48

> 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

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.
(0018009)
lpw25 (developer)
2017-06-28 12:46

> Another case that is apparently not considered a re-raise:
>
> match expr with
> | v -> v
> | exception exn -> raise exn

Note that this was fixed some time before 4.04.0
(0019144)
dim (developer)
2018-05-28 16:03

I think this is fixed in 4.07 (since GPR#1567).
(0019145)
trefis (manager)
2018-05-29 10:59

I don't think so, GPR#1567 seems mostly unrelated to what is discussed here.

- Issue History
Date Modified Username Field Change
2014-09-11 17:37 dim New Issue
2014-09-11 17:45 shinwell Note Added: 0012116
2014-09-11 17:46 shinwell Status new => acknowledged
2014-09-11 17:54 shinwell Note Added: 0012117
2014-09-11 17:56 shinwell Note Edited: 0012116 View Revisions
2014-09-11 17:56 shinwell Note Edited: 0012116 View Revisions
2014-09-11 20:32 shinwell Note Added: 0012118
2014-09-14 21:26 doligez Target Version => 4.02.2+dev / +rc1
2014-12-03 16:55 lpw25 Note Added: 0012629
2014-12-03 18:04 frisch Note Added: 0012635
2014-12-03 18:56 lpw25 Note Added: 0012642
2014-12-04 11:27 frisch Note Added: 0012648
2014-12-04 12:06 gasche Note Added: 0012650
2014-12-04 12:14 frisch Note Added: 0012651
2014-12-04 13:12 gasche Note Added: 0012652
2014-12-04 13:13 gasche Note Edited: 0012652 View Revisions
2014-12-04 13:53 lpw25 Note Added: 0012653
2014-12-04 14:04 lpw25 Note Added: 0012654
2014-12-04 14:12 frisch Note Added: 0012656
2014-12-04 14:15 lpw25 Note Added: 0012657
2014-12-04 14:18 lpw25 Note Added: 0012658
2014-12-04 15:48 dim Note Added: 0012660
2015-01-15 19:17 doligez Target Version 4.02.2+dev / +rc1 => 4.03.0+dev / +beta1
2016-04-12 15:07 doligez Target Version 4.03.0+dev / +beta1 => 4.03.1+dev
2016-12-08 15:51 shinwell Category OCaml backend (code generation) => OCaml general
2017-02-16 14:00 doligez Target Version 4.03.1+dev => undecided
2017-02-23 16:36 doligez Category OCaml general => -OCaml general
2017-02-24 13:48 doligez Note Edited: 0012117 View Revisions
2017-02-24 13:50 doligez Assigned To => gasche
2017-02-24 13:50 doligez Status acknowledged => assigned
2017-02-24 13:50 doligez Category -OCaml general => standard library
2017-02-24 13:50 doligez Target Version undecided =>
2017-06-28 12:46 lpw25 Note Added: 0018009
2018-05-28 16:03 dim Note Added: 0019144
2018-05-28 16:03 dim Status assigned => resolved
2018-05-28 16:03 dim Resolution open => fixed
2018-05-29 10:59 trefis Note Added: 0019145


Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker