Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Update and spec out error recovery behavior.

Error callbacks are now specified at a controller level, rather than
globally. Error rendering may also be overridden per-controller too.
  • Loading branch information...
commit 995bba8c19d2b8a3f7bfb24d96a469e7b3135eb1 1 parent 43072c5
@oggy authored
View
5 lib/template_streaming.rb
@@ -7,6 +7,8 @@ def configure(config)
end
end
+ PROGRESSIVE_KEY = 'template_streaming.progressive'.freeze
+
module Controller
def self.included(base)
base.class_eval do
@@ -72,6 +74,7 @@ def render_with_template_streaming(*args, &block)
#
flash # ensure sweep
@template_streaming_flash = @_flash
+ request.env[PROGRESSIVE_KEY] = true
run_callbacks :when_streaming_template
else
@@ -368,3 +371,5 @@ def pre_process_with_template_streaming(*args, &block)
end
end
end
+
+require 'template_streaming/error_recovery'
View
255 lib/template_streaming/error_recovery.rb
@@ -1,26 +1,7 @@
module TemplateStreaming
- class << self
- #
- # Call the given block when an error occurs during rendering.
- #
- # The block is called with the exception object.
- #
- # This is where you should hook in your exception notification
- # system of choice (Hoptoad, Exceptional, etc.)
- #
- def on_render_error(&block)
- ErrorRecovery.callbacks << block
- end
- end
-
module ErrorRecovery
- class << self
- attr_accessor :callbacks
- end
- self.callbacks = []
-
- EXCEPTIONS_KEY = 'template_streaming.exceptions'.freeze
- CONTROLLER_KEY = 'template_streaming.template'.freeze
+ CONTROLLER_KEY = 'template_streaming.error_recovery.controller'.freeze
+ EXCEPTIONS_KEY = 'template_streaming.error_recovery.exceptions'.freeze
class Middleware
def initialize(app)
@@ -28,80 +9,216 @@ def initialize(app)
end
def call(env)
- @env = env
- env[EXCEPTIONS_KEY] = []
- status, headers, @body = *@app.call(env)
- [status, headers, self]
+ response = *@app.call(env)
+ if env[TemplateStreaming::PROGRESSIVE_KEY]
+ response[2] = BodyProxy.new(env, response[2].body)
+ response
+ else
+ response
+ end
end
- def each(&block)
- controller = @env[CONTROLLER_KEY]
- if controller && controller.send(:local_request?)
- exceptions = @env[EXCEPTIONS_KEY]
- template = controller.response.template
- @body.each do |chunk|
- if !exceptions.empty? && (insertion_point = chunk =~ %r'</body\s*>\s*(?:</html\s*>\s*)?\z'im)
- chunk.insert(insertion_point, template.render_exceptions(exceptions))
- exceptions.clear
+ class BodyProxy
+ def initialize(env, body)
+ @env = env
+ @body = body
+ @controller = @env[CONTROLLER_KEY]
+ end
+
+ def each(&block)
+ if @controller && @controller.show_errors?
+ exceptions = @env[EXCEPTIONS_KEY] = []
+ @state = :start
+ @body.each do |chunk|
+ advance_state(chunk)
+ if !exceptions.empty?
+ try_to_insert_errors(chunk, exceptions)
+ end
+ yield chunk
end
- yield chunk
+ if !exceptions.empty?
+ yield uninserted_errors(exceptions)
+ end
+ else
+ @body.each(&block)
end
- if !exceptions.empty?
- yield template.render_exceptions(exceptions)
+ end
+
+ def advance_state(chunk, cursor=0)
+ case @state
+ when :start
+ if index = chunk.index(%r'<!doctype\b.*?>'i, cursor)
+ @state = :before_html
+ advance_state(chunk, index)
+ end
+ when :before_html
+ if index = chunk.index(%r'<html\b'i, cursor)
+ @state = :before_head
+ advance_state(chunk, index)
+ end
+ when :before_head
+ if index = chunk.index(%r'<head\b'i, cursor)
+ @state = :in_head
+ advance_state(chunk, index)
+ end
+ when :in_head
+ if index = chunk.index(%r'</head\b.*?>'i, cursor)
+ @state = :between_head_and_body
+ advance_state(chunk, index)
+ end
+ when :between_head_and_body
+ if index = chunk.index(%r'<body\b'i, cursor)
+ @state = :in_body
+ advance_state(chunk, index)
+ end
+ when :in_body
+ if index = chunk.index(%r'</body\b.*?>'i, cursor)
+ @state = :after_body
+ advance_state(chunk, index)
+ end
+ when :after_body
+ if index = chunk.index(%r'</html\b.*?>'i, cursor)
+ @state = :after_html
+ advance_state(chunk, index)
+ end
end
- else
- @body.each(&block)
+ end
+
+ def try_to_insert_errors(chunk, exceptions)
+ if (index = chunk =~ %r'</body\s*>\s*(?:</html\s*>\s*)?\z'im)
+ chunk.insert(index, render_exceptions(exceptions))
+ exceptions.clear
+ end
+ end
+
+ def uninserted_errors(exceptions)
+ html = render_exceptions(exceptions)
+ exceptions.clear
+ case @state
+ when :start
+ head = "<head><title>Unhandled Exception</title></head>"
+ body = "<body>#{html}</body>"
+ "<!DOCTYPE html><html>#{head}#{body}</html>"
+ when :before_html
+ head = "<head><title>Unhandled Exception</title></head>"
+ body = "<body>#{html}</body>"
+ "<html>#{head}#{body}</html>"
+ when :before_head
+ head = "<head><title>Unhandled Exception</title></head>"
+ body = "<body>#{html}</body>"
+ "#{head}#{body}</html>"
+ when :in_head
+ "</head><body>#{html}</body></html>"
+ when :between_head_and_body
+ "<body>#{html}</body></html>"
+ when :in_body
+ "#{html}</body></html>"
+ when :after_body
+ # Errors aren't likely to happen at this point, as after the body
+ # there should only be "</html>". Just stick our error html in there
+ # - it's invalid HTML no matter what we do.
+ "#{html}</html>"
+ when :after_html
+ html
+ end
+ end
+
+ def render_exceptions(exceptions)
+ template = @controller.response.template
+ template.render_exceptions(exceptions)
end
end
end
module Controller
def self.included(base)
- base.when_streaming_template :recover_from_errors
- base.helper Helper
- base.helper_method :recover_from_errors?
+ base.when_streaming_template :set_template_streaming_controller
+ base.class_inheritable_accessor :progressive_rendering_error_callbacks
+ base.class_inheritable_accessor :progressive_rendering_error_renderer
+ base.progressive_rendering_error_callbacks = []
+ base.extend ClassMethods
+ end
+
+ module ClassMethods
+ #
+ # Call the given block when an error occurs while rendering
+ # progressively.
+ #
+ # The block is called with the controller instance and exception object.
+ #
+ # Hook in your exception notification system here.
+ #
+ def on_progressive_rendering_error(&block)
+ progressive_rendering_error_callbacks << block
+ end
+
+ #
+ # Call the give block to render errors injected into the page, when
+ # uncaught exceptions are raised while progressively rendering.
+ #
+ # The block is called with the view instance an list of exception
+ # objects. It should return the HTML to inject into the page.
+ #
+ def render_errors_progressively_with(&block)
+ self.progressive_rendering_error_renderer = block
+ end
end
- def recover_from_errors
- @recover_from_errors = true
+ def set_template_streaming_controller
request.env[CONTROLLER_KEY] = self
end
- def recover_from_errors?
- @recover_from_errors
+ def show_errors?
+ local_request?
end
end
- module Helper
- def render_partial(*)
- begin
- super
- rescue ActionView::MissingTemplate => e
- # ActionView uses this as a signal to try another template engine.
- raise e
- rescue Exception => e
- raise e if !recover_from_errors?
- Rails.logger.error("#{e.class}: #{e.message}")
- Rails.logger.error(e.backtrace.join("\n").gsub(/^/, ' '))
- callbacks = ErrorRecovery.callbacks and
- callbacks.each{|c| c.call(e)}
- request.env[EXCEPTIONS_KEY] << e
- ''
+ module View
+ def self.included(base)
+ base.class_eval do
+ alias_method_chain :render, :template_streaming_error_recovery
+ end
+ end
+
+ def render_with_template_streaming_error_recovery(*args, &block)
+ if render_progressively?
+ begin
+ render_without_template_streaming_error_recovery(*args, &block)
+ rescue ActionView::MissingTemplate => e
+ # ActionView uses this as a signal to try another template format.
+ raise e
+ rescue Exception => e
+ logger.error "#{e.class}: #{e.message}"
+ logger.error e.backtrace.join("\n").gsub(/^/, ' ')
+ controller.progressive_rendering_error_callbacks.each{|c| c.call(e)}
+ exceptions = controller.request.env[EXCEPTIONS_KEY] and
+ exceptions << e
+ ''
+ end
+ else
+ render_without_template_streaming_error_recovery(*args, &block)
end
end
def render_exceptions(exceptions)
- @content = exceptions.map do |exception|
- template_path = ActionController::Rescue::RESCUES_TEMPLATE_PATH
- @exception = exception
- @rescues_path = template_path
- render :file => "#{template_path}/rescues/template_error.erb"
- end.join
- render :file => "#{File.dirname(__FILE__)}/templates/errors.erb"
+ controller.progressive_rendering_error_renderer.call(self, exceptions)
end
end
- ActionController::Base.send :include, Controller
+ DEFAULT_ERROR_RENDERER = lambda do |view, exceptions|
+ @content = exceptions.map do |exception|
+ template_path = ActionController::Rescue::RESCUES_TEMPLATE_PATH
+ @exception = exception
+ @rescues_path = template_path
+ view.render :file => "#{template_path}/rescues/template_error.erb"
+ end.join
+ view.render :file => "#{File.dirname(__FILE__)}/templates/errors.erb"
+ end
+
ActionController::Dispatcher.middleware.insert_after ActionController::Failsafe, Middleware
+ ActionController::Base.send :include, Controller
+ ActionView::Base.send :include, View
+
+ ActionController::Base.progressive_rendering_error_renderer = DEFAULT_ERROR_RENDERER
end
end
View
2  spec/support/progressive_rendering_test.rb
@@ -14,6 +14,8 @@ def setup_progressive_rendering_test
push_constant_value Object, :TestController, Class.new(Controller)
TestController.view_paths = [VIEW_PATH]
+ @log_buffer = ''
+ TestController.logger = Logger.new(StringIO.new(@log_buffer))
FileUtils.rm_rf VIEW_PATH
FileUtils.mkdir_p VIEW_PATH
View
233 spec/template_streaming_spec.rb
@@ -838,4 +838,237 @@ def render_call(layout, partial, style)
end
end
end
+
+ describe "when there is an error during rendering" do
+ before do
+ class TestController
+ def rescue_action(exception)
+ # Run the default handler.
+ ActionController::Base.instance_method(:rescue_action).bind(self).call(exception)
+ end
+ end
+ end
+
+ describe "when not progressively rendering" do
+ it "should show the standard error page" do
+ view "<% raise 'test exception' %>"
+ run
+ received.should include("Action Controller: Exception caught")
+ received.should include('test exception')
+ end
+ end
+
+ describe "when progressively rendering" do
+ before do
+ controller.render_errors_progressively_with do |view, exceptions|
+ messages = exceptions.map { |e| e.original_exception.message }
+ "(#{messages.join(',')})"
+ end
+ end
+
+ describe "for local requests" do
+ before do
+ controller.class_eval do
+ def local_request?
+ true
+ end
+ end
+ end
+
+ it "should run the error callback for each error raised" do
+ messages = []
+ controller.on_progressive_rendering_error do |error|
+ messages << error.original_exception.message
+ end
+ view "<% render :partial => 'a' %><% render :partial => 'b' %>"
+ template 'test/_a', "<% raise 'a' %>"
+ template 'test/_b', "<% raise 'b' %>"
+ action { render :progressive => true, :layout => nil }
+ run
+ messages.should == ['a', 'b']
+ end
+
+ describe "when a structurally-complete response is rendered" do
+ before do
+ view "<% raise 'x' %>"
+ action { render :progressive => true, :layout => 'layout' }
+ end
+
+ it "should inject errors correctly when the error occurs before the doctype" do
+ layout "<%= yield %><!DOCTYPE html><html><head></head><body></body></html>"
+ run
+ received.should == chunks("<!DOCTYPE html><html><head></head><body>(x)</body></html>", :end => true)
+ end
+
+ it "should inject errors correctly when the error occurs before the opening html tag" do
+ layout "<!DOCTYPE html><% flush %><% yield %><html><head></head><body></body></html>"
+ run
+ received.should == chunks("<!DOCTYPE html>", "<html><head></head><body>(x)</body></html>", :end => true)
+ end
+
+ it "should inject errors correctly when the error occurs before the beginning of the head" do
+ layout "<!DOCTYPE html><html><% flush %><% yield %><head></head><body></body></html>"
+ run
+ received.should == chunks("<!DOCTYPE html><html>", "<head></head><body>(x)</body></html>", :end => true)
+ end
+
+ it "should inject errors correctly when the error occurs during the head" do
+ layout "<!DOCTYPE html><html><head><% flush %><% yield %></head><body></body></html>"
+ run
+ received.should == chunks("<!DOCTYPE html><html><head>", "</head><body>(x)</body></html>", :end => true)
+ end
+
+ it "should inject errors correctly when the error occurs between the head and body" do
+ layout "<!DOCTYPE html><html><head></head><% flush %><% yield %><body></body></html>"
+ run
+ received.should == chunks("<!DOCTYPE html><html><head></head>", "<body>(x)</body></html>", :end => true)
+ end
+
+ it "should inject errors correctly when the error occurs during the body" do
+ layout "<!DOCTYPE html><html><head></head><body><% flush %><% yield %></body></html>"
+ run
+ received.should == chunks("<!DOCTYPE html><html><head></head><body>", "(x)</body></html>", :end => true)
+ end
+
+ it "should inject errors correctly when the error occurs after the body" do
+ layout "<!DOCTYPE html><html><head></head><body></body><% flush %><% yield %></html>"
+ run
+ received.should == chunks("<!DOCTYPE html><html><head></head><body></body>", "</html>", "(x)", :end => true)
+ end
+
+ it "should inject errors correctly when the error occurs after the closing html tag" do
+ layout "<!DOCTYPE html><html><head></head><body></body></html><% flush %><% yield %>"
+ run
+ received.should == chunks("<!DOCTYPE html><html><head></head><body></body></html>", "(x)", :end => true)
+ end
+ end
+
+ describe "when an structurally-incomplete response is rendered" do
+ before do
+ action { render :progressive => true, :layout => nil }
+ end
+
+ it "should inject errors correctly when nothing is rendered" do
+ view "<% flush %><% raise 'x' %>"
+ run
+ received.should == chunks("<!DOCTYPE html><html><head><title>Unhandled Exception</title></head><body>(x)</body></html>", :end => true)
+ end
+
+ it "should inject errors correctly when just the doctype is rendered" do
+ view "<!DOCTYPE html><% flush %><% raise 'x' %>"
+ run
+ received.should == chunks("<!DOCTYPE html>", "<html><head><title>Unhandled Exception</title></head><body>(x)</body></html>", :end => true)
+ end
+
+ it "should inject errors correctly when just the doctype and opening html tag are rendered" do
+ view "<!DOCTYPE html><html><% flush %><% raise 'x' %>"
+ run
+ received.should == chunks("<!DOCTYPE html><html>", "<head><title>Unhandled Exception</title></head><body>(x)</body></html>", :end => true)
+ end
+
+ it "should inject errors correctly when only half the head is rendered" do
+ view "<!DOCTYPE html><html><head><% flush %><% raise 'x' %>"
+ run
+ received.should == chunks("<!DOCTYPE html><html><head>", "</head><body>(x)</body></html>", :end => true)
+ end
+
+ it "should inject errors correctly when only a head is rendered" do
+ view "<!DOCTYPE html><html><head></head><% flush %><% raise 'x' %>"
+ run
+ received.should == chunks("<!DOCTYPE html><html><head></head>", "<body>(x)</body></html>", :end => true)
+ end
+
+ it "should inject errors correctly when the closing body tag is missing" do
+ view "<!DOCTYPE html><html><head></head><body><% flush %><% raise 'x' %>"
+ run
+ received.should == chunks("<!DOCTYPE html><html><head></head><body>", "(x)</body></html>", :end => true)
+ end
+
+ it "should inject errors correctly when the closing html tag is missing" do
+ view "<!DOCTYPE html><html><head></head><body></body><% flush %><% raise 'x' %>"
+ run
+ received.should == chunks("<!DOCTYPE html><html><head></head><body></body>", "(x)</html>", :end => true)
+ end
+ end
+
+ describe "when the response consists of multiple templates" do
+ before do
+ action { render :progressive => true, :layout => 'layout' }
+ end
+
+ it "should inject errors when there is an error in the toplevel layout" do
+ layout "<!DOCTYPE html><html><head></head><body><% flush %><%= raise 'x' %></body></html>"
+ view ''
+ run
+ received.should == chunks("<!DOCTYPE html><html><head></head><body>", "(x)</body></html>", :end => true)
+ end
+
+ it "should inject errors when there is an error in the toplevel view" do
+ layout "<!DOCTYPE html><html><head></head><body><% flush %>[<%= yield %>]</body></html>"
+ view "<% raise 'x' %>"
+ run
+ received.should == chunks("<!DOCTYPE html><html><head></head><body>", "[](x)</body></html>", :end => true)
+ end
+
+ it "should inject errors when there is an error in a partial" do
+ layout "<!DOCTYPE html><html><head></head><body><% flush %>[<%= yield %>]</body></html>"
+ view "view{<%= render :partial => 'partial' %>}"
+ partial "<% raise 'x' %>"
+ run
+ received.should == chunks("<!DOCTYPE html><html><head></head><body>", "[view{}](x)</body></html>", :end => true)
+ end
+
+ it "should inject errors when there is an error in a subpartial" do
+ layout "<!DOCTYPE html><html><head></head><body><% flush %><%= yield %></body></html>"
+ view "view{<%= render :partial => 'partial' %>}"
+ partial "partial`<%= render :partial => 'subpartial' %>'"
+ template 'test/_subpartial', "<% raise 'x' %>"
+ run
+ received.should == chunks("<!DOCTYPE html><html><head></head><body>", "view{partial`'}(x)</body></html>", :end => true)
+ end
+
+ it "should inject errors from all partials which raised an unhandled exception" do
+ layout "<!DOCTYPE html><html><head></head><body><% flush %><%= yield %></body></html>"
+ view "view[<%= render :partial => 'x' %><%= render :partial => 'ok' %><%= render :partial => 'y' %>]"
+ template 'test/_x', "<% raise 'x' %>"
+ template 'test/_y', "<% raise 'y' %>"
+ template 'test/_ok', "ok"
+ run
+ received.should == chunks("<!DOCTYPE html><html><head></head><body>", "view[ok](x,y)</body></html>", :end => true)
+ end
+ end
+ end
+
+ describe "for nonlocal requests" do
+ before do
+ controller.class_eval do
+ def local_request?
+ false
+ end
+ end
+ end
+
+ it "should run the error callback for each error raised" do
+ messages = []
+ controller.on_progressive_rendering_error do |error|
+ messages << error.original_exception.message
+ end
+ view "<% render :partial => 'a' %><% render :partial => 'b' %>"
+ template 'test/_a', "<% raise 'a' %>"
+ template 'test/_b', "<% raise 'b' %>"
+ action { render :progressive => true, :layout => nil }
+ run
+ messages.should == ['a', 'b']
+ end
+
+ it "should not inject any error information" do
+ layout "<!DOCTYPE html><html><head></head><body><% flush %><%= yield %></body></html>"
+ view "...<% raise 'x' %>..."
+ action { render :progressive => true, :layout => 'layout' }
+ run
+ received.should == chunks("<!DOCTYPE html><html><head></head><body>", "</body></html>", :end => true)
+ end
+ end
+ end
+ end
end
Please sign in to comment.
Something went wrong with that request. Please try again.