Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
kenn opened this issue May 18, 2016 · 10 comments
Open

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

kenn opened this issue May 18, 2016 · 10 comments

Comments

@kenn
Copy link

kenn commented May 18, 2016

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.

@leehambley
Copy link
Member

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.

@mattbrictson
Copy link
Member

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

@kenn
Copy link
Author

kenn commented May 18, 2016

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 <wayneeseguin@gmail.com>, Michal Papis <mpapis@gmail.com> [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
Copy link
Author

kenn commented May 18, 2016

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 <wayneeseguin@gmail.com>, Michal Papis <mpapis@gmail.com> [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 😄

@mattbrictson
Copy link
Member

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 mattbrictson changed the title Redundant error messages on execute failure Feature: Don't display redundant error messages on execute failure May 18, 2016
@kenn
Copy link
Author

kenn commented May 18, 2016

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
Copy link
Author

kenn commented May 18, 2016

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.

@mattbrictson
Copy link
Member

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.

@ivanovaleksey
Copy link

Hi guys, any updates on this issue?

@mattbrictson
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants