Improve CLI error reporting

features/cli_error_reporting.feature

@@ -0,0 +1,20 @@
+Feature: CLI error reporting
+
+  Background:
+    Given a recipe with:
+      """
+      task(:trigger_error) { fail 'some error' }
+      """
+
+  Scenario: reports recipe errors
+    When I execute the recipe
+    Then the exit status must be 70
+    And the output must match /\ARuntimeError: some error\n/
+
+  Scenario: reports errors with a backtrace
+    When I execute the recipe
+    Then the output must match /^\s+recipe\.rb:\d+:in /
+
+  Scenario: prepends recipe file path in the backtrace
+    When I execute the recipe
+    Then the output must match /^\s+recipe\.rb \(recipe\)\n\s+recipe\.rb:/

lib/producer/core.rb

@@ -32,10 +32,12 @@ require 'producer/core/tests/has_file'
 require 'producer/core/tests/shell_command_status'
 require 'producer/core/tests/yaml_eq'
 
+require 'producer/core/errors'
+
 require 'producer/core/cli'
 require 'producer/core/condition'
 require 'producer/core/env'
-require 'producer/core/errors'
+require 'producer/core/error_formatter'
 require 'producer/core/logger_formatter'
 require 'producer/core/prompter'
 require 'producer/core/recipe'

lib/producer/core/cli.rb

@@ -17,8 +17,9 @@ module Producer
           rescue ArgumentError => e
             stderr.puts e.message
             exit EX_USAGE
-          rescue RuntimeError => e
-            stderr.puts "#{e.class.name.split('::').last}: #{e.message}"
+          rescue Exception => e
+            ef = ErrorFormatter.new(force_cause: [RecipeEvaluationError])
+            stderr.puts ef.format e
             exit EX_SOFTWARE
           end
         end

lib/producer/core/error_formatter.rb

@@ -0,0 +1,53 @@
+module Producer
+  module Core
+    class ErrorFormatter
+      def initialize(debug: false, force_cause: [])
+        @debug        = debug
+        @force_cause  = force_cause
+      end
+
+      def debug?
+        !!@debug
+      end
+
+      def format(exception)
+        lines = format_exception exception
+
+        if debug? && exception.cause
+          lines << ''
+          lines << 'cause:'
+          lines << format_exception(exception.cause, filter: false)
+        end
+
+        lines.join("\n")
+      end
+
+      private
+
+      def format_exception(exception, filter: true)
+        [
+          format_message(exception),
+          *format_backtrace(exception.backtrace, filter: filter)
+        ]
+      end
+
+      def format_message(exception)
+        exception = exception.cause if @force_cause.include? exception.class
+        "#{exception.class.name.split('::').last}: #{exception.message}"
+      end
+
+      def format_backtrace(backtrace, filter: true)
+        backtrace = filter_backtrace backtrace if filter
+        indent_backtrace backtrace
+      end
+
+      def filter_backtrace(backtrace)
+        backtrace.reject { |l| l =~ /\/producer-\w+\/(?:bin|lib)\// }
+      end
+
+      def indent_backtrace(backtrace)
+        backtrace.map { |e| '  %s' % e }
+      end
+    end
+  end
+end

lib/producer/core/errors.rb

@@ -2,8 +2,10 @@ module Producer
   module Core
     Error                       = Class.new(StandardError)
     RuntimeError                = Class.new(RuntimeError)
+
     ArgumentError               = Class.new(Error)
     ConditionNotMetError        = Class.new(Error)
+    RecipeEvaluationError       = Class.new(RuntimeError)
     RemoteCommandExecutionError = Class.new(RuntimeError)
     RegistryKeyError            = Class.new(RuntimeError)
   end

lib/producer/core/recipe/file_evaluator.rb

@@ -5,7 +5,14 @@ module Producer
         class << self
           def evaluate(file_path, env)
             content = File.read(file_path)
-            Recipe.new(env).tap { |o| o.instance_eval content, file_path }
+            begin
+              Recipe.new(env).tap { |o| o.instance_eval content, file_path }
+            rescue Exception => e
+              fail RecipeEvaluationError, e.message, [
+                '%s (recipe)' % file_path,
+                *e.backtrace
+              ]
+            end
           end
         end
       end

spec/producer/core/cli_spec.rb

@@ -44,7 +44,7 @@ module Producer::Core
         end
       end
 
-      context 'when a runtime error is raised' do
+      context 'when an error is raised' do
         let(:recipe_file) { fixture_path_for 'recipes/raise.rb' }
 
         it 'exits with a return status of 70' do
@@ -52,9 +52,9 @@ module Producer::Core
             .to raise_error(SystemExit) { |e| expect(e.status).to eq 70 }
         end
 
-        it 'prints exception name and message and the error stream' do
+        it 'prints a report to the error stream' do
           expect { trap_exit { run! } }
-            .to output("RemoteCommandExecutionError: false\n").to_stderr
+            .to output(/\ARemoteCommandExecutionError: false$/).to_stderr
         end
       end
     end

spec/producer/core/error_formatter_spec.rb

@@ -0,0 +1,50 @@
+require 'spec_helper'
+
+module Producer
+  module Core
+    describe ErrorFormatter do
+      let(:debug)         { false }
+      let(:force_cause)   { [] }
+      let(:options)       { { debug: debug, force_cause: force_cause } }
+      subject(:formatter) { described_class.new(options) }
+
+      describe '#debug?' do
+        it 'returns false' do
+          expect(formatter.debug?).to be false
+        end
+
+        context 'when debug is enabled' do
+          let(:debug) { true }
+
+          it 'returns true' do
+            expect(formatter.debug?).to be true
+          end
+        end
+      end
+
+      describe '#format' do
+        let(:message)   { 'some exception' }
+        let(:exception) { Exception.new(message) }
+
+        before { exception.set_backtrace %w[back trace] }
+
+        it 'formats the message' do
+          expect(formatter.format exception)
+            .to match /^Exception: some exception$/
+        end
+
+        it 'indents the backtrace' do
+          expect(formatter.format exception).to match /^\s+back$/
+        end
+
+        context 'filtering' do
+          before { exception.set_backtrace %w[back trace /producer-core/lib/] }
+
+          it 'excludes producer code from the backtrace' do
+            expect(formatter.format exception).not_to include 'producer-core'
+          end
+        end
+      end
+    end
+  end
+end