Fix exception handling for Base.run()

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
 

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)))

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