Merge pull request #8562 from brodock/8561-include-filename-template-error

Improving error handling for template render

3 files changed, 64 insertions, 9 deletions

diff --git a/lib/chef/mixin/template.rb b/lib/chef/mixin/template.rb
index e230044bae..f32b560e0e 100644
--- a/lib/chef/mixin/template.rb
+++ b/lib/chef/mixin/template.rb
@@ -138,11 +138,11 @@ class Chef
           partial_context._extend_modules(@_extension_modules)
 
           template_location = @template_finder.find(partial_name, options)
-          _render_template(IO.binread(template_location), partial_context)
+          _render_template(IO.binread(template_location), partial_context, filename: template_location)
         end
 
         def render_template(template_location)
-          _render_template(IO.binread(template_location), self)
+          _render_template(IO.binread(template_location), self, filename: template_location)
         end
 
         def render_template_from_string(template)
@@ -153,12 +153,13 @@ class Chef
         # INTERNAL PUBLIC API
         ###
 
-        def _render_template(template, context)
+        def _render_template(template, context, options = {})
           begin
-            eruby = Erubis::Eruby.new(template)
+            # eruby = Erubis::Eruby.new(template, options)
+            eruby = Erubis::Eruby.new(template, options)
             output = eruby.evaluate(context)
           rescue Object => e
-            raise TemplateError.new(e, template, context)
+            raise TemplateError.new(e, template, context, options)
           end
 
           # CHEF-4399
@@ -210,11 +211,11 @@ class Chef
       end
 
       class TemplateError < RuntimeError
-        attr_reader :original_exception, :context
+        attr_reader :original_exception, :context, :options
         SOURCE_CONTEXT_WINDOW = 2
 
-        def initialize(original_exception, template, context)
-          @original_exception, @template, @context = original_exception, template, context
+        def initialize(original_exception, template, context, options)
+          @original_exception, @template, @context, @options = original_exception, template, context, options
         end
 
         def message
@@ -222,7 +223,11 @@ class Chef
         end
 
         def line_number
-          @line_number ||= $1.to_i if original_exception.backtrace.find { |line| line =~ /\(erubis\):(\d+)/ }
+          @line_number ||= if options[:filename]
+                             $1.to_i if original_exception.backtrace.find { |line| line =~ /#{Regexp.escape(options[:filename])}:(\d+)/ }
+                           else
+                             $1.to_i if original_exception.backtrace.find { |line| line =~ /\(erubis\):(\d+)/ }
+                           end
         end
 
         def source_location
diff --git a/spec/data/templates/failed.erb b/spec/data/templates/failed.erb
new file mode 100644
index 0000000000..e077ac8684
--- /dev/null
+++ b/spec/data/templates/failed.erb
@@ -0,0 +1,5 @@
+This is a template
+
+Which includes some content
+
+And will fail <%= nil[] %>
diff --git a/spec/unit/mixin/template_spec.rb b/spec/unit/mixin/template_spec.rb
index ab7ed5bc5a..96b983a9dd 100644
--- a/spec/unit/mixin/template_spec.rb
+++ b/spec/unit/mixin/template_spec.rb
@@ -182,6 +182,51 @@ describe Chef::Mixin::Template, "render_template" do
       expect(output).to eq("before {partial one We could be diving for pearls! calling home} after")
     end
 
+    describe "when an exception is raised in the template" do
+      let(:template_file) { File.expand_path(File.join(CHEF_SPEC_DATA, "templates", "failed.erb")) }
+
+      def do_raise
+        @template_context.render_template(template_file)
+      end
+
+      it "should catch and re-raise the exception as a TemplateError" do
+        expect { do_raise }.to raise_error(Chef::Mixin::Template::TemplateError)
+      end
+
+      describe "the raised TemplateError" do
+        subject(:exception) do
+          begin
+            do_raise
+          rescue Chef::Mixin::Template::TemplateError => e
+            e
+          end
+        end
+
+        it "should contain template file and line numbers" do
+          expect(exception.line_number).to eq(5)
+        end
+
+        it "should provide a source listing of the template around the exception" do
+          expect(exception.source_listing).to eq("  3: Which includes some content\n  4: \n  5: And will fail <%= nil[] %>")
+        end
+
+        it "should provide a nice source location" do
+          expect(exception.source_location).to eq("on line #5")
+        end
+
+        it "should create a pretty output for the terminal" do
+          expect(exception.to_s).to match(/Chef::Mixin::Template::TemplateError/)
+          expect(exception.to_s).to match(/undefined method `\[\]' for nil:NilClass/)
+          expect(exception.to_s).to include("  3: Which includes some content\n  4: \n  5: And will fail <%= nil[] %>")
+          expect(exception.to_s).to include(exception.original_exception.backtrace.first)
+        end
+
+        it "should include template file on original_exception backtrace" do
+          expect(exception.original_exception.backtrace).to include(/#{Regexp.escape(template_file)}/)
+        end
+      end
+    end
+
     describe "when customizing the template context" do
 
       it "extends the context to include modules" do