Add handling for multi-target errors

This wraps the actions for a multi-target command
(converge in this case) in a Terminal::Job instance,
which tracks its failure state.

After all jobs are complete, this collects those that failed
and sends them over to ErrorPrinter for formatting and capture
to file, while a top level MULTI001 (basically: one or more jobs failed)
error is shown to the user with the location of the errors file.

Signed-off-by: Marc A. Paradise <marc.paradise@gmail.com>

components/chef-workstation/i18n/en.yml

@@ -301,7 +301,7 @@ errors:
 
   CHEFUPL003: |
     Uploading config to target failed.
-  
+
   CHEFUPL004: |
     Uploading handler to target failed.
 
@@ -318,10 +318,18 @@ errors:
       [connection.winrm]
       ssl_verify=false
 
+  CHEFMULTI001: |
+    One or more actions has failed.
+
+    A complete list of failures and possible resolutions can
+    be found in the file below:
+
+      %1
+
   footer:
     both: |
       If you are not able to resolve this issue, please contact Chef support
-      at beta@chef.io and include the log file and strack trace from the
+      at beta@chef.io and include the log file and stack trace from the
       locations below:
 
         %1

components/chef-workstation/lib/chef-workstation/cli.rb

@@ -46,7 +46,6 @@ def run
         setup_cli()
         perform_command()
       end
-
     rescue WrappedError => e
       UI::ErrorPrinter.show_error(e)
       @rc = RC_COMMAND_FAILED
@@ -82,7 +81,6 @@ def perform_command
       update_args_for_help
       update_args_for_version
       root_command, *leftover = @argv
-
       run_command!(root_command, leftover)
     rescue => e
       handle_perform_error(e)

components/chef-workstation/lib/chef-workstation/command/base.rb

@@ -191,6 +191,18 @@ def show_help_aliases
         end
       end
 
+      # When running multiple jobs, exceptions are captured to the
+      # job to avoid interrupting other jobs in process.  This function
+      # collects them and raises a MultiJobFailure if failure has occurred;
+      # we do *not* differentiate between one failed jobs and multiple failed jobs
+      # - if you're in the 'multi-job' path (eg, multiple targets) we handle
+      # all errors the same to provide a consistent experience.
+      def handle_job_failures(jobs)
+        failed_jobs = jobs.select { |j| !j.exception.nil? }
+        return if failed_jobs.empty?
+        raise ChefWorkstation::MultiJobFailure.new(failed_jobs)
+      end
+
       def subcommands
         # The base class behavior subcommands are actually the full list
         # of top-level commands - those are subcommands of 'chef'.

components/chef-workstation/lib/chef-workstation/command/target/converge.rb

@@ -89,27 +89,29 @@ def run(params)
         def run_single_target(initial_status_msg, target_host, converge_args)
           connect_target(target_host)
           prefix = "[#{target_host.hostname}]"
-          UI::Terminal.render_action(TS.install_chef.verifying, prefix: prefix) do |r|
-            install(target_host, r)
+          UI::Terminal.render_job(TS.install_chef.verifying, prefix: prefix) do |reporter|
+            install(target_host, reporter)
           end
-          UI::Terminal.render_action(initial_status_msg, prefix: "[#{target_host.hostname}]") do |r|
+          UI::Terminal.render_job(initial_status_msg, prefix: "[#{target_host.hostname}]") do |r|
             converge(r, converge_args.merge(target_host: target_host))
           end
         end
 
         def run_multi_target(initial_status_msg, target_hosts, converge_args)
           # Our multi-host UX does not show a line item per action,
           # but rather a line-item per connection.
-          actions = target_hosts.map do |target_host|
-            UI::Terminal::Action.new("[#{target_host.hostname}]") do |reporter|
+          jobs = target_hosts.map do |target_host|
+            # This block will run in its own thread during render.
+            UI::Terminal::Job.new("[#{target_host.hostname}]", target_host) do |reporter|
               connect_target(target_host, reporter)
               reporter.update(TS.install_chef.verifying)
               install(target_host, reporter)
               reporter.update(initial_status_msg)
               converge(reporter, converge_args.merge(target_host: target_host))
             end
           end
-          UI::Terminal.render_parallel_actions(TS.converge.multi_header, actions)
+          UI::Terminal.render_parallel_jobs(TS.converge.multi_header, jobs)
+          handle_job_failures(jobs)
         end
 
         # The first param is always hostname. Then we either have
@@ -217,25 +219,25 @@ def recipe_strategy?(cli_arguments)
 
         # Runs the InstallChef action and renders UI updates as
         # the action reports back
-        def install(target_host, r)
+        def install(target_host, reporter)
           installer = Action::InstallChef.instance_for_target(target_host)
           context = Text.status.install_chef
           installer.run do |event, data|
             case event
             when :installing
-              r.update(context.installing)
+              reporter.update(context.installing)
             when :uploading
-              r.update(context.uploading)
+              reporter.update(context.uploading)
             when :downloading
-              r.update(context.downloading)
+              reporter.update(context.downloading)
             when :success
               meth = @multi_target ? :update : :success
               msg = (data[0] == :already_installed) ? context.already_present : context.success
-              r.send(meth, msg)
+              reporter.send(meth, msg)
             when :error
               # Message may or may not be present. First arg if it is.
               msg = data.length > 0 ? data[0] : T.aborted
-              r.error(context.failure(msg))
+              reporter.error(context.failure(msg))
             end
           end
         end

components/chef-workstation/lib/chef-workstation/config.rb

@@ -28,6 +28,10 @@ def default_location
         File.join(WS_BASE_PATH, "config.toml")
       end
 
+      def error_output_path
+        File.join(File.dirname(log.location), "errors.txt")
+      end
+
       def stack_trace_path
         File.join(File.dirname(log.location), "stack-trace.log")
       end

components/chef-workstation/lib/chef-workstation/error.rb

@@ -20,7 +20,7 @@ class Error < StandardError
     attr_accessor :show_stack, :show_log, :decorate
     def initialize(id, *params)
       @id = id
-      @params = params
+      @params = params || []
       @show_log = true
       @show_stack = true
       @decorate = true
@@ -52,6 +52,16 @@ def initialize(e, target_host)
     end
   end
 
+  class MultiJobFailure < ChefWorkstation::ErrorNoLogs
+    attr_reader :jobs
+    def initialize(jobs)
+      super("CHEFMULTI001")
+      @jobs = jobs
+      @decorate = false
+    end
+  end
+
+
   # Provides mappings of common errors that we don't explicitly
   # handle, but can offer expanded help text around.
   class StandardErrorResolver

components/chef-workstation/lib/chef-workstation/ui/error_printer.rb

@@ -23,7 +23,7 @@
 
 module ChefWorkstation::UI
   class ErrorPrinter
-    attr_reader :pastel, :show_log, :show_stack, :exception, :target_host
+    attr_reader :id, :pastel, :show_log, :show_stack, :exception, :target_host
     # TODO define 't' as a method is a temporary workaround
     # to ensure that text key lookups are testable.
     def t
@@ -33,13 +33,43 @@ def t
     DEFAULT_ERROR_NO = "CHEFINT001"
 
     def self.show_error(e)
+      # Name is misleading - it's unwrapping but also doing furtehr
+      # error resolution for common errors:
       unwrapped = ChefWorkstation::StandardErrorResolver.unwrap_exception(e)
+      if unwrapped.class == ChefWorkstation::MultiJobFailure
+        capture_multiple_failures(unwrapped)
+      end
       formatter = ErrorPrinter.new(e, unwrapped)
       Terminal.output(formatter.format_error)
     rescue => e
       dump_unexpected_error(e)
     end
 
+    def self.capture_multiple_failures(e)
+      out_file = ChefWorkstation::Config.error_output_path
+      e.params << out_file # Tell the operator where to find this info
+      File.open(out_file, "w") do |out|
+        e.jobs.each do |j|
+          # ErrorPrinter only instantiates with a WrappedError:
+          wrapped = ChefWorkstation::WrappedError.new(j.exception, j.target_host)
+          # This is silly, but necessary 'til we clean up (or do away with)
+          # wrapped errors because 'unwrap_exception' does further processing to resolve
+          # the error
+          unwrapped = ChefWorkstation::StandardErrorResolver.unwrap_exception(wrapped)
+          ep = ErrorPrinter.new(wrapped, unwrapped)
+          msg = ep.format_body().tr("\n", " ").gsub(/ {2,}/, " ")
+          out.write("Host: #{j.target_host.hostname} ")
+          if unwrapped.respond_to? :id
+            out.write("Error: #{unwrapped.id}: ")
+          else
+            out.write(": ")
+          end
+
+          out.write("#{msg}\n")
+        end
+      end
+    end
+
     def self.write_backtrace(e, args)
       formatter = ErrorPrinter.new(e)
       out = StringIO.new

components/chef-workstation/lib/chef-workstation/ui/terminal.rb

@@ -7,11 +7,20 @@
 module ChefWorkstation
   module UI
     class Terminal
-      class Action
-        attr_reader :proc, :prefix
-        def initialize(prefix, &block)
+      class Job
+        attr_reader :proc, :prefix, :target_host, :exception
+        def initialize(prefix, target_host, &block)
           @proc = block
           @prefix = prefix
+          @target_host = target_host
+          @error = nil
+        end
+
+        def run(reporter)
+          @proc.call(reporter)
+        rescue => e
+          reporter.error(e.to_s)
+          @exception = e
         end
       end
 
@@ -36,19 +45,20 @@ def output(msg)
           @location.puts msg
         end
 
-        def render_parallel_actions(header, actions, prefix: "")
+        def render_parallel_jobs(header, actions, prefix: "")
           multispinner = TTY::Spinner::Multi.new("[:spinner] #{header}")
           actions.each do |a|
             multispinner.register(":spinner #{a.prefix} :status") do |spinner|
               reporter = StatusReporter.new(spinner, prefix: prefix, key: :status)
-              a.proc.call(reporter)
+              a.run(reporter)
             end
           end
           multispinner.auto_spin
         end
 
-        def render_action(msg, prefix: "", &block)
-          klass = Object.const_get("ChefWorkstation::UI::#{ChefWorkstation::Config.dev.spinner}")
+        # TODO this should also accept a job.
+        def render_job(msg, prefix: "", &block)
+          klass = ChefWorkstation::UI.const_get(ChefWorkstation::Config.dev.spinner)
           spinner = klass.new("[:spinner] :prefix :status", output: @location)
           reporter = StatusReporter.new(spinner, prefix: prefix, key: :status)
           reporter.update(msg)

components/chef-workstation/spec/unit/ui/terminal_spec.rb

@@ -20,32 +20,81 @@
     end.to output("test\n").to_terminal
   end
 
-  context "#render_action" do
-    it "executes the provided action" do
+  context "#render_job" do
+    it "executes the provided block" do
       @ran = false
-      Terminal.render_action("a message") { |reporter| @ran = true }
+      Terminal.render_job("a message") { |reporter| @ran = true }
       expect(@ran).to eq true
     end
   end
 
-  context "#render_parallel_actions" do
-    it "executes the provided actions" do
-      @action1ran = false
-      @action2ran = false
-      action1 = Terminal::Action.new("prefix") do
-        @action1ran = true
+  context "#render_parallel_jobs" do
+    it "executes the provided job instances" do
+      @job1ran = false
+      @job2ran = false
+      job1 = Terminal::Job.new("prefix", nil) do
+        @job1ran = true
       end
-      action2 = Terminal::Action.new("prefix") do
-        @action2ran = true
+      job2 = Terminal::Job.new("prefix", nil) do
+        @job2ran = true
       end
-      Terminal.render_parallel_actions("a message", [action1, action2])
-      expect(@action1ran).to eq true
-      expect(@action2ran).to eq true
+      Terminal.render_parallel_jobs("a message", [job1, job2])
+      expect(@job1ran).to eq true
+      expect(@job2ran).to eq true
     end
   end
 
-  # The spinner REALLY doesn't want to send output to anything besides a real
-  # stdout. Maybe it has something to do with a tty check?
-  it "correctly passes a block to the spinner and executes it" do
+  describe ChefWorkstation::UI::Terminal::Job do
+    subject { ChefWorkstation::UI::Terminal::Job }
+    context "#exception" do
+      context "when no exception occurs in execution" do
+        context "and it's been invoked directly" do
+          it "exception is nil" do
+            job = subject.new("", nil) { 0 }
+            job.run(ChefWorkstation::MockReporter.new)
+            expect(job.exception).to eq nil
+          end
+        end
+        context "and it's running in a thread alongside other jobs" do
+          it "exception is nil for each job" do
+            job1 = subject.new("", nil) { 0 }
+            job2 = subject.new("", nil) { 0 }
+            threads = []
+            threads << Thread.new { job1.run(ChefWorkstation::MockReporter.new) }
+            threads << Thread.new { job2.run(ChefWorkstation::MockReporter.new) }
+            threads.each(&:join)
+            expect(job1.exception).to eq nil
+            expect(job2.exception).to eq nil
+
+          end
+        end
+      end
+      context "when an exception occurs in execution" do
+        context "and it's been invoked directly" do
+          it "captures the exception in #exception" do
+            expected_exception = StandardError.new("exception 1")
+            job = subject.new("", nil) { |arg| raise expected_exception }
+            job.run(ChefWorkstation::MockReporter.new)
+            expect(job.exception).to eq expected_exception
+          end
+        end
+
+        context "and it's running in a thread alongside other jobs" do
+          it "each job holds its own exception" do
+            e1 = StandardError.new("exception 1")
+            e2 = StandardError.new("exception 2")
+
+            job1 = subject.new("", nil) { |_| raise e1 }
+            job2 = subject.new("", nil) { |_| raise e2 }
+            threads = []
+            threads << Thread.new { job1.run(ChefWorkstation::MockReporter.new) }
+            threads << Thread.new { job2.run(ChefWorkstation::MockReporter.new) }
+            threads.each(&:join)
+            expect(job1.exception).to eq e1
+            expect(job2.exception).to eq e2
+          end
+        end
+      end
+    end
   end
 end