diff options
author | Jeremy Kemper <jeremy@bitsweat.net> | 2009-05-04 08:35:35 -0700 |
---|---|---|
committer | Jeremy Kemper <jeremy@bitsweat.net> | 2009-05-04 08:35:35 -0700 |
commit | 8b6d2ef2f562c34ebc2ef547b0a18c31f5820569 (patch) | |
tree | b1397a8374b465ea2f300e776de126bb88386bcc | |
parent | d3ee8756c8b09b8524a9599926b02ededd27319c (diff) | |
parent | bcc4537f2a0d37fc02d67e9564fa5c9c5555b3d5 (diff) | |
download | rails-8b6d2ef2f562c34ebc2ef547b0a18c31f5820569.tar.gz rails-8b6d2ef2f562c34ebc2ef547b0a18c31f5820569.tar.bz2 rails-8b6d2ef2f562c34ebc2ef547b0a18c31f5820569.zip |
Merge branch 'master' of git@github.com:rails/rails
30 files changed, 422 insertions, 592 deletions
diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb index aee6b9dc9f..ab78bc9222 100644 --- a/actionpack/lib/action_controller.rb +++ b/actionpack/lib/action_controller.rb @@ -60,7 +60,7 @@ module ActionController autoload :Redirector, 'action_controller/base/redirect' autoload :Renderer, 'action_controller/base/render' autoload :RequestForgeryProtection, 'action_controller/base/request_forgery_protection' - autoload :Rescue, 'action_controller/dispatch/rescue' + autoload :Rescue, 'action_controller/base/rescue' autoload :Resources, 'action_controller/routing/resources' autoload :Responder, 'action_controller/base/responder' autoload :Routing, 'action_controller/routing' diff --git a/actionpack/lib/action_controller/base/base.rb b/actionpack/lib/action_controller/base/base.rb index 60567454b1..7bbde519cc 100644 --- a/actionpack/lib/action_controller/base/base.rb +++ b/actionpack/lib/action_controller/base/base.rb @@ -30,10 +30,6 @@ module ActionController #:nodoc: def allowed_methods_header allowed_methods.map { |method_symbol| method_symbol.to_s.upcase } * ', ' end - - def handle_response!(response) - response.headers['Allow'] ||= allowed_methods_header - end end class NotImplemented < MethodNotAllowed #:nodoc: @@ -510,9 +506,8 @@ module ActionController #:nodoc: public def call(env) - # HACK: For global rescue to have access to the original request and response - request = env["action_dispatch.rescue.request"] ||= ActionDispatch::Request.new(env) - response = env["action_dispatch.rescue.response"] ||= ActionDispatch::Response.new + request = ActionDispatch::Request.new(env) + response = ActionDispatch::Response.new process(request, response).to_a end diff --git a/actionpack/lib/action_controller/base/rescue.rb b/actionpack/lib/action_controller/base/rescue.rb new file mode 100644 index 0000000000..2717a06a37 --- /dev/null +++ b/actionpack/lib/action_controller/base/rescue.rb @@ -0,0 +1,50 @@ +module ActionController #:nodoc: + # Actions that fail to perform as expected throw exceptions. These + # exceptions can either be rescued for the public view (with a nice + # user-friendly explanation) or for the developers view (with tons of + # debugging information). The developers view is already implemented by + # the Action Controller, but the public view should be tailored to your + # specific application. + # + # The default behavior for public exceptions is to render a static html + # file with the name of the error code thrown. If no such file exists, an + # empty response is sent with the correct status code. + # + # You can override what constitutes a local request by overriding the + # <tt>local_request?</tt> method in your own controller. Custom rescue + # behavior is achieved by overriding the <tt>rescue_action_in_public</tt> + # and <tt>rescue_action_locally</tt> methods. + module Rescue + def self.included(base) #:nodoc: + base.send :include, ActiveSupport::Rescuable + base.extend(ClassMethods) + + base.class_eval do + alias_method_chain :perform_action, :rescue + end + end + + module ClassMethods + def rescue_action(env) + exception = env.delete('action_dispatch.rescue.exception') + request = ActionDispatch::Request.new(env) + response = ActionDispatch::Response.new + new.process(request, response, :rescue_action, exception).to_a + end + end + + protected + # Exception handler called when the performance of an action raises + # an exception. + def rescue_action(exception) + rescue_with_handler(exception) || raise(exception) + end + + private + def perform_action_with_rescue + perform_action_without_rescue + rescue Exception => exception + rescue_action(exception) + end + end +end diff --git a/actionpack/lib/action_controller/dispatch/dispatcher.rb b/actionpack/lib/action_controller/dispatch/dispatcher.rb index 25844bf2a2..cce3b6175d 100644 --- a/actionpack/lib/action_controller/dispatch/dispatcher.rb +++ b/actionpack/lib/action_controller/dispatch/dispatcher.rb @@ -5,9 +5,9 @@ module ActionController class << self def define_dispatcher_callbacks(cache_classes) unless cache_classes - unless self.middleware.include?(ActionDispatch::Reloader) - self.middleware.insert_after(ActionDispatch::Failsafe, ActionDispatch::Reloader) - end + # Development mode callbacks + before_dispatch :reload_application + after_dispatch :cleanup_application ActionView::Helpers::AssetTagHelper.cache_asset_timestamps = false end @@ -40,22 +40,11 @@ module ActionController def run_prepare_callbacks new.send :run_callbacks, :prepare_dispatch end - - def reload_application - # Run prepare callbacks before every request in development mode - run_prepare_callbacks - - Routing::Routes.reload - end - - def cleanup_application - # Cleanup the application before processing the current request. - ActiveRecord::Base.reset_subclasses if defined?(ActiveRecord) - ActiveSupport::Dependencies.clear - ActiveRecord::Base.clear_reloadable_connections! if defined?(ActiveRecord) - end end + cattr_accessor :router + self.router = Routing::Routes + cattr_accessor :middleware self.middleware = ActionDispatch::MiddlewareStack.new do |middleware| middlewares = File.join(File.dirname(__FILE__), "middlewares.rb") @@ -66,27 +55,29 @@ module ActionController define_callbacks :prepare_dispatch, :before_dispatch, :after_dispatch def initialize - @app = @@middleware.build(lambda { |env| self._call(env) }) + @app = @@middleware.build(@@router) freeze end def call(env) + run_callbacks :before_dispatch @app.call(env) + ensure + run_callbacks :after_dispatch, :enumerator => :reverse_each end - def _call(env) - begin - run_callbacks :before_dispatch - Routing::Routes.call(env) - rescue Exception => exception - if controller ||= (::ApplicationController rescue Base) - controller.call_with_exception(env, exception) - else - raise exception - end - ensure - run_callbacks :after_dispatch, :enumerator => :reverse_each - end + def reload_application + # Run prepare callbacks before every request in development mode + run_callbacks :prepare_dispatch + + @@router.reload + end + + def cleanup_application + # Cleanup the application before processing the current request. + ActiveRecord::Base.reset_subclasses if defined?(ActiveRecord) + ActiveSupport::Dependencies.clear + ActiveRecord::Base.clear_reloadable_connections! if defined?(ActiveRecord) end def flush_logger diff --git a/actionpack/lib/action_controller/dispatch/middlewares.rb b/actionpack/lib/action_controller/dispatch/middlewares.rb index b5adbae746..31a7b00d28 100644 --- a/actionpack/lib/action_controller/dispatch/middlewares.rb +++ b/actionpack/lib/action_controller/dispatch/middlewares.rb @@ -3,6 +3,11 @@ use "Rack::Lock", :if => lambda { } use "ActionDispatch::Failsafe" +use "ActionDispatch::ShowExceptions", lambda { ActionController::Base.consider_all_requests_local } +use "ActionDispatch::Rescue", lambda { + controller = (::ApplicationController rescue ActionController::Base) + controller.method(:rescue_action) +} use lambda { ActionController::Base.session_store }, lambda { ActionController::Base.session_options } diff --git a/actionpack/lib/action_controller/dispatch/rescue.rb b/actionpack/lib/action_controller/dispatch/rescue.rb deleted file mode 100644 index 2f3b40c231..0000000000 --- a/actionpack/lib/action_controller/dispatch/rescue.rb +++ /dev/null @@ -1,185 +0,0 @@ -module ActionController #:nodoc: - # Actions that fail to perform as expected throw exceptions. These - # exceptions can either be rescued for the public view (with a nice - # user-friendly explanation) or for the developers view (with tons of - # debugging information). The developers view is already implemented by - # the Action Controller, but the public view should be tailored to your - # specific application. - # - # The default behavior for public exceptions is to render a static html - # file with the name of the error code thrown. If no such file exists, an - # empty response is sent with the correct status code. - # - # You can override what constitutes a local request by overriding the - # <tt>local_request?</tt> method in your own controller. Custom rescue - # behavior is achieved by overriding the <tt>rescue_action_in_public</tt> - # and <tt>rescue_action_locally</tt> methods. - module Rescue - LOCALHOST = '127.0.0.1'.freeze - - DEFAULT_RESCUE_RESPONSE = :internal_server_error - DEFAULT_RESCUE_RESPONSES = { - 'ActionController::RoutingError' => :not_found, - 'ActionController::UnknownAction' => :not_found, - 'ActiveRecord::RecordNotFound' => :not_found, - 'ActiveRecord::StaleObjectError' => :conflict, - 'ActiveRecord::RecordInvalid' => :unprocessable_entity, - 'ActiveRecord::RecordNotSaved' => :unprocessable_entity, - 'ActionController::MethodNotAllowed' => :method_not_allowed, - 'ActionController::NotImplemented' => :not_implemented, - 'ActionController::InvalidAuthenticityToken' => :unprocessable_entity - } - - DEFAULT_RESCUE_TEMPLATE = 'diagnostics' - DEFAULT_RESCUE_TEMPLATES = { - 'ActionView::MissingTemplate' => 'missing_template', - 'ActionController::RoutingError' => 'routing_error', - 'ActionController::UnknownAction' => 'unknown_action', - 'ActionView::TemplateError' => 'template_error' - } - - RESCUES_TEMPLATE_PATH = ActionView::Template::FileSystemPath.new( - File.join(File.dirname(__FILE__), "templates")) - - def self.included(base) #:nodoc: - base.cattr_accessor :rescue_responses - base.rescue_responses = Hash.new(DEFAULT_RESCUE_RESPONSE) - base.rescue_responses.update DEFAULT_RESCUE_RESPONSES - - base.cattr_accessor :rescue_templates - base.rescue_templates = Hash.new(DEFAULT_RESCUE_TEMPLATE) - base.rescue_templates.update DEFAULT_RESCUE_TEMPLATES - - base.extend(ClassMethods) - base.send :include, ActiveSupport::Rescuable - - base.class_eval do - alias_method_chain :perform_action, :rescue - end - end - - module ClassMethods - def call_with_exception(env, exception) #:nodoc: - request = env["action_controller.rescue.request"] ||= ActionDispatch::Request.new(env) - response = env["action_controller.rescue.response"] ||= ActionDispatch::Response.new - new.process(request, response, :rescue_action, exception).to_a - end - end - - protected - # Exception handler called when the performance of an action raises - # an exception. - def rescue_action(exception) - rescue_with_handler(exception) || - rescue_action_without_handler(exception) - end - - # Overwrite to implement custom logging of errors. By default - # logs as fatal. - def log_error(exception) #:doc: - ActiveSupport::Deprecation.silence do - if ActionView::TemplateError === exception - logger.fatal(exception.to_s) - else - logger.fatal( - "\n#{exception.class} (#{exception.message}):\n " + - clean_backtrace(exception).join("\n ") + "\n\n" - ) - end - end - end - - # Overwrite to implement public exception handling (for requests - # answering false to <tt>local_request?</tt>). By default will call - # render_optional_error_file. Override this method to provide more - # user friendly error messages. - def rescue_action_in_public(exception) #:doc: - render_optional_error_file response_code_for_rescue(exception) - end - - # Attempts to render a static error page based on the - # <tt>status_code</tt> thrown, or just return headers if no such file - # exists. At first, it will try to render a localized static page. - # For example, if a 500 error is being handled Rails and locale is :da, - # it will first attempt to render the file at <tt>public/500.da.html</tt> - # then attempt to render <tt>public/500.html</tt>. If none of them exist, - # the body of the response will be left empty. - def render_optional_error_file(status_code) - status = interpret_status(status_code) - locale_path = "#{Rails.public_path}/#{status[0,3]}.#{I18n.locale}.html" if I18n.locale - path = "#{Rails.public_path}/#{status[0,3]}.html" - - if locale_path && File.exist?(locale_path) - render :file => locale_path, :status => status, :content_type => Mime::HTML - elsif File.exist?(path) - render :file => path, :status => status, :content_type => Mime::HTML - else - head status - end - end - - # True if the request came from localhost, 127.0.0.1. Override this - # method if you wish to redefine the meaning of a local request to - # include remote IP addresses or other criteria. - def local_request? #:doc: - request.remote_addr == LOCALHOST && request.remote_ip == LOCALHOST - end - - # Render detailed diagnostics for unhandled exceptions rescued from - # a controller action. - def rescue_action_locally(exception) - @template.instance_variable_set("@exception", exception) - @template.instance_variable_set("@rescues_path", RESCUES_TEMPLATE_PATH) - @template.instance_variable_set("@contents", - @template._render_template(template_path_for_local_rescue(exception))) - - response.content_type = Mime::HTML - response.status = interpret_status(response_code_for_rescue(exception)) - - content = @template._render_template(rescues_path("layout")) - render_for_text(content) - end - - def rescue_action_without_handler(exception) - log_error(exception) if logger - erase_results if performed? - - # Let the exception alter the response if it wants. - # For example, MethodNotAllowed sets the Allow header. - if exception.respond_to?(:handle_response!) - exception.handle_response!(response) - end - - if consider_all_requests_local || local_request? - rescue_action_locally(exception) - else - rescue_action_in_public(exception) - end - end - - private - def perform_action_with_rescue #:nodoc: - perform_action_without_rescue - rescue Exception => exception - rescue_action(exception) - end - - def rescues_path(template_name) - RESCUES_TEMPLATE_PATH.find_by_parts("rescues/#{template_name}.erb") - end - - def template_path_for_local_rescue(exception) - rescues_path(rescue_templates[exception.class.name]) - end - - def response_code_for_rescue(exception) - rescue_responses[exception.class.name] - end - - def clean_backtrace(exception) - defined?(Rails) && Rails.respond_to?(:backtrace_cleaner) ? - Rails.backtrace_cleaner.clean(exception.backtrace) : - exception.backtrace - end - end -end diff --git a/actionpack/lib/action_controller/dispatch/templates/rescues/diagnostics.erb b/actionpack/lib/action_controller/dispatch/templates/rescues/diagnostics.erb deleted file mode 100644 index e5c647c826..0000000000 --- a/actionpack/lib/action_controller/dispatch/templates/rescues/diagnostics.erb +++ /dev/null @@ -1,10 +0,0 @@ -<h1> - <%=h @exception.class.to_s %> - <% if request.parameters['controller'] %> - in <%=h request.parameters['controller'].humanize %>Controller<% if request.parameters['action'] %>#<%=h request.parameters['action'] %><% end %> - <% end %> -</h1> -<pre><%=h @exception.clean_message %></pre> - -<%= @template._render_template(@rescues_path.find_by_parts("rescues/_trace.erb")) %> -<%= @template._render_template(@rescues_path.find_by_parts("rescues/_request_and_response.erb")) %>
\ No newline at end of file diff --git a/actionpack/lib/action_dispatch.rb b/actionpack/lib/action_dispatch.rb index 5f6202d153..c882930e64 100644 --- a/actionpack/lib/action_dispatch.rb +++ b/actionpack/lib/action_dispatch.rb @@ -47,7 +47,8 @@ module ActionDispatch autoload :Failsafe, 'action_dispatch/middleware/failsafe' autoload :ParamsParser, 'action_dispatch/middleware/params_parser' - autoload :Reloader, 'action_dispatch/middleware/reloader' + autoload :Rescue, 'action_dispatch/middleware/rescue' + autoload :ShowExceptions, 'action_dispatch/middleware/show_exceptions' autoload :MiddlewareStack, 'action_dispatch/middleware/stack' autoload :Assertions, 'action_dispatch/testing/assertions' diff --git a/actionpack/lib/action_dispatch/middleware/reloader.rb b/actionpack/lib/action_dispatch/middleware/reloader.rb deleted file mode 100644 index 67313e30e4..0000000000 --- a/actionpack/lib/action_dispatch/middleware/reloader.rb +++ /dev/null @@ -1,14 +0,0 @@ -module ActionDispatch - class Reloader - def initialize(app) - @app = app - end - - def call(env) - ActionController::Dispatcher.reload_application - @app.call(env) - ensure - ActionController::Dispatcher.cleanup_application - end - end -end diff --git a/actionpack/lib/action_dispatch/middleware/rescue.rb b/actionpack/lib/action_dispatch/middleware/rescue.rb new file mode 100644 index 0000000000..1456825526 --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/rescue.rb @@ -0,0 +1,14 @@ +module ActionDispatch + class Rescue + def initialize(app, rescuer) + @app, @rescuer = app, rescuer + end + + def call(env) + @app.call(env) + rescue Exception => exception + env['action_dispatch.rescue.exception'] = exception + @rescuer.call(env) + end + end +end diff --git a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb new file mode 100644 index 0000000000..791254cdf2 --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb @@ -0,0 +1,142 @@ +module ActionDispatch + class ShowExceptions + include StatusCodes + + LOCALHOST = '127.0.0.1'.freeze + + DEFAULT_RESCUE_RESPONSE = :internal_server_error + DEFAULT_RESCUE_RESPONSES = { + 'ActionController::RoutingError' => :not_found, + 'ActionController::UnknownAction' => :not_found, + 'ActiveRecord::RecordNotFound' => :not_found, + 'ActiveRecord::StaleObjectError' => :conflict, + 'ActiveRecord::RecordInvalid' => :unprocessable_entity, + 'ActiveRecord::RecordNotSaved' => :unprocessable_entity, + 'ActionController::MethodNotAllowed' => :method_not_allowed, + 'ActionController::NotImplemented' => :not_implemented, + 'ActionController::InvalidAuthenticityToken' => :unprocessable_entity + } + + DEFAULT_RESCUE_TEMPLATE = 'diagnostics' + DEFAULT_RESCUE_TEMPLATES = { + 'ActionView::MissingTemplate' => 'missing_template', + 'ActionController::RoutingError' => 'routing_error', + 'ActionController::UnknownAction' => 'unknown_action', + 'ActionView::TemplateError' => 'template_error' + } + + RESCUES_TEMPLATE_PATH = File.join(File.dirname(__FILE__), 'templates') + + cattr_accessor :rescue_responses + @@rescue_responses = Hash.new(DEFAULT_RESCUE_RESPONSE) + @@rescue_responses.update DEFAULT_RESCUE_RESPONSES + + cattr_accessor :rescue_templates + @@rescue_templates = Hash.new(DEFAULT_RESCUE_TEMPLATE) + @@rescue_templates.update DEFAULT_RESCUE_TEMPLATES + + def initialize(app, consider_all_requests_local = false) + @app = app + @consider_all_requests_local = consider_all_requests_local + end + + def call(env) + @app.call(env) + rescue Exception => exception + log_error(exception) if logger + + request = Request.new(env) + if @consider_all_requests_local || local_request?(request) + rescue_action_locally(request, exception) + else + rescue_action_in_public(exception) + end + end + + private + # Render detailed diagnostics for unhandled exceptions rescued from + # a controller action. + def rescue_action_locally(request, exception) + template = ActionView::Base.new([RESCUES_TEMPLATE_PATH], + :template => template, + :request => request, + :exception => exception + ) + file = "rescues/#{@@rescue_templates[exception.class.name]}.erb" + body = template.render(:file => file, :layout => 'rescues/layout.erb') + + headers = {'Content-Type' => 'text/html', 'Content-Length' => body.length.to_s} + status = status_code(exception) + + [status, headers, body] + end + + # Attempts to render a static error page based on the + # <tt>status_code</tt> thrown, or just return headers if no such file + # exists. At first, it will try to render a localized static page. + # For example, if a 500 error is being handled Rails and locale is :da, + # it will first attempt to render the file at <tt>public/500.da.html</tt> + # then attempt to render <tt>public/500.html</tt>. If none of them exist, + # the body of the response will be left empty. + def rescue_action_in_public(exception) + status = status_code(exception) + locale_path = "#{public_path}/#{status}.#{I18n.locale}.html" if I18n.locale + path = "#{public_path}/#{status}.html" + + if locale_path && File.exist?(locale_path) + render_public_file(status, locale_path) + elsif File.exist?(path) + render_public_file(status, path) + else + [status, {'Content-Type' => 'text/html', 'Content-Length' => '0'}, []] + end + end + + # True if the request came from localhost, 127.0.0.1. + def local_request?(request) + request.remote_addr == LOCALHOST && request.remote_ip == LOCALHOST + end + + def render_public_file(status, path) + body = File.read(path) + [status, {'Content-Type' => 'text/html', 'Content-Length' => body.length.to_s}, body] + end + + def status_code(exception) + interpret_status(@@rescue_responses[exception.class.name]).to_i + end + + def public_path + if defined?(Rails) + Rails.public_path + else + "public" + end + end + + def log_error(exception) #:doc: + ActiveSupport::Deprecation.silence do + if ActionView::TemplateError === exception + logger.fatal(exception.to_s) + else + logger.fatal( + "\n#{exception.class} (#{exception.message}):\n " + + clean_backtrace(exception).join("\n ") + "\n\n" + ) + end + end + end + + def clean_backtrace(exception) + defined?(Rails) && Rails.respond_to?(:backtrace_cleaner) ? + Rails.backtrace_cleaner.clean(exception.backtrace) : + exception.backtrace + end + + def logger + if defined?(Rails.logger) + Rails.logger + end + end + end +end diff --git a/actionpack/lib/action_dispatch/middleware/stack.rb b/actionpack/lib/action_dispatch/middleware/stack.rb index ee5f28d5cb..ec8a9ca76e 100644 --- a/actionpack/lib/action_dispatch/middleware/stack.rb +++ b/actionpack/lib/action_dispatch/middleware/stack.rb @@ -61,7 +61,7 @@ module ActionDispatch def inspect str = klass.to_s - args.each { |arg| str += ", #{arg.inspect}" } + args.each { |arg| str += ", #{build_args.inspect}" } str end @@ -74,7 +74,6 @@ module ActionDispatch end private - def build_args Array(args).map { |arg| arg.respond_to?(:call) ? arg.call : arg } end diff --git a/actionpack/lib/action_controller/dispatch/templates/rescues/_request_and_response.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.erb index 64b34650b1..5224403dab 100644 --- a/actionpack/lib/action_controller/dispatch/templates/rescues/_request_and_response.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.erb @@ -6,7 +6,7 @@ <% end %> <% - clean_params = request.parameters.clone + clean_params = @request.parameters.clone clean_params.delete("action") clean_params.delete("controller") @@ -17,8 +17,8 @@ <p><b>Parameters</b>: <pre><%=h request_dump %></pre></p> <p><a href="#" onclick="document.getElementById('session_dump').style.display='block'; return false;">Show session dump</a></p> -<div id="session_dump" style="display:none"><%= debug(request.session.instance_variable_get("@data")) %></div> +<div id="session_dump" style="display:none"><%= debug(@request.session.instance_variable_get("@data")) %></div> <h2 style="margin-top: 30px">Response</h2> -<p><b>Headers</b>: <pre><%=h response ? response.headers.inspect.gsub(',', ",\n") : 'None' %></pre></p> +<p><b>Headers</b>: <pre><%=h @response ? @response.headers.inspect.gsub(',', ",\n") : 'None' %></pre></p> diff --git a/actionpack/lib/action_controller/dispatch/templates/rescues/_trace.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.erb index bb2d8375bd..bb2d8375bd 100644 --- a/actionpack/lib/action_controller/dispatch/templates/rescues/_trace.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.erb diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb new file mode 100644 index 0000000000..693e56270a --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb @@ -0,0 +1,10 @@ +<h1> + <%=h @exception.class.to_s %> + <% if @request.parameters['controller'] %> + in <%=h @request.parameters['controller'].humanize %>Controller<% if @request.parameters['action'] %>#<%=h @request.parameters['action'] %><% end %> + <% end %> +</h1> +<pre><%=h @exception.clean_message %></pre> + +<%= render :file => "rescues/_trace.erb" %> +<%= render :file => "rescues/_request_and_response.erb" %> diff --git a/actionpack/lib/action_controller/dispatch/templates/rescues/layout.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb index 4a04742e40..6c32fb17b8 100644 --- a/actionpack/lib/action_controller/dispatch/templates/rescues/layout.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb @@ -23,7 +23,7 @@ </head> <body> -<%= @contents %> +<%= yield %> </body> -</html>
\ No newline at end of file +</html> diff --git a/actionpack/lib/action_controller/dispatch/templates/rescues/missing_template.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.erb index dbfdf76947..dbfdf76947 100644 --- a/actionpack/lib/action_controller/dispatch/templates/rescues/missing_template.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.erb diff --git a/actionpack/lib/action_controller/dispatch/templates/rescues/routing_error.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.erb index ccfa858cce..ccfa858cce 100644 --- a/actionpack/lib/action_controller/dispatch/templates/rescues/routing_error.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.erb diff --git a/actionpack/lib/action_controller/dispatch/templates/rescues/template_error.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.erb index 2e34e03bd5..02fa18211d 100644 --- a/actionpack/lib/action_controller/dispatch/templates/rescues/template_error.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.erb @@ -1,6 +1,6 @@ <h1> <%=h @exception.original_exception.class.to_s %> in - <%=h request.parameters["controller"].capitalize if request.parameters["controller"]%>#<%=h request.parameters["action"] %> + <%=h @request.parameters["controller"].capitalize if @request.parameters["controller"]%>#<%=h @request.parameters["action"] %> </h1> <p> @@ -15,7 +15,7 @@ <% @real_exception = @exception @exception = @exception.original_exception || @exception %> -<%= render :file => @rescues_path["rescues/_trace.erb"] %> +<%= render :file => "rescues/_trace.erb" %> <% @exception = @real_exception %> -<%= render :file => @rescues_path["rescues/_request_and_response.erb"] %> +<%= render :file => "rescues/_request_and_response.erb" %> diff --git a/actionpack/lib/action_controller/dispatch/templates/rescues/unknown_action.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/unknown_action.erb index 683379da10..683379da10 100644 --- a/actionpack/lib/action_controller/dispatch/templates/rescues/unknown_action.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/unknown_action.erb diff --git a/actionpack/lib/action_dispatch/testing/assertions/response.rb b/actionpack/lib/action_dispatch/testing/assertions/response.rb index 92e3da580e..501a7c4dfb 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/response.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/response.rb @@ -31,13 +31,7 @@ module ActionDispatch elsif type.is_a?(Symbol) && @response.response_code == ActionDispatch::StatusCodes::SYMBOL_TO_STATUS_CODE[type] assert_block("") { true } # to count the assertion else - if @controller && @response.error? - exception = @controller.template.instance_variable_get(:@exception) - exception_message = exception && exception.message - assert_block(build_message(message, "Expected response to be a <?>, but was <?>\n<?>", type, @response.response_code, exception_message.to_s)) { false } - else - assert_block(build_message(message, "Expected response to be a <?>, but was <?>", type, @response.response_code)) { false } - end + assert_block(build_message(message, "Expected response to be a <?>, but was <?>", type, @response.response_code)) { false } end end diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb index 60957a2f0e..484d3c5ce7 100644 --- a/actionpack/test/controller/action_pack_assertions_test.rb +++ b/actionpack/test/controller/action_pack_assertions_test.rb @@ -489,7 +489,7 @@ class ActionPackAssertionsControllerTest < ActionController::TestCase get :index assert_response :success flunk 'Expected non-success response' - rescue ActiveSupport::TestCase::Assertion => e + rescue RuntimeError => e assert e.message.include?('FAIL') end diff --git a/actionpack/test/controller/base_test.rb b/actionpack/test/controller/base_test.rb index f4517d06c4..0c54d61426 100644 --- a/actionpack/test/controller/base_test.rb +++ b/actionpack/test/controller/base_test.rb @@ -27,6 +27,7 @@ class EmptyController < ActionController::Base end class NonEmptyController < ActionController::Base def public_action + render :nothing => true end hide_action :hidden_action @@ -51,6 +52,7 @@ end class DefaultUrlOptionsController < ActionController::Base def default_url_options_action + render :nothing => true end def default_url_options(options = nil) @@ -151,11 +153,8 @@ class PerformActionTest < ActionController::TestCase def test_get_on_hidden_should_fail use_controller NonEmptyController - get :hidden_action - assert_response 404 - - get :another_hidden_action - assert_response 404 + assert_raise(ActionController::UnknownAction) { get :hidden_action } + assert_raise(ActionController::UnknownAction) { get :another_hidden_action } end def test_namespaced_action_should_log_module_name diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index 6b9d1056a3..560c09509b 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -149,6 +149,9 @@ class PageCachingTest < ActionController::TestCase end class ActionCachingTestController < ActionController::Base + rescue_from(Exception) { head 500 } + rescue_from(ActiveRecord::RecordNotFound) { head :not_found } + caches_action :index, :redirected, :forbidden, :if => Proc.new { |c| !c.request.format.json? }, :expires_in => 1.hour caches_action :show, :cache_path => 'http://test.host/custom/show' caches_action :edit, :cache_path => Proc.new { |c| c.params[:id] ? "http://test.host/#{c.params[:id]};edit" : "http://test.host/edit" } diff --git a/actionpack/test/controller/deprecation/deprecated_base_methods_test.rb b/actionpack/test/controller/deprecation/deprecated_base_methods_test.rb index dd69a63020..0c02afea36 100644 --- a/actionpack/test/controller/deprecation/deprecated_base_methods_test.rb +++ b/actionpack/test/controller/deprecation/deprecated_base_methods_test.rb @@ -15,12 +15,6 @@ class DeprecatedBaseMethodsTest < ActionController::TestCase tests Target - def test_log_error_silences_deprecation_warnings - get :raises_name_error - rescue => e - assert_not_deprecated { @controller.send :log_error, e } - end - if defined? Test::Unit::Error def test_assertion_failed_error_silences_deprecation_warnings get :raises_name_error diff --git a/actionpack/test/controller/dispatcher_test.rb b/actionpack/test/controller/dispatcher_test.rb index c3bd113b86..b315232a7b 100644 --- a/actionpack/test/controller/dispatcher_test.rb +++ b/actionpack/test/controller/dispatcher_test.rb @@ -45,19 +45,6 @@ class DispatcherTest < Test::Unit::TestCase def log_failsafe_exception(status, exception); end end - def test_failsafe_response - Dispatcher.any_instance.expects(:_call).raises('b00m') - ActionDispatch::Failsafe.any_instance.expects(:log_failsafe_exception) - - assert_nothing_raised do - assert_equal [ - 500, - {"Content-Type" => "text/html"}, - ["<html><body><h1>500 Internal Server Error</h1></body></html>"] - ], dispatch - end - end - def test_prepare_callbacks a = b = c = nil Dispatcher.to_prepare { |*args| a = b = c = 1 } diff --git a/actionpack/test/controller/filters_test.rb b/actionpack/test/controller/filters_test.rb index 5876f55420..5fd3eb0446 100644 --- a/actionpack/test/controller/filters_test.rb +++ b/actionpack/test/controller/filters_test.rb @@ -169,6 +169,7 @@ class FilterTest < Test::Unit::TestCase end def public + render :text => 'ok' end end @@ -177,6 +178,10 @@ class FilterTest < Test::Unit::TestCase before_filter :find_record before_filter :ensure_login + def index + render :text => 'ok' + end + private def find_record @ran_filter ||= [] diff --git a/actionpack/test/controller/layout_test.rb b/actionpack/test/controller/layout_test.rb index da3f7b0cb8..5b7d40d16d 100644 --- a/actionpack/test/controller/layout_test.rb +++ b/actionpack/test/controller/layout_test.rb @@ -205,8 +205,7 @@ end class LayoutExceptionRaised < ActionController::TestCase def test_exception_raised_when_layout_file_not_found @controller = SetsNonExistentLayoutFile.new - get :hello - assert_kind_of ActionView::MissingTemplate, @controller.template.instance_eval { @exception } + assert_raise(ActionView::MissingTemplate) { get :hello } end end diff --git a/actionpack/test/controller/rescue_test.rb b/actionpack/test/controller/rescue_test.rb index 894420a910..f745926b20 100644 --- a/actionpack/test/controller/rescue_test.rb +++ b/actionpack/test/controller/rescue_test.rb @@ -138,213 +138,88 @@ class RescueController < ActionController::Base end end -class RescueControllerTest < ActionController::TestCase - FIXTURE_PUBLIC = "#{File.dirname(__FILE__)}/../fixtures".freeze - - def setup - super - set_all_requests_local - populate_exception_object - end - - def set_all_requests_local - RescueController.consider_all_requests_local = true - @request.remote_addr = '1.2.3.4' - @request.host = 'example.com' - end - - def populate_exception_object - begin - raise 'foo' - rescue => @exception - end - end - - def test_rescue_exceptions_raised_by_filters - with_rails_root FIXTURE_PUBLIC do - with_all_requests_local false do - get :before_filter_raises - end - end +class ExceptionInheritanceRescueController < ActionController::Base - assert_response :internal_server_error + class ParentException < StandardError end - def test_rescue_action_locally_if_all_requests_local - @controller.expects(:local_request?).never - @controller.expects(:rescue_action_locally).with(@exception) - @controller.expects(:rescue_action_in_public).never - - with_all_requests_local do - @controller.send :rescue_action, @exception - end + class ChildException < ParentException end - def test_rescue_action_locally_if_remote_addr_is_localhost - @controller.expects(:local_request?).returns(true) - @controller.expects(:rescue_action_locally).with(@exception) - @controller.expects(:rescue_action_in_public).never - - with_all_requests_local false do - @controller.send :rescue_action, @exception - end + class GrandchildException < ChildException end - def test_rescue_action_in_public_otherwise - @controller.expects(:local_request?).returns(false) - @controller.expects(:rescue_action_locally).never - @controller.expects(:rescue_action_in_public).with(@exception) + rescue_from ChildException, :with => lambda { head :ok } + rescue_from ParentException, :with => lambda { head :created } + rescue_from GrandchildException, :with => lambda { head :no_content } - with_all_requests_local false do - @controller.send :rescue_action, @exception - end + def raise_parent_exception + raise ParentException end - def test_rescue_action_in_public_with_localized_error_file - # Change locale - old_locale = I18n.locale - I18n.locale = :da - - with_rails_root FIXTURE_PUBLIC do - with_all_requests_local false do - get :raises - end - end - - assert_response :internal_server_error - body = File.read("#{FIXTURE_PUBLIC}/public/500.da.html") - assert_equal body, @response.body - ensure - I18n.locale = old_locale + def raise_child_exception + raise ChildException end - def test_rescue_action_in_public_with_error_file - with_rails_root FIXTURE_PUBLIC do - with_all_requests_local false do - get :raises - end - end - - assert_response :internal_server_error - body = File.read("#{FIXTURE_PUBLIC}/public/500.html") - assert_equal body, @response.body + def raise_grandchild_exception + raise GrandchildException end +end - def test_rescue_action_in_public_without_error_file - with_rails_root '/tmp' do - with_all_requests_local false do - get :raises - end - end - - assert_response :internal_server_error - assert_equal ' ', @response.body +class ExceptionInheritanceRescueControllerTest < ActionController::TestCase + def test_bottom_first + get :raise_grandchild_exception + assert_response :no_content end - def test_rescue_unknown_action_in_public_with_error_file - with_rails_root FIXTURE_PUBLIC do - with_all_requests_local false do - get :foobar_doesnt_exist - end - end - - assert_response :not_found - body = File.read("#{FIXTURE_PUBLIC}/public/404.html") - assert_equal body, @response.body + def test_inheritance_works + get :raise_child_exception + assert_response :created end +end - def test_rescue_unknown_action_in_public_without_error_file - with_rails_root '/tmp' do - with_all_requests_local false do - get :foobar_doesnt_exist - end - end - - assert_response :not_found - assert_equal ' ', @response.body +class ControllerInheritanceRescueController < ExceptionInheritanceRescueController + class FirstExceptionInChildController < StandardError end - def test_rescue_missing_template_in_public - with_rails_root FIXTURE_PUBLIC do - with_all_requests_local true do - get :missing_template - end - end - - assert_response :internal_server_error - assert @response.body.include?('missing_template'), "Response should include the template name." + class SecondExceptionInChildController < StandardError end - def test_rescue_action_locally - get :raises - assert_response :internal_server_error - assert_template 'diagnostics.erb' - assert @response.body.include?('RescueController#raises'), "Response should include controller and action." - assert @response.body.include?("don't panic"), "Response should include exception message." - end + rescue_from FirstExceptionInChildController, 'SecondExceptionInChildController', :with => lambda { head :gone } - def test_local_request_when_remote_addr_is_localhost - @controller.expects(:request).returns(@request).at_least_once - with_remote_addr '127.0.0.1' do - assert @controller.send(:local_request?) - end + def raise_first_exception_in_child_controller + raise FirstExceptionInChildController end - def test_local_request_when_remote_addr_isnt_locahost - @controller.expects(:request).returns(@request) - with_remote_addr '1.2.3.4' do - assert !@controller.send(:local_request?) - end + def raise_second_exception_in_child_controller + raise SecondExceptionInChildController end +end - def test_rescue_responses - responses = ActionController::Base.rescue_responses - - assert_equal ActionController::Rescue::DEFAULT_RESCUE_RESPONSE, responses.default - assert_equal ActionController::Rescue::DEFAULT_RESCUE_RESPONSE, responses[Exception.new] - - assert_equal :not_found, responses[ActionController::RoutingError.name] - assert_equal :not_found, responses[ActionController::UnknownAction.name] - assert_equal :not_found, responses['ActiveRecord::RecordNotFound'] - assert_equal :conflict, responses['ActiveRecord::StaleObjectError'] - assert_equal :unprocessable_entity, responses['ActiveRecord::RecordInvalid'] - assert_equal :unprocessable_entity, responses['ActiveRecord::RecordNotSaved'] - assert_equal :method_not_allowed, responses['ActionController::MethodNotAllowed'] - assert_equal :not_implemented, responses['ActionController::NotImplemented'] +class ControllerInheritanceRescueControllerTest < ActionController::TestCase + def test_first_exception_in_child_controller + get :raise_first_exception_in_child_controller + assert_response :gone end - def test_rescue_templates - templates = ActionController::Base.rescue_templates - - assert_equal ActionController::Rescue::DEFAULT_RESCUE_TEMPLATE, templates.default - assert_equal ActionController::Rescue::DEFAULT_RESCUE_TEMPLATE, templates[Exception.new] - - assert_equal 'missing_template', templates[ActionView::MissingTemplate.name] - assert_equal 'routing_error', templates[ActionController::RoutingError.name] - assert_equal 'unknown_action', templates[ActionController::UnknownAction.name] - assert_equal 'template_error', templates[ActionView::TemplateError.name] + def test_second_exception_in_child_controller + get :raise_second_exception_in_child_controller + assert_response :gone end - def test_not_implemented - with_all_requests_local false do - with_rails_public_path(".") do - head :not_implemented - end - end - assert_response :not_implemented - assert_equal "GET, PUT", @response.headers['Allow'] + def test_exception_in_parent_controller + get :raise_parent_exception + assert_response :created end +end - def test_method_not_allowed - with_all_requests_local false do - with_rails_public_path(".") do - get :method_not_allowed - end - end - assert_response :method_not_allowed - assert_equal "GET, HEAD, PUT", @response.headers['Allow'] +class ApplicationController < ActionController::Base + rescue_from ActionController::RoutingError do + render :text => 'no way' end +end +class RescueControllerTest < ActionController::TestCase def test_rescue_handler get :not_authorized assert_response :forbidden @@ -399,133 +274,6 @@ class RescueControllerTest < ActionController::TestCase get :resource_unavailable_raise_as_string assert_equal "RescueController::ResourceUnavailableToRescueAsString", @response.body end - - protected - def with_all_requests_local(local = true) - old_local, ActionController::Base.consider_all_requests_local = - ActionController::Base.consider_all_requests_local, local - yield - ensure - ActionController::Base.consider_all_requests_local = old_local - end - - def with_remote_addr(addr) - old_remote_addr, @request.remote_addr = @request.remote_addr, addr - yield - ensure - @request.remote_addr = old_remote_addr - end - - def with_rails_public_path(rails_root) - old_rails = Object.const_get(:Rails) rescue nil - mod = Object.const_set(:Rails, Module.new) - (class << mod; self; end).instance_eval do - define_method(:public_path) { "#{rails_root}/public" } - end - yield - ensure - Object.module_eval { remove_const(:Rails) } if defined?(Rails) - Object.const_set(:Rails, old_rails) if old_rails - end - - def with_rails_root(path = nil,&block) - old_rails_root = RAILS_ROOT if defined?(RAILS_ROOT) - if path - silence_warnings { Object.const_set(:RAILS_ROOT, path) } - else - Object.remove_const(:RAILS_ROOT) rescue nil - end - - with_rails_public_path(path, &block) - - ensure - if old_rails_root - silence_warnings { Object.const_set(:RAILS_ROOT, old_rails_root) } - else - Object.remove_const(:RAILS_ROOT) rescue nil - end - end -end - -class ExceptionInheritanceRescueController < ActionController::Base - - class ParentException < StandardError - end - - class ChildException < ParentException - end - - class GrandchildException < ChildException - end - - rescue_from ChildException, :with => lambda { head :ok } - rescue_from ParentException, :with => lambda { head :created } - rescue_from GrandchildException, :with => lambda { head :no_content } - - def raise_parent_exception - raise ParentException - end - - def raise_child_exception - raise ChildException - end - - def raise_grandchild_exception - raise GrandchildException - end -end - -class ExceptionInheritanceRescueControllerTest < ActionController::TestCase - def test_bottom_first - get :raise_grandchild_exception - assert_response :no_content - end - - def test_inheritance_works - get :raise_child_exception - assert_response :created - end -end - -class ControllerInheritanceRescueController < ExceptionInheritanceRescueController - class FirstExceptionInChildController < StandardError - end - - class SecondExceptionInChildController < StandardError - end - - rescue_from FirstExceptionInChildController, 'SecondExceptionInChildController', :with => lambda { head :gone } - - def raise_first_exception_in_child_controller - raise FirstExceptionInChildController - end - - def raise_second_exception_in_child_controller - raise SecondExceptionInChildController - end -end - -class ControllerInheritanceRescueControllerTest < ActionController::TestCase - def test_first_exception_in_child_controller - get :raise_first_exception_in_child_controller - assert_response :gone - end - - def test_second_exception_in_child_controller - get :raise_second_exception_in_child_controller - assert_response :gone - end - - def test_exception_in_parent_controller - get :raise_parent_exception - assert_response :created - end -end - -class ApplicationController < ActionController::Base - rescue_from ActionController::RoutingError do - render :text => 'no way' - end end class RescueTest < ActionController::IntegrationTest diff --git a/actionpack/test/dispatch/show_exceptions_test.rb b/actionpack/test/dispatch/show_exceptions_test.rb new file mode 100644 index 0000000000..f8f562e7c1 --- /dev/null +++ b/actionpack/test/dispatch/show_exceptions_test.rb @@ -0,0 +1,103 @@ +require 'abstract_unit' + +module ActionDispatch + class ShowExceptions + private + def public_path + "#{FIXTURE_LOAD_PATH}/public" + end + end +end + +class ShowExceptionsTest < ActionController::IntegrationTest + Boomer = lambda do |env| + req = ActionDispatch::Request.new(env) + case req.path + when "/not_found" + raise ActionController::UnknownAction + when "/method_not_allowed" + raise ActionController::MethodNotAllowed + when "/not_implemented" + raise ActionController::NotImplemented + when "/unprocessable_entity" + raise ActionController::InvalidAuthenticityToken + else + raise "puke!" + end + end + + ProductionApp = ActionDispatch::ShowExceptions.new(Boomer, false) + DevelopmentApp = ActionDispatch::ShowExceptions.new(Boomer, true) + + test "rescue in public from a remote ip" do + @integration_session = open_session(ProductionApp) + self.remote_addr = '208.77.188.166' + + get "/" + assert_response 500 + assert_equal "500 error fixture\n", body + + get "/not_found" + assert_response 404 + assert_equal "404 error fixture\n", body + + get "/method_not_allowed" + assert_response 405 + assert_equal "", body + end + + test "rescue locally from a local request" do + @integration_session = open_session(ProductionApp) + self.remote_addr = '127.0.0.1' + + get "/" + assert_response 500 + assert_match /puke/, body + + get "/not_found" + assert_response 404 + assert_match /ActionController::UnknownAction/, body + + get "/method_not_allowed" + assert_response 405 + assert_match /ActionController::MethodNotAllowed/, body + end + + test "localize public rescue message" do + # Change locale + old_locale = I18n.locale + I18n.locale = :da + + begin + @integration_session = open_session(ProductionApp) + self.remote_addr = '208.77.188.166' + + get "/" + assert_response 500 + assert_equal "500 localized error fixture\n", body + + get "/not_found" + assert_response 404 + assert_equal "404 error fixture\n", body + ensure + I18n.locale = old_locale + end + end + + test "always rescue locally in development mode" do + @integration_session = open_session(DevelopmentApp) + self.remote_addr = '208.77.188.166' + + get "/" + assert_response 500 + assert_match /puke/, body + + get "/not_found" + assert_response 404 + assert_match /ActionController::UnknownAction/, body + + get "/method_not_allowed" + assert_response 405 + assert_match /ActionController::MethodNotAllowed/, body + end +end |