Skip to content

Commit

Permalink
error handling improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
IanButterworth committed Aug 16, 2024
1 parent e8ce187 commit f85aaf7
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 14 deletions.
14 changes: 7 additions & 7 deletions src/Operations.jl
Original file line number Diff line number Diff line change
Expand Up @@ -836,7 +836,7 @@ function download_artifacts(ctx::Context;
end
push!(pkg_roots, dirname(env.project_file))
used_artifact_tomls = Set{String}()
download_jobs = Channel{Function}(Inf)
download_jobs = Function[]

print_lock = Base.ReentrantLock() # for non-fancyprint printing

Expand Down Expand Up @@ -886,7 +886,6 @@ function download_artifacts(ctx::Context;
push!(used_artifact_tomls, artifacts_toml)
end
end
close(download_jobs)

if !isempty(download_jobs)
longest_name = maximum(textwidth, keys(download_states))
Expand All @@ -909,7 +908,7 @@ function download_artifacts(ctx::Context;
n_printed = 1
show_progress(iostr, main_bar; carriagereturn=false)
println(iostr)
for (name, dstate) in sort!(collect(download_states), by=kv->kv[2][2].max, rev=true)
for (name, dstate) in sort!(collect(download_states), by=kv->kv[2].bar.max, rev=true)
dstate.state == :running && dstate.bar.max > 1000 && dstate.bar.current > 0 || continue
show_progress(iostr, dstate.bar; carriagereturn=false)
println(iostr)
Expand All @@ -932,8 +931,9 @@ function download_artifacts(ctx::Context;
end
end
Base.errormonitor(t_print)
else
printpkgstyle(io, :Downloading, "$(length(download_jobs)) artifacts")
end

sema = Base.Semaphore(ctx.num_concurrent_downloads)
interrupted = false
@sync for f in download_jobs
Expand All @@ -949,18 +949,18 @@ function download_artifacts(ctx::Context;
end
end
is_done = true
wait(t_print)
fancyprint && wait(t_print)
close(errors)

if !isempty(errors)
all_errors = collect(errors)
str = sprint(context=io) do iostr
for e in all_errors
Base.showerror(iostr, e)
length(all_errors) > 1 && print(io, "\n----------------\n")
length(all_errors) > 1 && println(iostr)
end
end
pkgerror("Failed to download some artifacts:\n\n$str")
pkgerror("Failed to download some artifacts:\n\n$(strip(str, '\n'))")
end
end

Expand Down
25 changes: 18 additions & 7 deletions src/PlatformEngines.jl
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,8 @@ function download_verify(
end
end
end
if hash !== nothing && !verify(dest, hash; verbose=verbose)
details = String[]
if hash !== nothing && !verify(dest, hash; verbose, details)
# If the file already existed, it's possible the initially downloaded chunk
# was bad. If verification fails after downloading, auto-delete the file
# and start over from scratch.
Expand All @@ -380,13 +381,15 @@ function download_verify(

# Download and verify from scratch
download(url, dest; verbose=verbose || !quiet_download)
if hash !== nothing && !verify(dest, hash; verbose=verbose)
error("Verification failed. Download does not match expected hash")
if hash !== nothing && !verify(dest, hash; verbose, details)
@goto verification_failed
end
else
@label verification_failed
# If it didn't verify properly and we didn't resume, something is
# very wrong and we must complain mightily.
error("Verification failed. Download does not match expected hash")
details_indented = join(map(s -> " $s", split(join(details, "\n"), '\n')), "\n")
error("Verification failed:\n" * details_indented)
end
end

Expand Down Expand Up @@ -555,7 +558,8 @@ end

"""
verify(path::AbstractString, hash::AbstractString;
verbose::Bool = false, report_cache_status::Bool = false)
verbose::Bool = false, report_cache_status::Bool = false,
details::Union{Vector{String},Nothing} = nothing)
Given a file `path` and a `hash`, calculate the SHA256 of the file and compare
it to `hash`. This method caches verification results in a `"\$(path).sha256"`
Expand All @@ -571,9 +575,12 @@ If `report_cache_status` is set to `true`, then the return value will be a
`Symbol` giving a granular status report on the state of the hash cache, in
addition to the `true`/`false` signifying whether verification completed
successfully.
If `details` is provided, any pertinent detail will be pushed to it rather than logged.
"""
function verify(path::AbstractString, hash::AbstractString; verbose::Bool = false,
report_cache_status::Bool = false, hash_path::AbstractString="$(path).sha256")
report_cache_status::Bool = false, hash_path::AbstractString="$(path).sha256",
details::Union{Vector{String},Nothing} = nothing)

# Check hash string format
if !occursin(r"^[0-9a-f]{64}$"i, hash)
Expand Down Expand Up @@ -643,7 +650,11 @@ function verify(path::AbstractString, hash::AbstractString; verbose::Bool = fals
msg = "Hash Mismatch!\n"
msg *= " Expected sha256: $hash\n"
msg *= " Calculated sha256: $calc_hash"
@error(msg)
if isnothing(details)
@error(msg)
else
push!(details, msg)
end
if report_cache_status
return false, :hash_mismatch
else
Expand Down

0 comments on commit f85aaf7

Please sign in to comment.