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

Show exception with stack trace on send_message error #394

Merged
merged 3 commits into from
Mar 12, 2020

Conversation

yha
Copy link
Contributor

@yha yha commented Mar 9, 2020

Helps with JuliaGizmos/Interact.jl#290.
It doesn't solve that issue yet, since errors are still swallowed if produced in the initial position of the @manipulate slider.

@twavv
Copy link
Member

twavv commented Mar 9, 2020

Thanks for the PR.

Can we instead just include this in the @error invocation?

trace = sprint(io -> showerror(io, ex, stacktrace(catch_backtrace())))
@error(
    ...,
    trace,
)

@yha
Copy link
Contributor Author

yha commented Mar 10, 2020

I tried that initially, but passing the trace as a second argument like this shows it as an escaped string with \ns and no newlines, which is pretty hard to read.
The new commit adds the trace into the first argument, which seems to work fine.

@pfitzseb
Copy link
Member

The "right way" to do this is to pass an (exception, backtrace) tuple to @error:

julia> try
       0//0
       catch e
       @error("0//0", exception=(e, catch_backtrace()))
       end
┌ Error: 0//0
│   exception =
│    ArgumentError: invalid rational: zero(Int64)//zero(Int64)
│    Stacktrace:
│     [1] __throw_rational_argerror(::Type) at ./rational.jl:19
│     [2] Rational at ./rational.jl:14 [inlined]
│     [3] Rational at ./rational.jl:21 [inlined]
│     [4] //(::Int64, ::Int64) at ./rational.jl:44
│     [5] top-level scope at none:2

@yha
Copy link
Contributor Author

yha commented Mar 12, 2020

Right, I somehow missed that going through the @error docs. Updated the PR.

@twavv
Copy link
Member

twavv commented Mar 12, 2020

LGTM.

TravisCI failures are seemingly unrelated (though should be fixed).

@twavv twavv merged commit ee52ba2 into JuliaGizmos:master Mar 12, 2020
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.

3 participants