Translate exceptions in filters to Liquid::FilterError errors.

Make liquid error rendering optional.

lib/liquid.rb

@@ -50,7 +50,6 @@ require 'liquid/i18n'
 require 'liquid/drop'
 require 'liquid/extensions'
 require 'liquid/errors'
-require 'liquid/error_location'
 require 'liquid/interrupts'
 require 'liquid/strainer'
 require 'liquid/expression'

lib/liquid/context.rb

@@ -12,18 +12,17 @@ module Liquid
   #
   #   context['bob']  #=> nil  class Context
   class Context
-    attr_reader :scopes, :errors, :error_locations, :registers, :environments, :resource_limits
-    attr_accessor :exception_handler, :render_errors, :template_name
+    attr_reader :scopes, :errors, :registers, :environments, :resource_limits
+    attr_accessor :exception_handler, :render_errors
 
-    def initialize(environments = {}, outer_scope = {}, registers = {}, rethrow_errors = false, resource_limits = nil, render_errors = true)
+    def initialize(environments = {}, outer_scope = {}, registers = {}, rethrow_errors = false, resource_limits = nil)
       @environments     = [environments].flatten
       @scopes           = [(outer_scope || {})]
       @registers        = registers
       @errors           = []
-      @error_locations  = []
       @resource_limits  = resource_limits || ResourceLimits.new(Template.default_resource_limits)
       squash_instance_assigns_with_environments
-      @render_errors    = render_errors
+      @render_errors    = true
 
       @this_stack_used = false
 
@@ -66,12 +65,10 @@ module Liquid
 
     def handle_error(e, token = nil)
       if e.is_a?(Liquid::Error)
-        e.template_name = template_name
         e.set_line_number_from_token(token)
       end
 
       errors.push(e)
-      error_locations.push(ErrorLocation.from_token(template_name, token))
       raise if exception_handler && exception_handler.call(e)
       render_errors ? Liquid::Error.render(e) : ''
     end

lib/liquid/error_location.rb

@@ -1,11 +0,0 @@
-module Liquid
-  ErrorLocation = Struct.new(:template_name, :line_number) do
-    def self.line_number_from_token(token)
-      token.respond_to?(:line_number) ? token.line_number : nil
-    end
-
-    def self.from_token(template_name, token)
-      new(template_name, line_number_from_token(token))
-    end
-  end
-end

lib/liquid/errors.rb

@@ -1,7 +1,6 @@
 module Liquid
   class Error < ::StandardError
     attr_accessor :line_number
-    attr_accessor :template_name
     attr_accessor :markup_context
 
     def to_s(with_prefix = true)
@@ -18,8 +17,9 @@ module Liquid
     end
 
     def set_line_number_from_token(token)
+      return unless token.respond_to?(:line_number)
       return if line_number
-      self.line_number = ErrorLocation.line_number_from_token(token)
+      self.line_number = token.line_number
     end
 
     def self.render(e)
@@ -41,9 +41,7 @@ module Liquid
       end
 
       if line_number
-        str << " ("
-        str << template_name << " " if template_name
-        str << "line " << line_number.to_s << ")"
+        str << " (line #{line_number})"
       end
 
       str << ": "
@@ -51,6 +49,10 @@ module Liquid
     end
   end
 
+  class FilterError < Error
+    attr_accessor :original_exception
+  end
+
   ArgumentError = Class.new(Error)
   ContextError = Class.new(Error)
   FileSystemError = Class.new(Error)

lib/liquid/strainer.rb

@@ -47,12 +47,20 @@ module Liquid
 
     def invoke(method, *args)
       if self.class.invokable?(method)
-        send(method, *args)
+        begin
+          send(method, *args)
+        rescue ::ArgumentError => e
+          raise Liquid::ArgumentError.new(e.message)
+        rescue Liquid::Error
+          raise
+        rescue ::StandardError => exception
+          error = Liquid::FilterError.new(exception.message)
+          error.original_exception = exception
+          raise error
+        end
       else
         args.first
       end
-    rescue ::ArgumentError => e
-      raise Liquid::ArgumentError.new(e.message)
     end
   end
 end

lib/liquid/tags/include.rb

@@ -41,9 +41,9 @@ module Liquid
     end
 
     def render(context)
-      template_name = context.evaluate(@template_name_expr)
-      partial = load_cached_partial(template_name, context)
+      partial = load_cached_partial(context)
 
+      template_name = context.evaluate(@template_name_expr)
       context_variable_name = template_name.split('/'.freeze).last
 
       variable = if @variable_name_expr
@@ -52,33 +52,28 @@ module Liquid
         context.find_variable(template_name)
       end
 
-      old_template_name = context.template_name
-      begin
-        context.template_name = template_name
-        context.stack do
-          @attributes.each do |key, value|
-            context[key] = context.evaluate(value)
-          end
+      context.stack do
+        @attributes.each do |key, value|
+          context[key] = context.evaluate(value)
+        end
 
-          if variable.is_a?(Array)
-            variable.collect do |var|
-              context[context_variable_name] = var
-              partial.render(context)
-            end
-          else
-            context[context_variable_name] = variable
+        if variable.is_a?(Array)
+          variable.collect do |var|
+            context[context_variable_name] = var
             partial.render(context)
           end
+        else
+          context[context_variable_name] = variable
+          partial.render(context)
         end
-      ensure
-        context.template_name = old_template_name
       end
     end
 
     private
 
-    def load_cached_partial(template_name, context)
+    def load_cached_partial(context)
       cached_partials = context.registers[:cached_partials] || {}
+      template_name = context.evaluate(@template_name_expr)
 
       if cached = cached_partials[template_name]
         return cached

lib/liquid/template.rb

@@ -112,7 +112,6 @@ module Liquid
 
     def initialize
       @resource_limits = ResourceLimits.new(self.class.default_resource_limits)
-      @render_errors = true
     end
 
     # Parse source code.
@@ -164,8 +163,6 @@ module Liquid
     def render(*args)
       return ''.freeze if @root.nil?
 
-      render_errors = self.render_errors
-
       context = case args.first
       when Liquid::Context
         c = args.shift
@@ -177,15 +174,17 @@ module Liquid
         c
       when Liquid::Drop
         drop = args.shift
-        drop.context = Context.new([drop, assigns], instance_assigns, registers, @rethrow_errors, @resource_limits, render_errors)
+        drop.context = Context.new([drop, assigns], instance_assigns, registers, @rethrow_errors, @resource_limits)
       when Hash
-        Context.new([args.shift, assigns], instance_assigns, registers, @rethrow_errors, @resource_limits, render_errors)
+        Context.new([args.shift, assigns], instance_assigns, registers, @rethrow_errors, @resource_limits)
       when nil
-        Context.new(assigns, instance_assigns, registers, @rethrow_errors, @resource_limits, render_errors)
+        Context.new(assigns, instance_assigns, registers, @rethrow_errors, @resource_limits)
       else
         raise ArgumentError, "Expected Hash or Liquid::Context as parameter"
       end
 
+      context.render_errors = self.render_errors unless self.render_errors.nil?
+
       case args.last
       when Hash
         options = args.pop

test/integration/error_handling_test.rb

@@ -185,4 +185,11 @@ class ErrorHandlingTest < Minitest::Test
       template.render('errors' => ErrorDrop.new)
     end
   end
+
+  def test_disabling_error_rendering
+    template = Liquid::Template.parse('This is an argument error: {{ errors.argument_error }}')
+    template.render_errors = false
+    assert_equal 'This is an argument error: ', template.render('errors' => ErrorDrop.new)
+    assert_equal [ArgumentError], template.errors.map(&:class)
+  end
 end

test/integration/filter_test.rb

@@ -22,6 +22,12 @@ module SubstituteFilter
   end
 end
 
+module ErrorFilter
+  def standard_error(input)
+    raise ::StandardError, 'standard error'
+  end
+end
+
 class FiltersTest < Minitest::Test
   include Liquid
 
@@ -158,7 +164,14 @@ class FiltersInTemplate < Minitest::Test
     assert_equal " 1000$ CAD ", Template.parse("{{1000 | money}}").render!(nil, CanadianMoneyFilter)
     assert_equal " 1000$ CAD ", Template.parse("{{1000 | money}}").render!(nil, [CanadianMoneyFilter])
   end
-end # FiltersTest
+
+  def test_filter_error
+    context = Context.new
+    context.add_filters(ErrorFilter)
+    assert_equal "Liquid error: standard error", Template.parse("{{'var' | standard_error}}").render(context)
+    assert_equal [Liquid::FilterError], context.errors.map(&:class)
+  end
+end
 
 class TestObject
   attr_accessor :a

test/integration/template_test.rb

@@ -201,14 +201,16 @@ class TemplateTest < Minitest::Test
   def test_exception_handler_doesnt_reraise_if_it_returns_false
     exception = nil
     Template.parse("{{ 1 | divided_by: 0 }}").render({}, exception_handler: ->(e) { exception = e; false })
-    assert exception.is_a?(ZeroDivisionError)
+    assert exception.is_a?(Liquid::FilterError)
+    assert exception.original_exception.is_a?(ZeroDivisionError)
   end
 
   def test_exception_handler_does_reraise_if_it_returns_true
     exception = nil
-    assert_raises(ZeroDivisionError) do
+    assert_raises(Liquid::FilterError) do
       Template.parse("{{ 1 | divided_by: 0 }}").render({}, exception_handler: ->(e) { exception = e; true })
     end
-    assert exception.is_a?(ZeroDivisionError)
+    assert exception.is_a?(Liquid::FilterError)
+    assert exception.original_exception.is_a?(ZeroDivisionError)
   end
 end