From c72a412600294c99ffc06f00e1d720098ca53019 Mon Sep 17 00:00:00 2001 From: Shawn Neal Date: Tue, 24 Jun 2014 10:09:11 -0700 Subject: [PATCH] Better WinRM command failure messaging Command failures include the stdout and stderr in the error message just like the SSH communicator. Its now possible to specify only an error_class and have that use the correct error_key by default. --- plugins/communicators/winrm/communicator.rb | 19 +++++++++++++------ plugins/communicators/winrm/errors.rb | 4 ++++ templates/locales/comm_winrm.yml | 13 +++++++++++++ .../communicators/winrm/communicator_test.rb | 5 +++-- 4 files changed, 33 insertions(+), 8 deletions(-) diff --git a/plugins/communicators/winrm/communicator.rb b/plugins/communicators/winrm/communicator.rb index 268a3c5d4..5ce188229 100644 --- a/plugins/communicators/winrm/communicator.rb +++ b/plugins/communicators/winrm/communicator.rb @@ -60,8 +60,8 @@ module VagrantPlugins command: command, elevated: false, error_check: true, - error_class: Errors::ExecutionError, - error_key: :execution_error, + error_class: Errors::WinRMBadExitStatus, + error_key: nil, # use the error_class message key good_exit: 0, shell: :powershell, }.merge(opts || {}) @@ -154,10 +154,17 @@ module VagrantPlugins end def raise_execution_error(output, opts) - # The error classes expect the translation key to be _key, but that makes for an ugly - # configuration parameter, so we set it here from `error_key` - msg = "Command execution failed with an exit code of #{output[:exitcode]}" - error_opts = opts.merge(_key: opts[:error_key], message: msg) + # WinRM can return multiple stderr and stdout entries + error_opts = opts.merge( + stdout: output[:data].collect { |e| e[:stdout] }.join, + stderr: output[:data].collect { |e| e[:stderr] }.join + ) + + # Use a different error message key if the caller gave us one, + # otherwise use the error's default message + error_opts[:_key] = opts[:error_key] if opts[:error_key] + + # Raise the error, use the type the caller gave us or the comm default raise opts[:error_class], error_opts end end #WinRM class diff --git a/plugins/communicators/winrm/errors.rb b/plugins/communicators/winrm/errors.rb index 2285d2115..552b8a300 100644 --- a/plugins/communicators/winrm/errors.rb +++ b/plugins/communicators/winrm/errors.rb @@ -18,6 +18,10 @@ module VagrantPlugins error_key(:invalid_shell) end + class WinRMBadExitStatus < WinRMError + error_key(:winrm_bad_exit_status) + end + class WinRMNotReady < WinRMError error_key(:winrm_not_ready) end diff --git a/templates/locales/comm_winrm.yml b/templates/locales/comm_winrm.yml index 0189547f9..9de47a9cf 100644 --- a/templates/locales/comm_winrm.yml +++ b/templates/locales/comm_winrm.yml @@ -8,6 +8,19 @@ en: Password: %{password} Endpoint: %{endpoint} Message: %{message} + winrm_bad_exit_status: |- + The following WinRM command responded with a non-zero exit status. + Vagrant assumes that this means the command failed! + + %{command} + + Stdout from the command: + + %{stdout} + + Stderr from the command: + + %{stderr} execution_error: |- An error occurred executing a remote WinRM command. diff --git a/test/unit/plugins/communicators/winrm/communicator_test.rb b/test/unit/plugins/communicators/winrm/communicator_test.rb index 80d11d4c6..1433f4317 100644 --- a/test/unit/plugins/communicators/winrm/communicator_test.rb +++ b/test/unit/plugins/communicators/winrm/communicator_test.rb @@ -61,9 +61,10 @@ describe VagrantPlugins::CommunicatorWinRM::Communicator do end it "raises error when error_check is true and exit code is non-zero" do - expect(shell).to receive(:powershell).with(kind_of(String)).and_return({ exitcode: 1 }) + expect(shell).to receive(:powershell).with(kind_of(String)).and_return( + { exitcode: 1, data: [{ stdout: '', stderr: '' }] }) expect { subject.execute("dir") }.to raise_error( - VagrantPlugins::CommunicatorWinRM::Errors::ExecutionError) + VagrantPlugins::CommunicatorWinRM::Errors::WinRMBadExitStatus) end it "does not raise error when error_check is false and exit code is non-zero" do