From 333bd2423ac7f653960d31f3463539cb1fb63c4e Mon Sep 17 00:00:00 2001 From: Lyndon White Date: Mon, 2 Jul 2018 14:25:53 +0800 Subject: [PATCH 01/18] Add ProcessExitedError Update process.jl Update process.jl Update process.jl Use ProcessExitedError remove pre-opt in show ProcessExitedError --- base/process.jl | 30 +++++++++++++++++++++--------- test/spawn.jl | 3 ++- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/base/process.jl b/base/process.jl index a1c3063643fa6..4b815e8f768fb 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) @@ -381,7 +381,7 @@ const SpawnIOs = Vector{Any} # convenience name for readability h === C_NULL ? (0x00, UInt(0)) : h isa OS_HANDLE ? (0x02, UInt(cconvert(@static(Sys.iswindows() ? Ptr{Cvoid} : Cint), h))) : h isa Ptr{Cvoid} ? (0x04, UInt(h)) : - error("invalid spawn handle $h from $io") + error("invalid spawn handle $h from $io") #TODO use a real error type here end for io in stdio] handle = Libc.malloc(_sizeof_uv_process) @@ -783,9 +783,26 @@ An exception is raised if the process cannot be started. """ success(cmd::AbstractCmd) = success(_spawn(cmd)) +struct ProcessExitedError + procs::Vector{Process} +end +ProcessExitedError(proc::Process) = ProcessExitedError([proc]) + +function show(io::IO, err::ProcessExitedError) + 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(ProcessExitedError(proc)) end nothing end @@ -798,12 +815,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(ProcessExitedError(failed)) end """ diff --git a/test/spawn.jl b/test/spawn.jl index 921d31bbb8a4f..dccaca267b072 100644 --- a/test/spawn.jl +++ b/test/spawn.jl @@ -421,9 +421,10 @@ 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]"), + @test_throws(ProcessExitedError("failed process: Process($pcatcmd, ProcessExited(1)) [1]"), collect(p)) end From e7a320804b1bb0182f0f51e51bad3a0e0266c63d Mon Sep 17 00:00:00 2001 From: Lyndon White Date: Tue, 3 Jul 2018 17:39:31 +0800 Subject: [PATCH 02/18] Export ProcessExitedError fix missing comma =remove incorrectly readded ParseError --- base/exports.jl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/base/exports.jl b/base/exports.jl index 8a26eb8fdc73f..12887606cc8d8 100644 --- a/base/exports.jl +++ b/base/exports.jl @@ -117,13 +117,14 @@ export Cwstring, # Exceptions - DimensionMismatch, CapturedException, CompositeException, + DimensionMismatch, EOFError, InvalidStateException, KeyError, MissingException, + ProcessExitedException, SystemError, StringIndexError, From 45b95ea7e24cf1ed7bda820d63458f955df5c41d Mon Sep 17 00:00:00 2001 From: Lyndon White Date: Sat, 24 Nov 2018 17:31:28 +0000 Subject: [PATCH 03/18] =test new exception type =test errors fix typo in tests =fix test typo --- base/process.jl | 10 +++++----- test/spawn.jl | 21 ++++++++++++++++----- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/base/process.jl b/base/process.jl index 4b815e8f768fb..6bbac54f4506e 100644 --- a/base/process.jl +++ b/base/process.jl @@ -783,12 +783,12 @@ An exception is raised if the process cannot be started. """ success(cmd::AbstractCmd) = success(_spawn(cmd)) -struct ProcessExitedError +struct ProcessExitedException procs::Vector{Process} end -ProcessExitedError(proc::Process) = ProcessExitedError([proc]) +ProcessExitedException(proc::Process) = ProcessExitedException([proc]) -function show(io::IO, err::ProcessExitedError) +function show(io::IO, err::ProcessExitedException) if length(err.procs) == 1 proc = err.procs[1] println(io, "failed process: ", proc, " [", proc.exitcode, "]") @@ -802,7 +802,7 @@ end function pipeline_error(proc::Process) if !proc.cmd.ignorestatus - throw(ProcessExitedError(proc)) + throw(ProcessExitedException(proc)) end nothing end @@ -815,7 +815,7 @@ function pipeline_error(procs::ProcessChain) end end isempty(failed) && return nothing - throw(ProcessExitedError(failed)) + throw(ProcessExitedException(failed)) end """ diff --git a/test/spawn.jl b/test/spawn.jl index dccaca267b072..513e7f4c9c7c6 100644 --- a/test/spawn.jl +++ b/test/spawn.jl @@ -420,13 +420,24 @@ end @test Base.shell_split("\"\\\\\"") == ["\\"] +# 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 ProcessExitedException + iobuf = IOBuffer() + show(iobuf, err) + errmsg = String(take!(iobuf)) + @test occursin(string(failing_cmd), errmsg) + end +end + # issue #13616 +@test_throws(ProcessExitedException, collect(eachline(failing_pipeline))) -pcatcmd = `$catcmd _doesnt_exist__111_` -let p = eachline(pipeline(`$catcmd _doesnt_exist__111_`, stderr=devnull)) - @test_throws(ProcessExitedError("failed process: Process($pcatcmd, ProcessExited(1)) [1]"), - collect(p)) -end # make sure windows_verbatim strips quotes if Sys.iswindows() From 48eb7203d91fe6373f53f6729acc03d595e0e264 Mon Sep 17 00:00:00 2001 From: Lyndon White Date: Mon, 26 Nov 2018 17:30:22 +0000 Subject: [PATCH 04/18] download now throws ProcessExitedException --- test/download.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/download.jl b/test/download.jl index 5fa3eef9597b3..6f5120e6a7b08 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 ProcessExitedException 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 ProcessExitedException download("http://192.0.2.1", invalid_host_file) @test !isfile(invalid_host_file) end From 2e3be765059b8cc46cf91f56ef6ff02ecd5ff30f Mon Sep 17 00:00:00 2001 From: Lyndon White Date: Tue, 27 Nov 2018 21:49:57 +0000 Subject: [PATCH 05/18] Make Distributed use ProcessExitedError(nothing) --- base/process.jl | 7 +++++-- stdlib/Distributed/src/Distributed.jl | 1 - stdlib/Distributed/src/cluster.jl | 12 +----------- test/spawn.jl | 6 ++++++ 4 files changed, 12 insertions(+), 14 deletions(-) diff --git a/base/process.jl b/base/process.jl index 6bbac54f4506e..aa5f397c0a3a2 100644 --- a/base/process.jl +++ b/base/process.jl @@ -784,12 +784,15 @@ An exception is raised if the process cannot be started. success(cmd::AbstractCmd) = success(_spawn(cmd)) struct ProcessExitedException - procs::Vector{Process} + procs::Union{Vector{Process},Nothing} + # ProcessExitedException(nothing) is allowed for Distributed stdlib compat end ProcessExitedException(proc::Process) = ProcessExitedException([proc]) function show(io::IO, err::ProcessExitedException) - if length(err.procs) == 1 + if procs == nothing + println(io, "The process has exited.") + elseif length(err.procs) == 1 proc = err.procs[1] println(io, "failed process: ", proc, " [", proc.exitcode, "]") else diff --git a/stdlib/Distributed/src/Distributed.jl b/stdlib/Distributed/src/Distributed.jl index 1ab793f44d8ee..6134f12860270 100644 --- a/stdlib/Distributed/src/Distributed.jl +++ b/stdlib/Distributed/src/Distributed.jl @@ -56,7 +56,6 @@ export Future, WorkerConfig, RemoteException, - ProcessExitedException, process_messages, remoteref_id, diff --git a/stdlib/Distributed/src/cluster.jl b/stdlib/Distributed/src/cluster.jl index fcebf778d822b..1d5c542bd3135 100644 --- a/stdlib/Distributed/src/cluster.jl +++ b/stdlib/Distributed/src/cluster.jl @@ -1030,20 +1030,10 @@ function _rmprocs(pids, waitfor) end -struct ProcessExitedException <: Exception end - -""" - ProcessExitedException() - -After a client Julia process has exited, further attempts to reference the dead child will -throw this exception. -""" -ProcessExitedException() - worker_from_id(i) = worker_from_id(PGRP, i) function worker_from_id(pg::ProcessGroup, i) if !isempty(map_del_wrkr) && in(i, map_del_wrkr) - throw(ProcessExitedException()) + throw(ProcessExitedException(nothing)) end w = get(map_pid_wrkr, i, nothing) if w === nothing diff --git a/test/spawn.jl b/test/spawn.jl index 513e7f4c9c7c6..965d95f7dbe50 100644 --- a/test/spawn.jl +++ b/test/spawn.jl @@ -435,6 +435,12 @@ for testrun in (failing_pipeline, pipeline(failing_pipeline, failing_pipeline)) end end +let + iobuf = IOBuffer() + show(iobuf, ProcessExitedException(nothing)) + @test length(String(take!(iobuf))) > 0 +end + # issue #13616 @test_throws(ProcessExitedException, collect(eachline(failing_pipeline))) From 8002a9e646f311f0676868dc1df6e429eb3b8109 Mon Sep 17 00:00:00 2001 From: Lyndon White Date: Wed, 28 Nov 2018 08:01:16 +0000 Subject: [PATCH 06/18] zero argument ProcessExitError --- base/process.jl | 1 + stdlib/Distributed/src/cluster.jl | 2 +- test/spawn.jl | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/base/process.jl b/base/process.jl index aa5f397c0a3a2..81cfc3bde0a03 100644 --- a/base/process.jl +++ b/base/process.jl @@ -787,6 +787,7 @@ struct ProcessExitedException procs::Union{Vector{Process},Nothing} # ProcessExitedException(nothing) is allowed for Distributed stdlib compat end +ProcessExitedException() = ProcessExitedException(nothing) ProcessExitedException(proc::Process) = ProcessExitedException([proc]) function show(io::IO, err::ProcessExitedException) diff --git a/stdlib/Distributed/src/cluster.jl b/stdlib/Distributed/src/cluster.jl index 1d5c542bd3135..919a06919a61a 100644 --- a/stdlib/Distributed/src/cluster.jl +++ b/stdlib/Distributed/src/cluster.jl @@ -1033,7 +1033,7 @@ end worker_from_id(i) = worker_from_id(PGRP, i) function worker_from_id(pg::ProcessGroup, i) if !isempty(map_del_wrkr) && in(i, map_del_wrkr) - throw(ProcessExitedException(nothing)) + throw(ProcessExitedException()) end w = get(map_pid_wrkr, i, nothing) if w === nothing diff --git a/test/spawn.jl b/test/spawn.jl index 965d95f7dbe50..666d1c731ac9f 100644 --- a/test/spawn.jl +++ b/test/spawn.jl @@ -437,7 +437,7 @@ end let iobuf = IOBuffer() - show(iobuf, ProcessExitedException(nothing)) + show(iobuf, ProcessExitedException()) @test length(String(take!(iobuf))) > 0 end From f0fe1f20c59896d9b94161803a743fd95474f080 Mon Sep 17 00:00:00 2001 From: Lyndon White Date: Wed, 28 Nov 2018 08:35:57 +0000 Subject: [PATCH 07/18] fix show(ProcessExitedError --- base/process.jl | 2 +- test/spawn.jl | 9 +++------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/base/process.jl b/base/process.jl index 81cfc3bde0a03..84c80bf00bccb 100644 --- a/base/process.jl +++ b/base/process.jl @@ -791,7 +791,7 @@ ProcessExitedException() = ProcessExitedException(nothing) ProcessExitedException(proc::Process) = ProcessExitedException([proc]) function show(io::IO, err::ProcessExitedException) - if procs == nothing + if err.procs === nothing println(io, "The process has exited.") elseif length(err.procs) == 1 proc = err.procs[1] diff --git a/test/spawn.jl b/test/spawn.jl index 666d1c731ac9f..4c394a444c1b0 100644 --- a/test/spawn.jl +++ b/test/spawn.jl @@ -428,17 +428,14 @@ for testrun in (failing_pipeline, pipeline(failing_pipeline, failing_pipeline)) run(testrun) catch err @test err isa ProcessExitedException - iobuf = IOBuffer() - show(iobuf, err) - errmsg = String(take!(iobuf)) + errmsg = sprint(show, err) @test occursin(string(failing_cmd), errmsg) end end let - iobuf = IOBuffer() - show(iobuf, ProcessExitedException()) - @test length(String(take!(iobuf))) > 0 + errmsg = sprint(show, ProcessExitedException()) + @test occursin("exited" , errmsg) end # issue #13616 From 42d84a0d3c3c8c1a2b36513eea48e1d6a809340e Mon Sep 17 00:00:00 2001 From: Lyndon White Date: Wed, 28 Nov 2018 15:19:54 -0600 Subject: [PATCH 08/18] Add docstring to ProcessExitedException Reference Distributed Computing Manpage remove whitespace --- base/process.jl | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/base/process.jl b/base/process.jl index 84c80bf00bccb..0207b20a3eefd 100644 --- a/base/process.jl +++ b/base/process.jl @@ -783,6 +783,19 @@ An exception is raised if the process cannot be started. """ success(cmd::AbstractCmd) = success(_spawn(cmd)) + +""" + ProcessExitedException + +Indicates problematic exit status of a process. + +Usages include: + + - When running commands or pipelines, this is thrown to indicate + a nonzero exit code was returned (i.e. that the invoked process failed). + - In a [Distributed Computing](@ref) workflow, this is thrown + when work is sent to a worker julia process that has exited. +""" struct ProcessExitedException procs::Union{Vector{Process},Nothing} # ProcessExitedException(nothing) is allowed for Distributed stdlib compat From eeaafd33a53db44e315931e0aaeb4edde3eba67a Mon Sep 17 00:00:00 2001 From: Lyndon White Date: Wed, 28 Nov 2018 15:41:32 -0600 Subject: [PATCH 09/18] Add news move dot points to column 1 --- NEWS.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 0bff77b253102..8330395cb6419 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 `ProcessExitedException`s +rather than an `ErrorException`, if those commands fail and exit with non-zero exit code. +([#27900]). Command-line option changes --------------------------- From b778021682431b4270460b7e5f4f316917eef025 Mon Sep 17 00:00:00 2001 From: Lyndon White Date: Wed, 28 Nov 2018 15:44:27 -0600 Subject: [PATCH 10/18] Add docs --- doc/src/base/base.md | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/src/base/base.md b/doc/src/base/base.md index 027fe21b26b01..ab9f813875e95 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.ProcessExitedException Core.StackOverflowError Base.SystemError Core.TypeError From 77b04ccae65bfee2a64095df15287879b511e08c Mon Sep 17 00:00:00 2001 From: Lyndon White Date: Wed, 16 Jan 2019 14:19:06 +0000 Subject: [PATCH 11/18] Make ProcessExitedException and Exception and display it with showerror test showerror --- base/process.jl | 4 ++-- test/spawn.jl | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/base/process.jl b/base/process.jl index 0207b20a3eefd..2996d277255f8 100644 --- a/base/process.jl +++ b/base/process.jl @@ -796,14 +796,14 @@ Usages include: - In a [Distributed Computing](@ref) workflow, this is thrown when work is sent to a worker julia process that has exited. """ -struct ProcessExitedException +struct ProcessExitedException <: Exception procs::Union{Vector{Process},Nothing} # ProcessExitedException(nothing) is allowed for Distributed stdlib compat end ProcessExitedException() = ProcessExitedException(nothing) ProcessExitedException(proc::Process) = ProcessExitedException([proc]) -function show(io::IO, err::ProcessExitedException) +function showerror(io::IO, err::ProcessExitedException) if err.procs === nothing println(io, "The process has exited.") elseif length(err.procs) == 1 diff --git a/test/spawn.jl b/test/spawn.jl index 4c394a444c1b0..77a7a27a733ef 100644 --- a/test/spawn.jl +++ b/test/spawn.jl @@ -428,13 +428,13 @@ for testrun in (failing_pipeline, pipeline(failing_pipeline, failing_pipeline)) run(testrun) catch err @test err isa ProcessExitedException - errmsg = sprint(show, err) + errmsg = sprint(showerror, err) @test occursin(string(failing_cmd), errmsg) end end let - errmsg = sprint(show, ProcessExitedException()) + errmsg = sprint(showerror, ProcessExitedException()) @test occursin("exited" , errmsg) end From 7444bcc74b798fac687d1955e5cabaa167279ecc Mon Sep 17 00:00:00 2001 From: Lyndon White Date: Fri, 22 Feb 2019 10:19:58 +0000 Subject: [PATCH 12/18] revert combining worker failure with spawn failure --- base/process.jl | 21 +++++++-------------- stdlib/Distributed/src/Distributed.jl | 1 + stdlib/Distributed/src/cluster.jl | 10 ++++++++++ test/spawn.jl | 5 ----- 4 files changed, 18 insertions(+), 19 deletions(-) diff --git a/base/process.jl b/base/process.jl index 2996d277255f8..66df90cbd4c91 100644 --- a/base/process.jl +++ b/base/process.jl @@ -788,25 +788,18 @@ success(cmd::AbstractCmd) = success(_spawn(cmd)) ProcessExitedException Indicates problematic exit status of a process. - -Usages include: - - - When running commands or pipelines, this is thrown to indicate - a nonzero exit code was returned (i.e. that the invoked process failed). - - In a [Distributed Computing](@ref) workflow, this is thrown - when work is sent to a worker julia process that has exited. +When running commands or pipelines, this is thrown to indicate +a nonzero exit code was returned (i.e. that the invoked process failed). """ -struct ProcessExitedException <: Exception - procs::Union{Vector{Process},Nothing} - # ProcessExitedException(nothing) is allowed for Distributed stdlib compat +struct ProcessExitedException + procs::Vector{Process} + end ProcessExitedException() = ProcessExitedException(nothing) ProcessExitedException(proc::Process) = ProcessExitedException([proc]) -function showerror(io::IO, err::ProcessExitedException) - if err.procs === nothing - println(io, "The process has exited.") - elseif length(err.procs) == 1 +function show(io::IO, err::ProcessExitedException) + if length(err.procs) == 1 proc = err.procs[1] println(io, "failed process: ", proc, " [", proc.exitcode, "]") else diff --git a/stdlib/Distributed/src/Distributed.jl b/stdlib/Distributed/src/Distributed.jl index 6134f12860270..1ab793f44d8ee 100644 --- a/stdlib/Distributed/src/Distributed.jl +++ b/stdlib/Distributed/src/Distributed.jl @@ -56,6 +56,7 @@ export Future, WorkerConfig, RemoteException, + ProcessExitedException, process_messages, remoteref_id, diff --git a/stdlib/Distributed/src/cluster.jl b/stdlib/Distributed/src/cluster.jl index 919a06919a61a..fcebf778d822b 100644 --- a/stdlib/Distributed/src/cluster.jl +++ b/stdlib/Distributed/src/cluster.jl @@ -1030,6 +1030,16 @@ function _rmprocs(pids, waitfor) end +struct ProcessExitedException <: Exception end + +""" + ProcessExitedException() + +After a client Julia process has exited, further attempts to reference the dead child will +throw this exception. +""" +ProcessExitedException() + worker_from_id(i) = worker_from_id(PGRP, i) function worker_from_id(pg::ProcessGroup, i) if !isempty(map_del_wrkr) && in(i, map_del_wrkr) diff --git a/test/spawn.jl b/test/spawn.jl index 77a7a27a733ef..73d7e30678dca 100644 --- a/test/spawn.jl +++ b/test/spawn.jl @@ -433,11 +433,6 @@ for testrun in (failing_pipeline, pipeline(failing_pipeline, failing_pipeline)) end end -let - errmsg = sprint(showerror, ProcessExitedException()) - @test occursin("exited" , errmsg) -end - # issue #13616 @test_throws(ProcessExitedException, collect(eachline(failing_pipeline))) From 74fbe21dcf61772bcce8a330f448abfde0da1ead Mon Sep 17 00:00:00 2001 From: Lyndon White Date: Fri, 22 Feb 2019 10:30:56 +0000 Subject: [PATCH 13/18] Rename ProcessExitedException to ProcessFailedException --- NEWS.md | 4 ++-- base/exports.jl | 2 +- base/process.jl | 14 +++++++------- doc/src/base/base.md | 2 +- test/download.jl | 4 ++-- test/spawn.jl | 4 ++-- 6 files changed, 15 insertions(+), 15 deletions(-) diff --git a/NEWS.md b/NEWS.md index 8330395cb6419..c0aa12a9907f3 100644 --- a/NEWS.md +++ b/NEWS.md @@ -29,8 +29,8 @@ Language changes * `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 `ProcessExitedException`s -rather than an `ErrorException`, if those commands fail and exit with non-zero exit code. +* 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/exports.jl b/base/exports.jl index 12887606cc8d8..23b4141a06c5e 100644 --- a/base/exports.jl +++ b/base/exports.jl @@ -124,7 +124,7 @@ export InvalidStateException, KeyError, MissingException, - ProcessExitedException, + ProcessFailedException, SystemError, StringIndexError, diff --git a/base/process.jl b/base/process.jl index 66df90cbd4c91..cb862849294ab 100644 --- a/base/process.jl +++ b/base/process.jl @@ -785,20 +785,20 @@ success(cmd::AbstractCmd) = success(_spawn(cmd)) """ - ProcessExitedException + 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 ProcessExitedException +struct ProcessFailedException procs::Vector{Process} end -ProcessExitedException() = ProcessExitedException(nothing) -ProcessExitedException(proc::Process) = ProcessExitedException([proc]) +ProcessFailedException() = ProcessFailedException(nothing) +ProcessFailedException(proc::Process) = ProcessFailedException([proc]) -function show(io::IO, err::ProcessExitedException) +function show(io::IO, err::ProcessFailedException) if length(err.procs) == 1 proc = err.procs[1] println(io, "failed process: ", proc, " [", proc.exitcode, "]") @@ -812,7 +812,7 @@ end function pipeline_error(proc::Process) if !proc.cmd.ignorestatus - throw(ProcessExitedException(proc)) + throw(ProcessFailedException(proc)) end nothing end @@ -825,7 +825,7 @@ function pipeline_error(procs::ProcessChain) end end isempty(failed) && return nothing - throw(ProcessExitedException(failed)) + throw(ProcessFailedException(failed)) end """ diff --git a/doc/src/base/base.md b/doc/src/base/base.md index ab9f813875e95..e7fe19d7517e8 100644 --- a/doc/src/base/base.md +++ b/doc/src/base/base.md @@ -347,7 +347,7 @@ Base.MissingException Core.OutOfMemoryError Core.ReadOnlyMemoryError Core.OverflowError -Base.ProcessExitedException +Base.ProcessFailedException Core.StackOverflowError Base.SystemError Core.TypeError diff --git a/test/download.jl b/test/download.jl index 6f5120e6a7b08..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 ProcessExitedException 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 ProcessExitedException 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 73d7e30678dca..8a2aec96dd499 100644 --- a/test/spawn.jl +++ b/test/spawn.jl @@ -427,14 +427,14 @@ for testrun in (failing_pipeline, pipeline(failing_pipeline, failing_pipeline)) try run(testrun) catch err - @test err isa ProcessExitedException + @test err isa ProcessFailedException errmsg = sprint(showerror, err) @test occursin(string(failing_cmd), errmsg) end end # issue #13616 -@test_throws(ProcessExitedException, collect(eachline(failing_pipeline))) +@test_throws(ProcessFailedException, collect(eachline(failing_pipeline))) # make sure windows_verbatim strips quotes From 1f6414f2e9b0d3d1418a06b2b84971a974d2be67 Mon Sep 17 00:00:00 2001 From: Lyndon White Date: Fri, 22 Feb 2019 10:35:35 +0000 Subject: [PATCH 14/18] make ProcessFailedException an Exception (again) --- base/process.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base/process.jl b/base/process.jl index cb862849294ab..48473bd4287f9 100644 --- a/base/process.jl +++ b/base/process.jl @@ -791,7 +791,7 @@ 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 +struct ProcessFailedException <: Exception procs::Vector{Process} end From 2ea87ae30ba40307e3e6dd8c9a47096c5e4f789a Mon Sep 17 00:00:00 2001 From: Lyndon White Date: Sat, 23 Feb 2019 11:10:22 +0000 Subject: [PATCH 15/18] remove invalid nothing constructor --- base/process.jl | 2 -- 1 file changed, 2 deletions(-) diff --git a/base/process.jl b/base/process.jl index 48473bd4287f9..794683f137082 100644 --- a/base/process.jl +++ b/base/process.jl @@ -793,9 +793,7 @@ a nonzero exit code was returned (i.e. that the invoked process failed). """ struct ProcessFailedException <: Exception procs::Vector{Process} - end -ProcessFailedException() = ProcessFailedException(nothing) ProcessFailedException(proc::Process) = ProcessFailedException([proc]) function show(io::IO, err::ProcessFailedException) From 6aac39cfeaec63e3e2c793f2e8ab246b628be69d Mon Sep 17 00:00:00 2001 From: Lyndon White Date: Sat, 23 Feb 2019 11:11:21 +0000 Subject: [PATCH 16/18] use showerror --- base/process.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base/process.jl b/base/process.jl index 794683f137082..11c7b7aca32fa 100644 --- a/base/process.jl +++ b/base/process.jl @@ -796,7 +796,7 @@ struct ProcessFailedException <: Exception end ProcessFailedException(proc::Process) = ProcessFailedException([proc]) -function show(io::IO, err::ProcessFailedException) +function showerror(io::IO, err::ProcessFailedException) if length(err.procs) == 1 proc = err.procs[1] println(io, "failed process: ", proc, " [", proc.exitcode, "]") From e2d07dcac626e386dfb8fcad96113f71645db26e Mon Sep 17 00:00:00 2001 From: Lyndon White Date: Thu, 14 Mar 2019 16:01:25 -0300 Subject: [PATCH 17/18] Remove todo about replacing invalid spawn handle erroor --- base/process.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base/process.jl b/base/process.jl index 11c7b7aca32fa..3e578c2e4ec3f 100644 --- a/base/process.jl +++ b/base/process.jl @@ -381,7 +381,7 @@ const SpawnIOs = Vector{Any} # convenience name for readability h === C_NULL ? (0x00, UInt(0)) : h isa OS_HANDLE ? (0x02, UInt(cconvert(@static(Sys.iswindows() ? Ptr{Cvoid} : Cint), h))) : h isa Ptr{Cvoid} ? (0x04, UInt(h)) : - error("invalid spawn handle $h from $io") #TODO use a real error type here + error("invalid spawn handle $h from $io") end for io in stdio] handle = Libc.malloc(_sizeof_uv_process) From d7f1450c9ef3bc6b4da38b4678f861e7c9bf9265 Mon Sep 17 00:00:00 2001 From: Lyndon White Date: Mon, 1 Apr 2019 22:22:32 +0100 Subject: [PATCH 18/18] Log extra error related info rather than throwing different error --- base/download.jl | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) 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