Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

extract render detailed exception in own method #14225

Closed
wants to merge 4 commits into from

4 participants

@ecoologic

Refactor: Improves readability and modularity.

We're using Frisby.js. In our own project we monkey patched DebugExceptions#render_exception to return a JSON response which includes the stacktrace, it makes debug easier.

With this extraction you'll only have to override the logic that actually renders the HTML.

@dmathieu
Collaborator

I like that idea. :+1:
I'd push it a bit further though, as right now we'd need to rewrite the entire method.
How about having other methods for retrieving the template, and rendering in html and json?

A bit like what we already do with responders in to_html and to_format.

Also, as this would become public API, it will need a changelog entry and to be documented.

@ecoologic

@dmathieu thanks and sorry I didn't follow up, I'll keep an eye one this from now on.

I implemented a solution that handles the exception in a throw away class
https://github.com/net-engine/rails/compare/exception_handler?expand=1

There's currently 1 failing test, if you think there's any value in that branch I'll fix it, if not, please let me know how it's different from what you had in mind and I'll amend accordingly.

Thanks

@robin850 robin850 added the actionpack label
@robin850 robin850 added this to the 4.2.0 milestone
@rafaelfranca rafaelfranca modified the milestone: 4.2.0, 5.0.0
@ecoologic ecoologic closed this
@ecoologic ecoologic deleted the branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
131 actionpack/lib/action_dispatch/middleware/debug_exceptions.rb
@@ -24,75 +24,108 @@ def call(env)
response
rescue Exception => exception
raise exception if env['action_dispatch.show_exceptions'] == false
- render_exception(env, exception)
+ ExceptionHandler.new(env, exception, @routes_app).log_and_render
end
- private
+ class ExceptionHandler < Struct.new(:env, :exception, :routes_app)
+ delegate :status_code, to: :wrapper
- def render_exception(env, exception)
- wrapper = ExceptionWrapper.new(env, exception)
- log_error(env, wrapper)
+ def log_and_render
+ log_error
- if env['action_dispatch.show_detailed_exceptions']
- request = Request.new(env)
- template = ActionView::Base.new([RESCUES_TEMPLATE_PATH],
- request: request,
- exception: wrapper.exception,
+ if env['action_dispatch.show_detailed_exceptions']
+ render
+ else
+ raise exception
+ end
+ end
+
+ def to_html
+ ActionView::Base.new([RESCUES_TEMPLATE_PATH],
+ request: request,
+ exception: wrapper.exception,
application_trace: wrapper.application_trace,
- framework_trace: wrapper.framework_trace,
- full_trace: wrapper.full_trace,
- routes_inspector: routes_inspector(exception),
- source_extract: wrapper.source_extract,
- line_number: wrapper.line_number,
- file: wrapper.file
+ framework_trace: wrapper.framework_trace,
+ full_trace: wrapper.full_trace,
+ routes_inspector: routes_inspector,
+ source_extract: wrapper.source_extract,
+ line_number: wrapper.line_number,
+ file: wrapper.file
)
- file = "rescues/#{wrapper.rescue_template}"
+ end
+
+ def format
+ request.xhr? ? "text/plain" : "text/html"
+ end
+ def body
if request.xhr?
- body = template.render(template: file, layout: false, formats: [:text])
- format = "text/plain"
+ to_html.render(template: file, layout: false, formats: [:text])
else
- body = template.render(template: file, layout: 'rescues/layout')
- format = "text/html"
+ to_html.render(template: file, layout: 'rescues/layout')
end
- render(wrapper.status_code, body, format)
- else
- raise exception
end
- end
- def render(status, body, format)
- [status, {'Content-Type' => "#{format}; charset=#{Response.default_charset}", 'Content-Length' => body.bytesize.to_s}, [body]]
- end
+ private
+ def render
+ [
+ status_code,
+ {
+ 'Content-Type' => "#{format}; charset=#{Response.default_charset}",
+ 'Content-Length' => body.bytesize.to_s
+ },
+ [body]
+ ]
+ end
+
+ def file
+ "rescues/#{wrapper.rescue_template}"
+ end
+
+ def wrapper
+ ExceptionWrapper.new(env, exception)
+ end
- def log_error(env, wrapper)
- logger = logger(env)
- return unless logger
+ def request
+ Request.new(env)
+ end
- exception = wrapper.exception
+ def logger
+ env['action_dispatch.logger'] || stderr_logger
+ end
- trace = wrapper.application_trace
- trace = wrapper.framework_trace if trace.empty?
+ def stderr_logger
+ @stderr_logger ||= ActiveSupport::Logger.new($stderr)
+ end
- ActiveSupport::Deprecation.silence do
- message = "\n#{exception.class} (#{exception.message}):\n"
- message << exception.annoted_source_code.to_s if exception.respond_to?(:annoted_source_code)
- message << " " << trace.join("\n ")
- logger.fatal("#{message}\n\n")
+ def routes_inspector?
+ @routes_app.respond_to?(:routes) &&
+ (
+ exception.is_a?(ActionController::RoutingError) ||
+ exception.is_a?(ActionView::Template::Error)
+ )
end
- end
- def logger(env)
- env['action_dispatch.logger'] || stderr_logger
- end
+ def routes_inspector
+ if routes_inspector?
+ ActionDispatch::Routing::RoutesInspector.new(@routes_app.routes.routes)
+ end
+ end
- def stderr_logger
- @stderr_logger ||= ActiveSupport::Logger.new($stderr)
- end
+ def log_error
+ return unless logger
- def routes_inspector(exception)
- if @routes_app.respond_to?(:routes) && (exception.is_a?(ActionController::RoutingError) || exception.is_a?(ActionView::Template::Error))
- ActionDispatch::Routing::RoutesInspector.new(@routes_app.routes.routes)
+ exception = wrapper.exception
+
+ trace = wrapper.application_trace
+ trace = wrapper.framework_trace if trace.empty?
+
+ ActiveSupport::Deprecation.silence do
+ message = "\n#{exception.class} (#{exception.message}):\n"
+ message << exception.annoted_source_code.to_s if exception.respond_to?(:annoted_source_code)
+ message << " " << trace.join("\n ")
+ logger.fatal("#{message}\n\n")
+ end
end
end
end
View
12 actionview/test/abstract_unit.rb
@@ -322,11 +322,13 @@ def to_s
module ActionDispatch
class DebugExceptions
- private
- remove_method :stderr_logger
- # Silence logger
- def stderr_logger
- nil
+ class ExceptionHandler
+ private
+ remove_method :stderr_logger
+ # Silence logger
+ def stderr_logger
+ nil
+ end
end
end
end
Something went wrong with that request. Please try again.