sshkit icon indicating copy to clipboard operation
sshkit copied to clipboard

Feature: Don't display redundant error messages on execute failure

Open kenn opened this issue 9 years ago • 10 comments

Showing the same error in three different ways seems redundant. Is it possible to collapse into one?

The second one looks most relevant.

cap aborted!
SSHKit::Runner::ExecuteError: Exception while executing on host 12.34.56.78: Exception while executing on host 12.34.56.78: logrotate -f /etc/logrotate.conf exit status: 1
logrotate -f /etc/logrotate.conf stdout: Nothing written
logrotate -f /etc/logrotate.conf stderr: error: error running non-shared postrotate script for /path/to/log1.log of '/path/to/log1.log
/path/to/log2.log
'

SSHKit::Runner::ExecuteError: Exception while executing on host 12.34.56.78: logrotate -f /etc/logrotate.conf exit status: 1
logrotate -f /etc/logrotate.conf stdout: Nothing written
logrotate -f /etc/logrotate.conf stderr: error: error running non-shared postrotate script for /path/to/log1.log of '/path/to/log1.log
/path/to/log2.log
'

SSHKit::Command::Failed: logrotate -f /etc/logrotate.conf exit status: 1
logrotate -f /etc/logrotate.conf stdout: Nothing written
logrotate -f /etc/logrotate.conf stderr: error: error running non-shared postrotate script for /path/to/log1.log of '/root/apps/eme/shared/log/staging.log
/path/to/log2.log
'

Background: It occurred when I run

execute 'logrotate -f /etc/logrotate.conf'

on a single server, where the postrotate script for logrotate exits with a failure status.

kenn avatar May 18 '16 18:05 kenn

Can you provide a complete output please, I'm not sure what you're comparing to, the first two look very similar apart from Exception while executing on host... when it's duplicated, but I'd have to run a command myself to know if that's a race condition or whether we really print the error twice.

leehambley avatar May 18 '16 20:05 leehambley

In my experience the error appears twice, like this:

(Backtrace restricted to imported tasks)
cap aborted!
SSHKit::Runner::ExecuteError: Exception while executing as deployer@###.###.###.###: git exit status: 128
git stdout: Nothing written
git stderr: conq: repository does not exist.
fatal: The remote end hung up unexpectedly

SSHKit::Command::Failed: git exit status: 128
git stdout: Nothing written
git stderr: conq: repository does not exist.
fatal: The remote end hung up unexpectedly

Tasks: TOP => git:check
(See full trace by running task with --trace)
zlib(finalizer): the stream was freed prematurely.

I agree we should not duplicate the message. But in your case it seems something else is going on. I don't understand why you are seeing the error three times. Let's solve that issue first.

The only thing I can think of is that you are nesting on blocks, like:

on ... do
  on ... do
    execute ...
  end
end

mattbrictson avatar May 18 '16 20:05 mattbrictson

I created a minimally reproducible task, but now I get 2 errors rather than 3.

namespace :config do
  task :false do on roles(:all) do
    execute 'false'
  end end
end
$ cap staging config:false
rvm 1.27.0 (latest) by Wayne E. Seguin <[email protected]>, Michal Papis <[email protected]> [https://rvm.io/]
ruby-2.3.0
ruby 2.3.0p0 (2015-12-25 revision 53290) [x86_64-linux]
00:00 config:false
      01 false
(Backtrace restricted to imported tasks)
cap aborted!
SSHKit::Runner::ExecuteError: Exception while executing on host 45.79.164.208: false exit status: 1
false stdout: Nothing written
false stderr: Nothing written

SSHKit::Command::Failed: false exit status: 1
false stdout: Nothing written
false stderr: Nothing written

Tasks: TOP => config:false
(See full trace by running task with --trace)

kenn avatar May 18 '16 20:05 kenn

The only thing I can think of is that you are nesting on blocks, like:

Ohh, does that include when invoke is nested? as in:

namespace :config do
  task :false do on roles(:all) do
    execute 'false'
  end end

  task :false_false do on roles(:all) do
    invoke 'config:false'
  end end
end
$ cap staging config:false_false
rvm 1.27.0 (latest) by Wayne E. Seguin <[email protected]>, Michal Papis <[email protected]> [https://rvm.io/]
ruby-2.3.0
ruby 2.3.0p0 (2015-12-25 revision 53290) [x86_64-linux]
00:00 config:false
      01 false
(Backtrace restricted to imported tasks)
cap aborted!
SSHKit::Runner::ExecuteError: Exception while executing on host 12.34.56.78: Exception while executing on host 12.34.56.78: false exit status: 1
false stdout: Nothing written
false stderr: Nothing written

SSHKit::Runner::ExecuteError: Exception while executing on host 12.34.56.78: false exit status: 1
false stdout: Nothing written
false stderr: Nothing written

SSHKit::Command::Failed: false exit status: 1
false stdout: Nothing written
false stderr: Nothing written

Tasks: TOP => config:false_false
(See full trace by running task with --trace)

YES 😄

kenn avatar May 18 '16 20:05 kenn

OK, good, that part is solved. The original issue still stands: SSHKit could do better about not duplicating the error message when a command fails. I'll mark this as a feature request.

mattbrictson avatar May 18 '16 20:05 mattbrictson

Is there any documentation that on blocks shouldn't be nested? If so, shouldn't we raise, or at least warn? Because I just assumed that it would take an intersection of the two role sets. Let me know if I should create a separate ticket in the capistrano repo.

kenn avatar May 18 '16 20:05 kenn

Also from the UX standpoint, this feature should print something like "command failed with non-zero exit status" rather than the use of word "Exception", as ruby-land error implies it's a bug in SSHKit itself rather than an error in the bash land.

kenn avatar May 18 '16 21:05 kenn

Is there any documentation that on blocks shouldn't be nested?

AFAIK nested on blocks were never considered in the design. The fact that it works at all is really just coincidence, due to how the SSHKit DSL is mixed into all objects. IMO it should raise an exception. You can open an SSHKit issue.

Also from the UX standpoint, this feature should print something like "command failed with non-zero exit status" rather than the use of word "Exception"

Agreed; the overall error reporting could be much improved from a UX standpoint. If you would like to update this issue with an example of what you think a good error report would look like, we can use that as a spec for building the feature.

mattbrictson avatar May 18 '16 21:05 mattbrictson

Hi guys, any updates on this issue?

ivanovaleksey avatar Jul 11 '17 06:07 ivanovaleksey

I can't make any guarantees as to if/when this issue will be addressed. However, as I mentioned above, if someone would like to submit an example of what a good error report should look like, that would at least be a starting point for further discussion.

mattbrictson avatar Jul 14 '17 19:07 mattbrictson