diff --git a/docs/src/changelog.md b/docs/src/changelog.md index c856e5e..d84ac49 100644 --- a/docs/src/changelog.md +++ b/docs/src/changelog.md @@ -24,6 +24,9 @@ Changelog](https://keepachangelog.com). ### Fixed - Improved handling of possible errors in [`Base.readdir()`](@ref) ([#20]). +- Fixed exception handling for [`Base.run()`](@ref), now it throws a + [`SshProcessFailedException`](@ref) or [`LibSSHException`](@ref) on command + failure instead of a plain `TaskFailedException` ([#25]). ## [v0.6.0] - 2024-10-11 diff --git a/src/channel.jl b/src/channel.jl index 57be5c1..1ca40a8 100644 --- a/src/channel.jl +++ b/src/channel.jl @@ -493,8 +493,18 @@ $(TYPEDSIGNATURES) function Base.wait(process::SshProcess) try wait(process._task) - catch ex - if process.cmd isa Cmd && !process.cmd.ignorestatus + catch task_ex + ex = process._task.exception + + # The idea is that SshProcessFailedException's and LibSSHException's are + # somewhat expected so we always unwrap them from the + # TaskFailedException before throwing, which is a slightly nicer API to + # work with. + if ex isa SshProcessFailedException || ex isa LibSSHException + if !(process.cmd isa Cmd && process.cmd.ignorestatus) + throw(process._task.exception) + end + else rethrow() end end @@ -591,6 +601,8 @@ An easy way of getting around these restrictions is to pass the command as a # Throws - [`SshProcessFailedException`](@ref): if the command fails and `ignorestatus()` wasn't used. +- [`LibSSHException`](@ref): if running the command fails for some other + reason. # Arguments - `cmd`: The command to run. This will be converted to a string for running @@ -652,10 +664,10 @@ function Base.run(cmd::Union{Cmd, String}, session::Session; set_channel_callbacks(process._sshchan, callbacks) process._task = Threads.@spawn _exec_command(process) - errormonitor(process._task) + if wait # Note the use of Base.wait() to avoid aliasing with the `wait` argument - Base.wait(process._task) + Base.wait(process) if print_out print(String(copy(process.out))) diff --git a/test/LibSSHTests.jl b/test/LibSSHTests.jl index 5c039b6..fa6f795 100644 --- a/test/LibSSHTests.jl +++ b/test/LibSSHTests.jl @@ -564,16 +564,8 @@ end cmd = setenv(`echo \$foo`, "foo" => "bar") @test readchomp(cmd, session) == "bar" - # Test command failure. We redirect error output to devnull to hide - # the errors displayed by the errormonitor() task. - try - redirect_stderr(devnull) do - run(`foo`, session) - end - catch ex - @test ex isa TaskFailedException - @test current_exceptions(ex.task)[1][1] isa ssh.SshProcessFailedException - end + # Test command failure + @test_throws ssh.SshProcessFailedException run(`foo`, session) # Test passing a String instead of a Cmd mktempdir() do tmpdir