Skip to content
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

What's needed to include ClearStacktrace.jl in Base? #36026

Closed
jkrumbiegel opened this issue May 25, 2020 · 99 comments
Closed

What's needed to include ClearStacktrace.jl in Base? #36026

jkrumbiegel opened this issue May 25, 2020 · 99 comments

Comments

@jkrumbiegel
Copy link
Contributor

jkrumbiegel commented May 25, 2020

The title says it all. I think I figured out a good format to present stack traces, and I think all users could benefit from it. It's currently in ClearStacktrace.jl and depends on Crayons.jl for the colors.

I want to know what I can do to make this PR-ready. I don't know how Julia Base handles REPL colors, which ones I'm allowed to use, how I access them without Crayons etc. I also don't know if there might be system-specific intricacies of stack traces that I have overlooked, testing this only a Windows and a Mac machine.

The basic idea is to have three columns, function, module and signature. Function and module together should basically always fit in a REPL horizontally, while signatures can be very long and are therefore allowed to overflow into new lines, while staying intact when resizing the REPL (compared to more complex but brittle table layouting). The code paths get a new line each, and are also allowed to overflow if they are too long. This keeps clickable links in Atom / VSCode and the like intact.

Just for reference, here is the before and after comparison from the README:
Before:
grafik
After:
grafik

@cossio
Copy link
Contributor

cossio commented May 25, 2020

This would be really nice to have. Or at least maybe a trimmed down version which improves printing of stacktraces by default in Base.

@JeffBezanson
Copy link
Sponsor Member

I'm all in favor. The main thing I don't get is the colors in the signatures. They seem...random? I guess the different colors are there to make it easier to pick out different elements, but it's a bit weird. How about color == nesting depth? I think the main weirdness is in e.g. Tuple{Symbol,Symbol} where the two Symbols are different colors.

@timholy
Copy link
Sponsor Member

timholy commented May 26, 2020

There's another thing I'd really like to see:

julia> foo(x::T, y::T) where T = error("whoops")
foo (generic function with 1 method)

julia> function bar()
           a = rand(3, 5)
           b = view(a, :, 2:3)
           c = reinterpret(Float32, b)
           foo(c, c)
       end
bar (generic function with 1 method)

julia> bar()
ERROR: whoops
Stacktrace:
 [1] error(::String) at ./error.jl:33
 [2] foo(::Base.ReinterpretArray{Float32,2,Float64,SubArray{Float64,2,Array{Float64,2},Tuple{Base.Slice{Base.OneTo{Int64}},UnitRange{Int64}},true}}, ::Base.ReinterpretArray{Float32,2,Float64,SubArray{Float64,2,Array{Float64,2},Tuple{Base.Slice{Base.OneTo{Int64}},UnitRange{Int64}},true}}) at ./REPL[1]:1
 [3] bar() at ./REPL[2]:5
 [4] top-level scope at REPL[3]:1

but since

julia> m = first(methods(foo))
foo(x::T, y::T) where T in Main at REPL[1]:1

julia> m.sig
Tuple{typeof(foo),T,T} where T

it seems that we should be able to print entry 2 as something like

[2] foo(::T, ::T) where T = Base.ReinterpretArray{Float32,2,Float64,SubArray{Float64,2,Array{Float64,2},Tuple{Base.Slice{Base.OneTo{Int64}},UnitRange{Int64}},true}}

@KristofferC
Copy link
Sponsor Member

A small comment, but to me it is currently maybe a bit too much color "salad". The stackframe counter being blue, all the different colors in the signature etc. For Base colors so far, we've only used the 16 system colors (8 + 8 bright ones) and that makes fancy coloring a bit harder.

@jkrumbiegel
Copy link
Contributor Author

jkrumbiegel commented May 26, 2020

Good suggestions overall. Maybe a couple comments why I made the color choices so far:

The numbers are blue because they are important, so shouldn't be grayed out, but making them white felt to me like they were fighting for attention with the function names.

The modules use the primary colors in a cycle, I think this is very useful because it allows to quickly filter the trace for relevant modules. This is something that is very to do with the current stack trace format. The idea is that people should be forced to read as little as possible to find what they’re looking for. (I mostly did eye movement and attention research in my academic life so far, so those aspects are important to me ;) )

The colors for the signatures are very debatable. The ones on the screenshot above were chosen to be only slightly different from dark gray. At the same time, I wanted the different semantic components to be discriminable. That’s why all types and type parameters (curly braces and commas) cause a color switch. There are three colors which means there won’t be two neighbors with the same color. I found that it’s quite difficult to make out single words in huge type signatures if they are all just white. The slightly different shades help a lot but also make everything a bit more noisy, visually.

As the 16 color set doesn’t have colors that are only marginally different from dark gray, it’s probably not something that can be included in Base. In a recent update, I replaced the colors with three slightly different shades of gray from the ANSI set, but even that will not be supported everywhere I guess.

@jkrumbiegel
Copy link
Contributor Author

Oh also the stack frame color and the module colors as well as the dark gray and white are all system colors. It’s just my particular VSCode theme that renders them like you see here.

@mcabbott
Copy link
Contributor

This would be great to have.

When scanning argument types, often it's pretty hard to figure out where one stops and the next starts. Colouring the top level differently might be very helpful here, and perhaps even numbering the arguments:

[3]   (_1::DataFrames.var"#fun##309#"{...lighter...}, _2::NamedTuple{...lighter...}, _3::Int64})

@JeffBezanson
Copy link
Sponsor Member

We should be able to show the argument names, like we do in the printing of methods(f).

@jkrumbiegel
Copy link
Contributor Author

how's this? I've replaced the signature colors with dark gray, but the :: is white so it's salient where argument types begin. especially helpful with the type monster at position 11
grafik

@JeffBezanson
Copy link
Sponsor Member

I like it. Will be even better with argument names.

It's too bad that the order "function - module - arguments" works so well for the layout but doesn't really make sense semantically. I don't have a solution though; clearly it's best for the function to come first. Maybe the module could go on the next line before the file location? It is part of the location after all. Then maybe we wouldn't need the header either.

I think we could also put the numbers in normal text, and the function names in bold and/or white.

@timotaularson
Copy link

timotaularson commented May 26, 2020

Could the filename at the end of the codepath (and possibly the line number) get their own lighter color too?

@jkrumbiegel
Copy link
Contributor Author

Maybe the module could go on the next line before the file location? It is part of the location after all. Then maybe we wouldn't need the header either.

I'll try out a couple things with these suggestions.

Could the filename at the end of the codepath (and possibly the line number) get their own lighter color too?

It technically can of course ;) It's always a tradeoff between better visibility and attention-grabbing noise.

@mcabbott
Copy link
Contributor

mcabbott commented May 26, 2020

Why not write the modules first, since they are part of the function's full name? Perhaps with colour it may be clear enough just to write Core.eval etc, or they could still be aligned as columns. (Or perhaps you tried this and it looked awful.)

@timholy
Copy link
Sponsor Member

timholy commented May 26, 2020

Really crazy thought: if printing the argument types would take up more than one line, print the signature in folded form and then use REPL.TerminalMenus to allow viewers to expand it. Something like:

julia> run_command(args...)
    Function    Arguments              Location
[1] f           (x::Int, y::Float64)   MyModule1, /path/to/file1.jl: 15
[2] g          +(...)                  MyModule2, /path/to/file2.jl: 64
...

julia> lasttrace()
# presents the above in menu form, allowing user to expand line 2

Related: #35915 (which I plan to use to create a folded-tree menu, https://github.com/JuliaCollections/FoldingTrees.jl).

@timotaularson
Copy link

How does it look with codepath rearranged in this order: LineNumber Filename Path
Then your eyes have a stable location to look for the line number and filename with no extra color needed for them (I seem to always be searching for the filename and line number buried in the details, can you tell?)
And for interactive REPL work I love the folded idea above :)

@jkrumbiegel
Copy link
Contributor Author

Why not write the modules first, since they are part of the function's full name

Yeah I kind of like the idea, I think I started out without color and that didn't work so well, but like this it's actually quite readable. I'll try without columns next, although columns are always nice because they guide your eyes.

grafik

@JeffBezanson
Copy link
Sponsor Member

Not sure about that; the function name is a vastly more important piece of information than the module. For me at least, the most important info is definitely the function name, file name, and line number. Also for scripts the left column would be lots of Main or blank, which is not ideal for that use case.

@ericphanson
Copy link
Contributor

I think when I’m reading stacktraces, I’m mostly looking for the last call in my code before it goes to Base/package code, since usually the bug is in my code. So in that case the module is pretty important and the function call isn’t what I’m scanning first.

@JeffBezanson
Copy link
Sponsor Member

How about this:

[1]  get_ticklabels   ::AbstractPlotting.Automatic, ::Int64
     in MakieLayout at ~/.julia/packages/MakieLayout/COfBu/src/lineaxis.jl:372

Module names would still line up so they would be easy to scan.

@jkrumbiegel
Copy link
Contributor Author

jkrumbiegel commented May 26, 2020

I had actually just tried out something close to that. I colored the function names themselves this time, so they are still the most salient but carry the Module information in their color. So there's still an obvious grouping, but with less focus on all the module names.

grafik

@JeffBezanson
Copy link
Sponsor Member

The first time I saw that I'd probably go nuts trying to figure out what the colors meant 😂 If there are going to be per-module colors, seems better to just apply the color to the module name.

@mcabbott
Copy link
Contributor

I like the module names, they get a bit lost in this latest picture.

If they are at the bottom, perhaps making them the same bright colour would connect things? (And make clear where you cross a module boundary.) And, perhaps, if the module name appears in the path, it could simply be highlighted there instead -- that would also move the colourful name off the left edge where the functions are.

@KristofferC
Copy link
Sponsor Member

For some inspiration, I post a few images from the last time tweaking the stacktrace formatting came up.

bild

bild

@antoine-levitt
Copy link
Contributor

Often the exact package is not so much of interest as the nature of the code: is it core julia, a library I'm using, the library I'm developing, or a script? We could have four colors: core/base/dev/stdlib, packages, packages currently ]deved, and all the rest (including script/REPL). Some of these distinctions are pretty arbitrary and the classification is a bit brittle, but coloring the method based on this would (at least for me) maximize the information without displaying any extra text and keeping a small, easy to memorize set of colors.

@jkrumbiegel
Copy link
Contributor Author

For me, the colors are more about boundary changes, where code from one module begins and the other ends. That's why I probably wouldn't assign inherent meaning to them. On the other hand, that might be confusing.

I've tried to simplify more, and removed columns again because without the module column they are not so useful anymore. Then first I colored only line number by modules, but this still left the file name not so visible, which many comments said is important. So I colored that in the module color as well. The module name itself is not colored because it's too close to the function name and that's too noisy.

Here is the version with both numbers and filenames colored:

grafik

@jkrumbiegel
Copy link
Contributor Author

here without filenames colored

grafik

@kmsquire
Copy link
Member

Can you also try coloring the full file path?

@timotaularson
Copy link

Some ideas...
Filename and line number in a consistent location with double spaces to set them off.
Stack level color and module color match each other to visually pair the related lines.
Filename, line number, and path in slightly different hue than parameter types to distinguish without clutter.
stacktrace
[Colors in this example are arbitrary, more thinking about how their positions relate.
Missing stack levels in example and shortened parameter type list on last item are because I typed example by hand.]

@timotaularson
Copy link

Code used to produce the sample above in case anyone wants to play with it:

function main()

    errors = [
              ("1", "get_ticklabels", ("AbstractPlotting.Automatic", "Int64"), "372", "lineaxis.jl", "MakieLayout", "~/.julia/packages/MakieLayout/COfBu/src")
              ("2", "get_ticklabels", ("AbstractPlotting.Automatic", "Int64"), "351", "lineaxis.jl", "MakieLayout", "~/.julia/packages/MakieLayout/COfBu/src")
              ("3", "#105", ("AbstractPlotting.Automatic",), "152", "lineaxis.jl", "MakieLayout", "~/.julia/packages/MakieLayout/COfBu/src")
              ("8", "OnUpdate", ("Tuple{Float32,Float32}",), "218", "Observables.jl", "Observables", "~/.julia/packages/Observables/0wrF6/src")
              ("9", "#setindex!#5", ("Observables.var\"#6#8\"",), "138", "Observables.jl", "Observables", "~/.julia/packages/Observables/0wrF6/src")
              ("10", "setindex!", ("Observables.Observable{Any}",), "126", "Observables.jl", "Observables", "~/.julia/packages/Observables/0wrF6/src")
              ("11", "#LineAxis#95", ("Base.Iterators.Pairs{Symbol,Observables.Observable,NTuple{28,Symbol},NamedTuple{(:endpoints, :limits, :flipped, :ticklabelrotation, :ticklabelalign, :lables
ize, :labelpadding, :ticklabelpad, :labelvisible, :label, :labelfont, :ticklabelfont, :ticklabelcolor}",), "270", "lineaxis.jl", "MakieLayout", "~/.julia/packages/MakieLayout/COfBu/src/lobjects")
    ]

    println()
    for (idx, err) in enumerate(errors)
        # Module color
        mc = idx <= 3 ? :light_red : (idx >= 7 ? :light_red : :light_yellow)
        # Path color
        pc = :blue
        printstyled("[", color=mc)
        printstyled("$(err[1])", color=mc)     # errorno
        printstyled("]  ", color=mc)
        printstyled("$(err[5])", color=pc)     # filename
        printstyled(":", color=pc)             # colon
        printstyled("$(err[4])", color=pc)     # lineno
        printstyled("  $(err[7])", color=pc)   # path
        println()
        printstyled("$(err[6]) ", color=mc)    # module
        printstyled("$(err[2]) ", color=:bold) # function
        printstyled("(", color=:light_blue)    # param types
        for t in err[3]
            printstyled("::", color=:white)
            printstyled("$t", color=:light_blue)
        end
        printstyled(")", color=:light_blue)
        println()
    end
end

@jkrumbiegel
Copy link
Contributor Author

The problem with this is that this doesn't leave the links clickable in Atom / VSCode, etc. This is something I use very often and I believe others do as well. In fact, I even explicitly expand the base paths to make them clickable. This does of course print more. But I think it increases utility. I know there's some way to jump to stack trace entries by number in the REPL with some shortcut, but this is far less intuitive than just clicking on a link in my opinion.

@StefanKarpinski
Copy link
Sponsor Member

Agree: let’s just do that last version.

@mcabbott
Copy link
Contributor

mcabbott commented Jun 1, 2020

Thanks for doing all the work here, @jkrumbiegel . Great to have a version we can try out...

And fork: I guess I think it's a bit concerning that rand(5) .* rand(7) plus error occupies 35 lines? So I made https://github.com/mcabbott/ClearStacktrace.jl/tree/milder to try out. (Plus colour issues etc discussed above.) This is almost #36026 (comment) with more colours.

@JeffBezanson
Copy link
Sponsor Member

Note in the current stacktrace printing frames 8-11 in that example are not shown (they are part of the REPL and would be in every REPL stacktrace).

@mcabbott
Copy link
Contributor

mcabbott commented Jun 1, 2020

Indeed, this has improved which is great. But it still goes from 8 lines (for the stack trace alone) to 20 (ClearStacktrace 0.2). There's a bit of a trade-off between how pretty it is & not losing your place.

The paths are also printed much more compactly in Base, ./broadcast.jl:495 instead of //Applications/Julia-1.5.app/Contents/Resources/julia/bin/../share/julia/base/broadcast.jl:495, this would also reduce the need for empty lines.

@jkrumbiegel
Copy link
Contributor Author

The base paths are expanded on purpose to make them clickable. You can disable that in ClearStacktrace. I could also make the newlines optional for people who don’t like them. Could just be an environment variable.

@jkrumbiegel
Copy link
Contributor Author

And I guess I must have missed copying the part of the function that clips off the last couple of frames that never change

@singularitti
Copy link
Contributor

I've moved away from using Crayon.jl, now that I don't actually use complex colors anymore. Helps with the load time of the package, too. I found a way to enhance the visibility of file name and line number without it becoming visually overwhelming by underlining in dark grey. That looks sensible, too, as we're used to paths / links being underlined. White or other highlighting was too strong there, bold too weak.

Also, I've switched to light colors as the first colors in the cycler, which I should have done in the first place, but they look the same in my theme so I never noticed. This should be better for themes where dark blue is hardly visible (that's the theme's fault though).

I've registered this style as version 0.2 in ClearStacktrace.jl so we can try it out a bit more.

Two examples:

example 1 example 2

Curious about why there is an extra / in //Applications/Julia-1.4.app/?

@jkrumbiegel
Copy link
Contributor Author

Probably a bug from splitting and rejoining the path

@jkrumbiegel
Copy link
Contributor Author

I've moved away from using Crayon.jl, now that I don't actually use complex colors anymore. Helps with the load time of the package, too.

Did it cause a significant load time? It loads pretty quickly on its own for me


julia> @time using Crayons

  0.014410 seconds (22.60 k allocations: 2.274 MiB)

No, I mostly removed it because I wouldn’t have it in base :) Load time was just a guess

@singularitti
Copy link
Contributor

I guess there might be too many blank lines if we have many frames of stacktrace as shown in #36026 (comment) ?

@KristofferC
Copy link
Sponsor Member

KristofferC commented Jun 2, 2020

Not to derail the discussion (the last version is great and a big improvement) but on the topic of too many newlines, the problem is that when working at the terminal it seems a lot better to me to print the stacktrace "inverted" (as I originally proposed in #18228) as:

...
[3] frame
[2] frame
[1] frame
Error: Some error happened
blah blah

The most important information is the error message itself and the frames towards the top of the stack (closer to the error) and printing it in this order that will always be visible without scrolling. Right now I frequently have to scroll up to even see the error message and not just the tail of a stacktrace.

However, when reading a stacktrace on a webstite that has been copy-pasted from the terminal you want the opposite order because you scroll from top to bottom as opposed to bottom to top as you do in a terminal... Kinda tricky to get a good design for both these cases.

@jkrumbiegel
Copy link
Contributor Author

jkrumbiegel commented Jun 2, 2020

However, when reading a stacktrace on a webstite that has been copy-pasted from the terminal you want the opposite order because you scroll from top to bottom as opposed to bottom to top as you do in a terminal... Kinda tricky to get a good design for both these cases.

I actually had code in ClearStacktrace.jl for a moment that allowed me to reprint the last stacktrace. I had envisioned that for cutting off super long types at some character maximum and being able to reprint in full if the whole information was needed. But your use case would also be interesting. I can imagine a reprint(inverted = true) or even reprint(html = true) where it would print a html version that would retain coloring for pasting into a website.

Also, I agree that given the scrolling direction it could make sense to invert the whole thing by default.

@c42f
Copy link
Member

c42f commented Jun 3, 2020

ipython prints frames in the opposite order, and despite not having to scroll I've always found it inexplicably confusing. Perhaps that's just due to my prior experience with gdb and other systems where the innermost frame is at the top, or perhaps other people feel this way too?

Speaking of gdb, they have a reasonable solution to very long traces with their pager: "press enter for more frames".

@c42f
Copy link
Member

c42f commented Jun 3, 2020

By the way, I Iove the latest visual design in #36026 (comment) and would be extremely happy if we merge that. Changing the frame order or adding interactive features seem like separate issues.

Regarding file names, I hope that we can eventually use terminal hyperlink OSC sequences (yes hyperlinks in terminals are somewhat widely supported!) and have a way for the user's editor to pick that up.

@ggggggggg
Copy link
Contributor

ggggggggg commented Jun 3, 2020

Speaking of finding the "innermost frame", I use enough languages in the course of my work that I can never remember if I should be looking at the top or the bottom of a stacktrace for my code in any particular language. So I end up scanning through looking at the filename until I see one I recognize. The underlining shown here would help, so this is a clear improvement. But I still wonder if there is a goodway to call out if I should be looking at the top or the bottom. In principle printing YOUR CODE HERE on one end and OTHER CODE HERE on the other end would help me, but it doesn't seem very elegant.

@jkrumbiegel
Copy link
Contributor Author

I've made a PR to Base here #36134
It works as far as I can tell, but I'll need some help to make it ready for merging

@ChrisRackauckas
Copy link
Member

Is there a way to omit type parameters? The problem we see a lot of the time is that the amount of type parameters in DiffEq, ForwardDiff, etc. can make things... daunting. If by default it just said Dual, except in the case where there is dispatch to other methods due to type parameters, then I think you'd reduce 90% of the noise in most stack traces I read (and in the other cases, @timholy 's suggestion is really what's required, since it's usually about making types or matching type parameters)

@vtjnash
Copy link
Sponsor Member

vtjnash commented Jul 2, 2020

If they correspond to some existing type alias, they'll just go away automatically now (#36107). Otherwise, they indicate possible compilation performance bottlenecks—so possibly worth investigating?

@KristofferC
Copy link
Sponsor Member

This is done now.

@ChrisRackauckas
Copy link
Member

For some people, but most people just get confused when it prints out a type that would take 3 pages of printed paper, so it should probably be something that's opt-in. I'll open up another issue to discuss that.

@ChrisRackauckas
Copy link
Member

Continued in #36517

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests