Skip to content

Conversation

@BioTurboNick
Copy link
Contributor

Split off from #40537, allows the last exception to be reprinted in the REPL.

@mcabbott
Copy link
Contributor

Is it possible that err, and in fact ans, should have a value on startup? This would prevent this:

 _/ |\__'_|_|_|\__'_|  |  Official https://julialang.org/ release
|__/                   |

julia> ans(x) = "function";

julia> 1+1
ERROR: invalid redefinition of constant ans

julia> 2;
ERROR: invalid redefinition of constant ans

in favour of this:

 _/ |\__'_|_|_|\__'_|  |  Official https://julialang.org/ release
|__/                   |

julia> 1+1
2

julia> ans(x) = "function";
ERROR: cannot define function ans; it already has a value

@StefanKarpinski
Copy link
Member

I do like err as it feels analogous to ans, even if errs may be more accurate. Perhaps we should be a bit more delicate about setting both ans and err if it already has a value that cannot be overwritten.

@BioTurboNick BioTurboNick changed the title Add errs global to REPL to store most recent errors Add err global to REPL to store most recent errors May 3, 2021
@antoine-levitt
Copy link
Contributor

Using the awesome AbbreviatedStackTraces, I find myself very often typing "errs" instead of "err". This creates a new error, and sends my previous stacktrace to oblivion. Not sure what if anything should be done to prevent against that, but I'm just putting it out there in case someone has an idea.

@BioTurboNick
Copy link
Contributor Author

The same thing happens with ans but that might be less of an issue than losing error info.

Could it be a vector of exceptions that is pushed into each time? Maybe just keep the last 5 to avoid unending growth in a long session?

Though wouldn't be as nice to dump 5 full traces at once accidentally either.

@BioTurboNick
Copy link
Contributor Author

BioTurboNick commented Jun 7, 2021

Maybe stack traces originating right at top level don't get stored? Or just those errors that Julia doesn't display traces for normally. (maybe that's the same thing)

@antoine-levitt
Copy link
Contributor

Could it be a vector of exceptions that is pushed into each time? Maybe just keep the last 5 to avoid unending growth in a long session?
Though wouldn't be as nice to dump 5 full traces at once accidentally either.

Could be good: err for the latest, errs for an array of the latest. You can avoid dumping them all by having custom printing for the errs object and displaying "array of last 3 errors", which you could then do errs[i] on

@BioTurboNick
Copy link
Contributor Author

@antoine-levitt - I'm testing out, both here and in AbbreviatedStackTraces.jl, the idea that err is only set when the stack goes deeper than top-level. That should solve the typo trap you mentioned and I think is limited enough to avoid losing anything you wouldn't want to lose.

@BioTurboNick
Copy link
Contributor Author

Now that 1.7 is just about out, wanted to raise this PR again.

Summarizing:

  • err global on the REPL
  • Trivial errors (where there is no stack trace, occurs at top level) aren't stored, which helps prevent issues like flubbing an attempt to consume it.

I believe the one outstanding previously-mentioned issue might be ensuring err is set when the REPL starts, so that it is reserved. (And if so, same should be done for ans presumably.) Should I include that?

Is there anything else?

Copy link
Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm very excited about this, and every couple weeks wonder why it isn't merged yet. Thanks for getting back to it.

@vtjnash
Copy link
Member

vtjnash commented Oct 13, 2021

Could it be a vector of exceptions that is pushed into each time

I don't want it to be added to this PR, but I think having another variable that is all of the previous results, errors, and expressions would be excellent.

@BioTurboNick
Copy link
Contributor Author

I think that does it. I also noticed that the display_error(er, bt) methods don't seem to be used anywhere anymore. Assuming tests prove that out, and since the method isn't documented, removing them is probably okay? But waiting to see if the tests confirm that.

@BioTurboNick
Copy link
Contributor Author

BioTurboNick commented Oct 29, 2021

Hmm... Lots of random test failures but one consistent one. Somehow a precompile test is failing. Unfortunately the test failure message is a bit unclear as to what was expected vs. actual.

Error in testset precompile:
Test Failed at /buildworker/worker/package_linuxaarch64/build/usr/share/julia/stdlib/v1.8/Test/src/Test.jl:811
  Expression: contains_warn(read(fname, String), $(Expr(:escape, :(r"LoadError: break me\nStacktrace:\n \[1\] [\e01m\[]*error"))))

EDIT: Oh, it's just that it's not scrubbed of REPL frames now, which adds a double-digit frame and indents the whole output by an additional space.

@BioTurboNick
Copy link
Contributor Author

That should do it. Restored scrubbing of REPL frames everywhere.

@BioTurboNick BioTurboNick requested a review from vtjnash November 4, 2021 16:11
Copy link
Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good to me, but need to put back the old functions (they don't need the scrub_repl_backtrace work), since there are many packages that have been relying on them.

@vtjnash vtjnash self-requested a review November 5, 2021 20:54
Co-authored-by: Jameson Nash <[email protected]>
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

Successfully merging this pull request may close these issues.

10 participants