Improve Exception backtrace API

[Blacksmoke16]

Currently an exception's backtrace is an Array(String). This is usually fine, however there are some use cases where you may want to present the trace in a different way; such as pretty printing an exception. Currently this is not very easy to do as each portion of the trace needs to be manually parsed either manually or via an external shard.

I propose that the backtrace is also exposed an a like Array(Frame), where each Frame would have getters for the related info of that line. E.g. filename, method name, line/column etc. This would provide a much cleaner API to work with.

Given Crystal handles printing each line I would assume it inherently has access to each portion of data. This could be kept backwards compatible by exposing the frames as another method that the current one could leverage. E.g. @frames.map &.to_s.

[straight-shoota]

It looks Rust is currently (although for quite some time already) refactoring their backtrace API with a BacktraceFrame type (rust-lang/rust#79676, https://doc.rust-lang.org/std/backtrace/index.html, rust-lang/rust#53487). That could certainly serve as inspiration.

[Blacksmoke16]

I think it's worth pointing out the two PRs open about this are mutually exclusive. We should probably figure out which one we want to move forward with and go from there, versus needing to review both all the time...

[asterite]

True. Also probably discuss outside of here what to expose. What's the reason to expose the address and the cryptic function name?

[Blacksmoke16]

Is there reason to not expose the address? I guess my thinking was even if most people won't ever need it, still might be helpful to expose; especially if we're going for a 1:1 mapping between the backtrace string and a Frame instance.

Regarding the function name, I combined both names into 1 function ivar. Mainly because currently there isn't a case where you'd have both (https://github.com/crystal-lang/crystal/blob/master/src/exception/call_stack.cr#L199-L201) in a given backtrace string. So technically it is still exposed, but the API is the same.

[asterite]

Let's say someone implements an interpreter for Crystal. What would be the address?

[asterite]

Ah, nevermind, in any case it won't happen.

[straight-shoota]

Let's please focus the discussion on the proposed API here in this issue, instead of arguing over details in the competing PRs.

[straight-shoota]

I think we should start with ironing out the data format for a frame.

I've looked at Rust's implementation (https://doc.rust-lang.org/src/std/backtrace.rs.html#152-155) and they actually separate frame and symbols and multiple symbols can be associated with one frame. I don't know when you would have multiple symbols per frame. But there must be a reason for that.

The data structures (converted to the Crystal equivalent) look like this:

record BacktraceFrame,
  frame : RawFrame,
  symbols : Array(BacktraceSymbol)

record BacktraceSymbol,
  name : String?,
  filename : String?,
  line_number : Int32?,
  column_number : Int32?

The process is also split into two parts, first retrieving the raw frames from whatever backend is used to walk the stack. The result is a collection of BacktraceFrame with empty symbols. The second step resolves the frames against the debug information and fills the symbols.

[rdp]

Rust exposes the ip addresses. No idea who would use them though, but I guess since it's a struct people are free to ignore it. Crystal even has an "original_ip" but again not sure if anyone cares but might not hurt. The reason Rust does multiple symbols per frame is Normally there is only one symbol per frame, but sometimes if a number of functions are inlined into one frame then multiple symbols will be returned. The first symbol listed is the "innermost function", whereas the last symbol is the outermost (last caller). https://docs.rs/backtrace/0.3.2/backtrace/struct.BacktraceFrame.html Also, apparently there are two ways to lookup the function name. The dwarf has just the name and dlAddr has the name with "parameter types" which I think might be also useful...FWIW.

[Fryguy]

Not to derail the thread, but I just noticed @asterite's foreshadowing 😂 ☝️ #10681 (comment)

[asterite]

It's fine, we can leave the ip address as zero for interpreted mode. In the end backtraces in the interpreter are done in a completely different way, so it makes sense that the values there will also be slightly different.