diff --git a/NEWS.md b/NEWS.md index 0bff77b253102..c0aa12a9907f3 100644 --- a/NEWS.md +++ b/NEWS.md @@ -20,13 +20,18 @@ New language features Multi-threading changes ----------------------- - * The `Condition` type now has a thread-safe replacement, accessed as `Threads.Condition`. +* The `Condition` type now has a thread-safe replacement, accessed as `Threads.Condition`. With that addition, task scheduling primitives such as `ReentrantLock` are now thread-safe ([#30061]). Language changes ---------------- * Empty entries in `JULIA_DEPOT_PATH` are now expanded to default depot entries ([#31009]). * `Enum` now behaves like a scalar when used in broadcasting ([#30670]). +* If a `pipeline` is specified with `append=true` set, but no redirection, an `ArgumentError` +is thrown, rather than a `ErrorException` ([#27900]). +* Functions that invoke commands (e.g. `run(::Cmd)`) now throw a `ProcessFailedException` +rather than an `ErrorException`, if those commands exit with non-zero exit code. +([#27900]). Command-line option changes --------------------------- diff --git a/base/download.jl b/base/download.jl index 5e2a2442e7e1e..cc7c75cc02422 100644 --- a/base/download.jl +++ b/base/download.jl @@ -15,7 +15,7 @@ if Sys.iswindows() if proc.exitcode % Int32 == -393216 # appears to be "wrong version" exit code, based on # https://docs.microsoft.com/en-us/azure/cloud-services/cloud-services-startup-tasks-common - error("Downloading files requires Windows Management Framework 3.0 or later.") + @error "Downloading files requires Windows Management Framework 3.0 or later." end pipeline_error(proc) end @@ -39,8 +39,9 @@ function download_curl(curl_exe::AbstractString, url::AbstractString, filename:: err = PipeBuffer() process = run(pipeline(`$curl_exe -s -S -g -L -f -o $filename $url`, stderr=err), wait=false) if !success(process) - stderr = readline(err) - error(stderr) + error_msg = readline(err) + @error "Download Failed" error_msg + pipeline_error(process) end return filename end diff --git a/base/exports.jl b/base/exports.jl index 8a26eb8fdc73f..23b4141a06c5e 100644 --- a/base/exports.jl +++ b/base/exports.jl @@ -117,13 +117,14 @@ export Cwstring, # Exceptions - DimensionMismatch, CapturedException, CompositeException, + DimensionMismatch, EOFError, InvalidStateException, KeyError, MissingException, + ProcessFailedException, SystemError, StringIndexError, diff --git a/base/process.jl b/base/process.jl index a1c3063643fa6..3e578c2e4ec3f 100644 --- a/base/process.jl +++ b/base/process.jl @@ -268,7 +268,7 @@ run(pipeline(`update`, stdout="log.txt", append=true)) """ function pipeline(cmd::AbstractCmd; stdin=nothing, stdout=nothing, stderr=nothing, append::Bool=false) if append && stdout === nothing && stderr === nothing - error("append set to true, but no output redirections specified") + throw(ArgumentError("append set to true, but no output redirections specified")) end if stdin !== nothing cmd = redir_out(stdin, cmd) @@ -783,9 +783,34 @@ An exception is raised if the process cannot be started. """ success(cmd::AbstractCmd) = success(_spawn(cmd)) + +""" + ProcessFailedException + +Indicates problematic exit status of a process. +When running commands or pipelines, this is thrown to indicate +a nonzero exit code was returned (i.e. that the invoked process failed). +""" +struct ProcessFailedException <: Exception + procs::Vector{Process} +end +ProcessFailedException(proc::Process) = ProcessFailedException([proc]) + +function showerror(io::IO, err::ProcessFailedException) + if length(err.procs) == 1 + proc = err.procs[1] + println(io, "failed process: ", proc, " [", proc.exitcode, "]") + else + println(io, "failed processes:") + for proc in err.procs + println(io, " ", proc, " [", proc.exitcode, "]") + end + end +end + function pipeline_error(proc::Process) if !proc.cmd.ignorestatus - error("failed process: ", proc, " [", proc.exitcode, "]") + throw(ProcessFailedException(proc)) end nothing end @@ -798,12 +823,7 @@ function pipeline_error(procs::ProcessChain) end end isempty(failed) && return nothing - length(failed) == 1 && pipeline_error(failed[1]) - msg = "failed processes:" - for proc in failed - msg = string(msg, "\n ", proc, " [", proc.exitcode, "]") - end - error(msg) + throw(ProcessFailedException(failed)) end """ diff --git a/doc/src/base/base.md b/doc/src/base/base.md index 027fe21b26b01..e7fe19d7517e8 100644 --- a/doc/src/base/base.md +++ b/doc/src/base/base.md @@ -347,6 +347,7 @@ Base.MissingException Core.OutOfMemoryError Core.ReadOnlyMemoryError Core.OverflowError +Base.ProcessFailedException Core.StackOverflowError Base.SystemError Core.TypeError diff --git a/test/download.jl b/test/download.jl index 5fa3eef9597b3..d8f939fc6a692 100644 --- a/test/download.jl +++ b/test/download.jl @@ -16,7 +16,7 @@ mktempdir() do temp_dir # Make sure that failed downloads do not leave files around missing_file = joinpath(temp_dir, "missing") - @test_throws ErrorException download("http://httpbin.org/status/404", missing_file) + @test_throws ProcessFailedException download("http://httpbin.org/status/404", missing_file) @test !isfile(missing_file) # Make sure we properly handle metachar ' @@ -28,6 +28,6 @@ mktempdir() do temp_dir # Use a TEST-NET (192.0.2.0/24) address which shouldn't be bound invalid_host_file = joinpath(temp_dir, "invalid_host") - @test_throws ErrorException download("http://192.0.2.1", invalid_host_file) + @test_throws ProcessFailedException download("http://192.0.2.1", invalid_host_file) @test !isfile(invalid_host_file) end diff --git a/test/spawn.jl b/test/spawn.jl index 921d31bbb8a4f..8a2aec96dd499 100644 --- a/test/spawn.jl +++ b/test/spawn.jl @@ -420,13 +420,23 @@ end @test Base.shell_split("\"\\\\\"") == ["\\"] -# issue #13616 -pcatcmd = `$catcmd _doesnt_exist__111_` -let p = eachline(pipeline(`$catcmd _doesnt_exist__111_`, stderr=devnull)) - @test_throws(ErrorException("failed process: Process($pcatcmd, ProcessExited(1)) [1]"), - collect(p)) +# Test failing commands +failing_cmd = `$catcmd _doesnt_exist__111_` +failing_pipeline = pipeline(failing_cmd, stderr=devnull) # make quiet for tests +for testrun in (failing_pipeline, pipeline(failing_pipeline, failing_pipeline)) + try + run(testrun) + catch err + @test err isa ProcessFailedException + errmsg = sprint(showerror, err) + @test occursin(string(failing_cmd), errmsg) + end end +# issue #13616 +@test_throws(ProcessFailedException, collect(eachline(failing_pipeline))) + + # make sure windows_verbatim strips quotes if Sys.iswindows() @test read(`cmd.exe /c dir /b spawn.jl`, String) == read(Cmd(`cmd.exe /c dir /b "\"spawn.jl\""`, windows_verbatim=true), String)