better support of liquid syntax errors as well as action errors. Wagon will now display the line where the error happens and the line of codes around the wrong statement

did committed Jan 12, 2017

commit b1feb8b6e04a892225344aafa5da2d074fd6ee90

locomotive/steam/decorators/template_decorator.rb b/lib/locomotive/steam/decorators/template_decorator.rb +1 -1

@@	@@ -38,7 +38,7 @@ module Locomotive
	begin
	Haml::Engine.new(source).render
	rescue Haml::SyntaxError => e
-	raise Steam::RenderError.new(e.message, template_path, source, e.line, e.backtrace)
+	raise Steam::RenderError.new(e, template_path, source)
	end
	end

locomotive/steam/errors.rb b/lib/locomotive/steam/errors.rb +26 -7

@@	@@ -3,18 +3,15 @@ module Locomotive::Steam
	class NoSiteException < ::Exception
	end

-	class ActionException < ::Exception
-	end
-
	class RedirectionException < ::Exception
	alias url message
	end

-	class RenderError < ::StandardError
+	class ParsingRenderingError < ::StandardError

	LINES_RANGE = 10

-	attr_reader :file, :source, :line, :original_backtrace
+	attr_accessor :file, :line, :source, :original_backtrace

	def initialize(message, file, source, line, original_backtrace)
	@file, @source, @line, @original_backtrace = file, source, line, original_backtrace
@@	@@ -27,10 +24,10 @@ module Locomotive::Steam
	lines = source.split("\n")

	start = line - (LINES_RANGE / 2)
-	start = 0 if start < 0
+	start = 1 if start <= 0
	finish = line + (LINES_RANGE / 2)

-	(start..finish).map { \|i\| [i, lines[i]] }
+	(start..finish).map { \|i\| [i, lines[i - 1]] }
	end

	def backtrace
@@	@@ -39,4 +36,26 @@ module Locomotive::Steam

	end

+	class RenderError < ParsingRenderingError
+
+	def initialize(error, file, source)
+	message = error.message
+	line = error.respond_to?(:line_number) ? error.line_number : error.line
+	backtrace = error.backtrace
+
+	super(message, file, source, line, backtrace)
+	end
+
+	end
+
+	class ActionError < ParsingRenderingError
+
+	attr_accessor :action
+
+	def initialize(error, script)
+	super(error.message, nil, script, 0, error.backtrace)
+	end
+
+	end
+
	end

locomotive/steam/liquid/tags/action.rb b/lib/locomotive/steam/liquid/tags/action.rb +6 -1

@@	@@ -38,7 +38,12 @@ module Locomotive

	def render(context)
	Locomotive::Common::Logger.info "[action] executing #{@description}"
-	service(context).run(super, safe_params(context), context)
+	begin
+	service(context).run(super, safe_params(context), context)
+	rescue Locomotive::Steam::ActionError => e
+	e.action = @description
+	raise e
+	end
	''
	end

locomotive/steam/liquid/tags/snippet.rb b/lib/locomotive/steam/liquid/tags/snippet.rb +7 -1

@@	@@ -19,7 +19,13 @@ module Locomotive
	@template_name = evaluate_snippet_name(context)
	# @options doesn't include the page key if cache is on
	@options[:page] = context.registers[:page]
-	super
+
+	begin
+	super
+	rescue Locomotive::Steam::ParsingRenderingError => e
+	e.file = @template_name + ' [Snippet]'
+	raise e
+	end
	end

	private

locomotive/steam/liquid/template.rb b/lib/locomotive/steam/liquid/template.rb +3 -1

@@	@@ -21,7 +21,9 @@ module Locomotive

	def parse(source, options = {})
	template = new
-	template.parse(source, options)
+	template.parse(source, options.merge({
+	line_numbers: true
+	}))
	end

	end

locomotive/steam/middlewares/renderer.rb b/lib/locomotive/steam/middlewares/renderer.rb +6 -1

@@	@@ -37,7 +37,12 @@ module Locomotive::Steam

	def parse_and_render_liquid
	document = services.liquid_parser.parse(page)
-	document.render(liquid_context)
+	begin
+	document.render(liquid_context)
+	rescue Locomotive::Steam::ParsingRenderingError => e
+	e.file = page.template_path if e.file.blank?
+	raise e
+	end
	end

	def liquid_context

locomotive/steam/services/action_service.rb b/lib/locomotive/steam/services/action_service.rb +6 -5

@@	@@ -36,11 +36,12 @@ module Locomotive
	}
	JS

-	# puts script.inspect # DEBUG
-
-	context.exec_string script
-
-	context.call_prop('locomotiveAction', site.as_json, params)
+	begin
+	context.exec_string script
+	context.call_prop('locomotiveAction', site.as_json, params)
+	rescue Exception => e
+	raise Locomotive::Steam::ActionError.new(e, script)
+	end
	end

	private

locomotive/steam/services/liquid_parser_service.rb b/lib/locomotive/steam/services/liquid_parser_service.rb +5 -1

@@	@@ -22,7 +22,11 @@ module Locomotive

	def _parse(object, options = {})
	# Note: the template must not be parsed here
-	Locomotive::Steam::Liquid::Template.parse(object.liquid_source, options)
+	begin
+	Locomotive::Steam::Liquid::Template.parse(object.liquid_source, options)
+	rescue ::Liquid::Error => e
+	raise Locomotive::Steam::RenderError.new(e, object.template_path, object.liquid_source)
+	end
	end

	end

spec/fixtures/default/app/views/pages/about_us/john_doe.fr.liquid.haml +5 -1

spec/fixtures/default/app/views/pages/about_us/john_doe.liquid.haml +1 -1

spec/integration/server/basic_spec.rb +18 -0

@@	@@ -173,4 +173,22 @@ describe Locomotive::Steam::Server do

	end

+	describe 'page with parse or render errors' do
+
+	context 'liquid parsing error' do
+
+	subject { get '/about-us/john-doe'; last_response.body }
+	it { expect { subject }.to raise_error(Locomotive::Steam::RenderError, "Liquid syntax error (line 1): Syntax Error in 'for loop' - Valid syntax: for [item] in [collection]") }
+
+	end
+
+	context 'rendering error' do
+
+	subject { get '/fr/a-notre-sujet/jean-personne'; last_response.body }
+	it { expect { subject }.to raise_error(Locomotive::Steam::ActionError, "identifier 'foo' undefined") }
+
+	end
+
+	end
+
	end

spec/unit/errors_spec.rb +2 -2

@@	@@ -1,6 +1,6 @@
	require 'spec_helper'

-	describe Locomotive::Steam::RenderError do
+	describe Locomotive::Steam::ParsingRenderingError do

	let(:message) { 'Wrong syntax' }
	let(:file) { 'template.liquid.haml' }
@@	@@ -13,7 +13,7 @@ describe Locomotive::Steam::RenderError do

	subject { error.code_lines }

-	it { is_expected.to eq [[5, 'f'], [6, 'g'], [7, 'h'], [8, 'i'], [9, 'j'], [10, 'k'], [11, 'l'], [12, 'm'], [13, 'n'], [14, 'o'], [15, 'p']] }
+	it { is_expected.to eq [[5, 'e'], [6, 'f'], [7, 'g'], [8, 'h'], [9, 'i'], [10, 'j'], [11, 'k'], [12, 'l'], [13, 'm'], [14, 'n'], [15, 'o']] }

	end

spec/unit/liquid/tags/action_spec.rb +9 -0

@@	@@ -12,6 +12,15 @@ describe Locomotive::Steam::Liquid::Tags::Action do

	subject { render_template(source, context) }

+	describe 'parsing' do
+
+	describe 'raises an error if the syntax is incorrect' do
+	let(:source) { '{% action %}{% endaction %}' }
+	it { expect { subject }.to raise_exception(Liquid::SyntaxError) }
+	end
+
+	end
+
	describe 'rendering' do

	it { is_expected.to eq '' }

spec/unit/liquid/tags/link_to_spec.rb +1 -1

@@	@@ -20,7 +20,7 @@ describe Locomotive::Steam::Liquid::Tags::PathTo do
	context 'unknown tag' do

	let(:source) { '{% link_to index %}{% endbar %}{% endlink_to %}' }
-	it { expect { subject }.to raise_error("Liquid syntax error: Unknown tag 'endbar'") }
+	it { expect { subject }.to raise_error("Liquid syntax error (line 1): Unknown tag 'endbar'") }

	end

spec/unit/liquid/tags/paginate_spec.rb +1 -1

@@	@@ -45,7 +45,7 @@ EOF

	subject { output }

-	it { expect { subject }.to raise_error(::Liquid::ArgumentError, "Liquid error: Cannot paginate 'projects'. Not found.") }
+	it { expect { subject }.to raise_error(::Liquid::ArgumentError, "Liquid error (line 1): Cannot paginate 'projects'. Not found.") }

	end

spec/unit/services/action_service_spec.rb +24 -0

@@	@@ -21,6 +21,30 @@ describe Locomotive::Steam::ActionService do

	it { is_expected.to eq 2.0 }

+	describe 'deal with exceptions' do
+
+	context 'wrong syntax' do
+
+	let(:script) { 'a +/ b * var;' }
+
+	it 'raises a meaningful exception' do
+	expect { subject }.to raise_error(Locomotive::Steam::ActionError, "eof or line terminator while parsing regexp (line 2)")
+	end
+
+	end
+
+	context 'other error' do
+
+	let(:script) { 'a.b' }
+
+	it 'raises a meaningful exception' do
+	expect { subject }.to raise_error(Locomotive::Steam::ActionError, "identifier 'a' undefined")
+	end
+
+	end
+
+	end
+
	describe 'with params' do

	let(:params) { { 'foo' => 'hello' } }