Skip to content

Commit

Permalink
Do a better job of capturing the source of errors (#305)
Browse files Browse the repository at this point in the history
Fixes #216
  • Loading branch information
timholy authored Mar 4, 2021
1 parent 35ddff1 commit 6ab2fea
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 12 deletions.
10 changes: 6 additions & 4 deletions src/error_handling.jl
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ LoaderError("ImageMagick", "Foo not available")
struct LoaderError <: Exception
library::String
msg::String
ex
end
Base.showerror(io::IO, e::LoaderError) = println(io, e.library, " load error: ",
e.msg, "\n Will try next loader.")
e.msg, "\n due to ", e.ex, "\n Will try next loader.")

"""
`WriterError` should be thrown when writer library code fails, and other libraries should
Expand All @@ -18,10 +19,11 @@ WriterError("ImageMagick", "Foo not available")
struct WriterError <: Exception
library::String
msg::String
ex
end
Base.showerror(io::IO, e::WriterError) = println(
io, e.library, " writer error: ",
e.msg, "\n Will try next writer."
e.msg, "\n due to ", e.ex, "\n Will try next loader."
)


Expand All @@ -41,7 +43,7 @@ function handle_exceptions(exceptions::Vector, action)
if multiple
println("All errors:")
println("===========================================")
for (err, file) in exceptions
for (err, file, bt) in exceptions
showerror(stdout, err)
println("\n===========================================")
end
Expand All @@ -56,4 +58,4 @@ function handle_exceptions(exceptions::Vector, action)
end
end

handle_error(e, q) = throw(e)
handle_error(e, q, bt) = throw(CapturedException(e, stacktrace(bt)))
6 changes: 3 additions & 3 deletions src/loadsave.jl
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ const fileiofuncs = Dict{Symbol,Function}(:load => load,

function action(call::Symbol, libraries::Vector{ActionSource}, @nospecialize(file::Formatted), args...; options...)
issave = call (:save, :savestreaming)
failures = Tuple{Any,ActionSource}[]
failures = Tuple{Any,ActionSource,Vector}[]
pkgfuncname = Symbol("fileio_", call)
local mod
for library in libraries
Expand All @@ -213,9 +213,9 @@ function action(call::Symbol, libraries::Vector{ActionSource}, @nospecialize(fil
catch e
if isa(e, MethodError) || isa(e, SpecError)
str = "neither $call nor $pkgfuncname is defined"
e = issave ? WriterError(string(mod), str) : LoaderError(string(mod), str)
e = issave ? WriterError(string(mod), str, e) : LoaderError(string(mod), str, e)
end
push!(failures, (e, library))
push!(failures, (e, library, catch_backtrace()))
end
end
handle_exceptions(failures, "$call $(repr(file))")
Expand Down
17 changes: 12 additions & 5 deletions test/error_handling.jl
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ end
@test_logs (:warn, r"supply `pkg` as a Module or `name=>uuid`") @test_throws(ArgumentError, add_format(format"NotInstalled", (), ".not_installed", [:NotInstalled]))
# Give it a fake UUID
add_format(format"NotInstalled", (), ".not_installed", [:NotInstalled=>UUID("79e393ae-7a7b-11eb-1530-bf4d98024096")])
@test_throws ArgumentError save("test.not_installed", nothing)
@test_throws CapturedException save("test.not_installed", nothing)

# Core.eval(Base, :(is_interactive = true)) # for interactive error handling
# add_format(format"NotInstalled", (), ".not_installed", [:NotInstalled])
Expand Down Expand Up @@ -62,8 +62,14 @@ add_format(format"BROKEN", (), ".brok", [BrokenIO])
@testset "Absent implementation" begin
stderr_copy = stderr
rserr, wrerr = redirect_stderr()
@test_throws FileIO.LoaderError load(Stream{format"BROKEN"}(stdin))
@test_throws FileIO.WriterError save(Stream{format"BROKEN"}(stdout), nothing)
ex = try load(Stream{format"BROKEN"}(stdin)) catch e e end
@test isa(ex, CapturedException)
@test isa(ex.ex, FileIO.LoaderError)
@test isa(ex.ex.ex, FileIO.SpecError)
ex = try save(Stream{format"BROKEN"}(stdout), nothing) catch e e end
@test isa(ex, CapturedException)
@test isa(ex.ex, FileIO.WriterError)
@test isa(ex.ex.ex, FileIO.SpecError)
redirect_stderr(stderr_copy)
close(rserr);close(wrerr)
end
Expand All @@ -89,6 +95,7 @@ end
open(tmpfile, "w") do io
println(io, "dummy content")
end
ret = @test_throws ErrorException load(tmpfile)
#@test ret.value.msg == "1" # this is 0.5 only
ex = try load(tmpfile) catch e e end
@test isa(ex, CapturedException)
@test isa(ex.ex, ErrorException)
end

2 comments on commit 6ab2fea

@timholy
Copy link
Member Author

@timholy timholy commented on 6ab2fea Mar 4, 2021

Choose a reason for hiding this comment

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

@JuliaRegistrator
Copy link

Choose a reason for hiding this comment

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

Registration pull request created: JuliaRegistries/General/31274

After the above pull request is merged, it is recommended that a tag is created on this repository for the registered package version.

This will be done automatically if the Julia TagBot GitHub Action is installed, or can be done manually through the github interface, or via:

git tag -a v1.6.0 -m "<description of version>" 6ab2feac52366c3077cfc3ae0febf6c48db50f6e
git push origin v1.6.0

Please sign in to comment.