Skip to content

Fix current_exceptions when called with backtrace=false (#757). #758

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

Merged
merged 1 commit into from
Sep 24, 2021

Conversation

Sacha0
Copy link
Member

@Sacha0 Sacha0 commented Sep 23, 2021

Fixes #757, which contains a description of the issue, and strengthens associated tests. Best! :)

@Sacha0 Sacha0 requested review from c42f and NHDaly September 23, 2021 21:42
@codecov
Copy link

codecov bot commented Sep 23, 2021

Codecov Report

Merging #758 (ad62e78) into master (79af07b) will decrease coverage by 0.05%.
The diff coverage is 85.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #758      +/-   ##
==========================================
- Coverage   78.41%   78.35%   -0.06%     
==========================================
  Files           4        4              
  Lines         630      633       +3     
==========================================
+ Hits          494      496       +2     
- Misses        136      137       +1     
Impacted Files Coverage Δ
src/Compat.jl 78.30% <85.71%> (-0.07%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 79af07b...ad62e78. Read the comment docs.

which bears a backtrace keyword argument. When backtrace=false,
the returned ExceptionStack's stack field should be a Vector of
NamedTuples where the second (backtrace) element of each tuple is
nothing. Instead the Compat implementation throws,
e.g. on Julia 1.6.2:

julia> using Compat: current_exceptions

julia> try
           throw(UndefVarError(:cat))
       catch
           current_exceptions(;backtrace=false)
       end
ERROR: MethodError: no method matching getindex(::UndefVarError, ::Int64)
Stacktrace:
 [1] current_exceptions(task::Task; backtrace::Bool)
   @ Compat ~/pkg/Compat.jl/src/Compat.jl:1105
 [2] top-level scope
   @ REPL[7]:4

caused by: UndefVarError: cat not defined
Stacktrace:
 [1] top-level scope
   @ REPL[7]:2

The trick is that the implementation

function current_exceptions(task=current_task(); backtrace=true)
    stack = Base.catch_stack(task, include_bt=backtrace)
    ExceptionStack(Any[(exception=x[1],backtrace=x[2]) for x in stack])
end

passes the backtrace kwarg through to catch_stack which,
when include_bt=false, emits a Vector of exceptions, rather than
a Vector of two-tuples (exception-backtrace pairs). The array
comprehension on the next line expects a Vector of two-tuples, though,
and consequently ends up attempting to index into exceptions.

This commit constructs the ExceptionStack-wrapped stack Vector
appropropriately for backtrace=false, and strengthens the tests
to cover that case.
Copy link
Member

@c42f c42f left a comment

Choose a reason for hiding this comment

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

This looks great 👍

@Sacha0
Copy link
Member Author

Sacha0 commented Sep 24, 2021

Thanks for reviewing Chris! I hope you're doing well! :)

@Sacha0 Sacha0 merged commit 15191cb into master Sep 24, 2021
@Sacha0 Sacha0 deleted the sv-fix-curr-exc branch September 24, 2021 15:19
@martinholters martinholters mentioned this pull request Mar 18, 2025
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.

current_exceptions implementation fails when backtrace=false
2 participants