Skip to content

Throw CapturedExceptions from asyncmap to avoid dropping the stacktrace #42105

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.

Sign up for GitHub

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 9 commits into from
Jan 8, 2022
Merged

Throw CapturedExceptions from asyncmap to avoid dropping the stacktrace #42105

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
15db473
Print stacktraces from asyncmap'd tasks
ericphanson Sep 3, 2021
d78d6ee
better fix
ericphanson Dec 28, 2021
aa1f350
update tests
ericphanson Dec 29, 2021
1786c4e
add test
ericphanson Dec 29, 2021
abff73a
rm trailing whitespace
ericphanson Dec 30, 2021
8e986cf
restore check that we're throwing a RemoteException
ericphanson Jan 3, 2022
97f4398
don't capture RemoteException's
ericphanson Jan 3, 2022
505dabd
add docstrings
ericphanson Jan 4, 2022
aef10d9
tweak wording
ericphanson Jan 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion base/asyncmap.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ function start_worker_task!(worker_tasks, exec_func, chnl, batch_size=nothing)
end
catch e
close(chnl)
retval = e
retval = capture_exception(e, catch_backtrace())
Comment on lines 237 to +239
Copy link
Member

Choose a reason for hiding this comment

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

Could we potentially make use of the exception stack here now?

Suggested change
catch e
close(chnl)
retval = e
retval = capture_exception(e, catch_backtrace())
catch
close(chnl)
retval = Base.ExceptionStack(Any[capture_exception(ex...) for ex in current_exceptions()])
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for taking a look. I think there should be a way to use ExceptionStacks here but it’s a bit subtle. I think we’d need more changes in Distributed to facilitate this, since code there is expecting RemoteExceptions to be thrown and to handle them specially. The current approach works around that by continuing to throw them.

end
retval
end
Expand Down
10 changes: 10 additions & 0 deletions base/task.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,16 @@ function showerror(io::IO, ce::CapturedException)
showerror(io, ce.ex, ce.processed_bt, backtrace=true)
end

"""
capture_exception(ex, bt) -> Exception

Returns an exception, possibly incorporating information from a backtrace `bt`. Defaults to returning [`CapturedException(ex, bt)`](@ref).

Used in [`asyncmap`](@ref) and [`asyncmap!`](@ref) to capture exceptions thrown during
the user-supplied function call.
"""
capture_exception(ex, bt) = CapturedException(ex, bt)

"""
CompositeException

Expand Down
7 changes: 7 additions & 0 deletions stdlib/Distributed/src/process_messages.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,13 @@ struct RemoteException <: Exception
captured::CapturedException
end

"""
capture_exception(ex::RemoteException, bt)

Returns `ex::RemoteException` which has already captured a backtrace (via it's [`CapturedException`](@ref) field `captured`).
"""
Base.capture_exception(ex::RemoteException, bt) = ex

"""
RemoteException(captured)

Expand Down
13 changes: 13 additions & 0 deletions test/asyncmap.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,19 @@ len_only_iterable = (1,2,3,4,5)
@test_throws ArgumentError asyncmap(identity, 1:10; batch_size="10")
@test_throws ArgumentError asyncmap(identity, 1:10; ntasks="10")

# Check that we throw a `CapturedException` holding the stacktrace if `f` throws
f42105(i) = i == 5 ? error("captured") : i
let
e = try
asyncmap(f42105, 1:5)
catch e
e
end
@test e isa CapturedException
@test e.ex == ErrorException("captured")
@test e.processed_bt[2][1].func == :f42105
end

include("generic_map_tests.jl")
generic_map_tests(asyncmap, asyncmap!)

Expand Down