From a725a453b38057b46878afae39dc107ada2cf326 Mon Sep 17 00:00:00 2001 From: Kir Shatrov Date: Wed, 21 Aug 2013 16:42:04 +0400 Subject: Display exceptions in text format for xhr request --- actionpack/CHANGELOG.md | 4 ++ .../action_dispatch/middleware/debug_exceptions.rb | 34 ++++++++++------- .../templates/rescues/_request_and_response.erb | 34 ----------------- .../rescues/_request_and_response.html.erb | 34 +++++++++++++++++ .../rescues/_request_and_response.text.erb | 23 ++++++++++++ .../middleware/templates/rescues/_trace.erb | 24 ------------ .../middleware/templates/rescues/_trace.html.erb | 24 ++++++++++++ .../middleware/templates/rescues/_trace.text.erb | 15 ++++++++ .../templates/rescues/missing_template.erb | 7 ---- .../templates/rescues/missing_template.html.erb | 7 ++++ .../templates/rescues/missing_template.text.erb | 3 ++ .../middleware/templates/rescues/routing_error.erb | 30 --------------- .../templates/rescues/routing_error.html.erb | 30 +++++++++++++++ .../templates/rescues/routing_error.text.erb | 11 ++++++ .../templates/rescues/template_error.erb | 43 ---------------------- .../templates/rescues/template_error.html.erb | 43 ++++++++++++++++++++++ .../templates/rescues/template_error.text.erb | 8 ++++ .../templates/rescues/unknown_action.erb | 6 --- .../templates/rescues/unknown_action.html.erb | 6 +++ .../templates/rescues/unknown_action.text.erb | 3 ++ actionpack/test/dispatch/debug_exceptions_test.rb | 41 +++++++++++++++++++++ 21 files changed, 273 insertions(+), 157 deletions(-) delete mode 100644 actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.erb create mode 100644 actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.html.erb create mode 100644 actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.text.erb delete mode 100644 actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.erb create mode 100644 actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.html.erb create mode 100644 actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.text.erb delete mode 100644 actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.erb create mode 100644 actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.html.erb create mode 100644 actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.text.erb delete mode 100644 actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.erb create mode 100644 actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.html.erb create mode 100644 actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.text.erb delete mode 100644 actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.erb create mode 100644 actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.html.erb create mode 100644 actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.text.erb delete mode 100644 actionpack/lib/action_dispatch/middleware/templates/rescues/unknown_action.erb create mode 100644 actionpack/lib/action_dispatch/middleware/templates/rescues/unknown_action.html.erb create mode 100644 actionpack/lib/action_dispatch/middleware/templates/rescues/unknown_action.text.erb diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 4e090ed526..6e8ba88b77 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,7 @@ +* Development mode exceptions are rendered in text format in case of XHR request. + + *Kir Shatrov* + * Fix an issue where :if and :unless controller action procs were being run before checking for the correct action in the :only and :unless options. diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index 64230ff1ae..0ca1a87645 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -34,27 +34,35 @@ module ActionDispatch log_error(env, wrapper) if env['action_dispatch.show_detailed_exceptions'] + request = Request.new(env) template = ActionView::Base.new([RESCUES_TEMPLATE_PATH], - :request => Request.new(env), - :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 + 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 ) file = "rescues/#{wrapper.rescue_template}" - body = template.render(:template => file, :layout => 'rescues/layout') - render(wrapper.status_code, body) + + if request.xhr? + body = template.render(template: file, layout: false, formats: [:text]) + format = "text/plain" + else + body = template.render(template: file, layout: 'rescues/layout') + format = "text/html" + end + render(wrapper.status_code, body, format) else raise exception end end - def render(status, body) - [status, {'Content-Type' => "text/html; charset=#{Response.default_charset}", 'Content-Length' => body.bytesize.to_s}, [body]] + def render(status, body, format) + [status, {'Content-Type' => "#{format}; charset=#{Response.default_charset}", 'Content-Length' => body.bytesize.to_s}, [body]] end def log_error(env, wrapper) diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.erb deleted file mode 100644 index db219c8fa9..0000000000 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.erb +++ /dev/null @@ -1,34 +0,0 @@ -<% unless @exception.blamed_files.blank? %> - <% if (hide = @exception.blamed_files.length > 8) %> - Toggle blamed files - <% end %> - 
><%= @exception.describe_blame %>
-<% end %> - -<% - clean_params = @request.filtered_parameters.clone - clean_params.delete("action") - clean_params.delete("controller") - - request_dump = clean_params.empty? ? 'None' : clean_params.inspect.gsub(',', ",\n") - - def debug_hash(object) - object.to_hash.sort_by { |k, _| k.to_s }.map { |k, v| "#{k}: #{v.inspect rescue $!.message}" }.join("\n") - end unless self.class.method_defined?(:debug_hash) -%> - -

Request

-

Parameters:

<%= request_dump %>
- -
-
Toggle session dump
- -
- -
-
Toggle env dump
- -
- -

Response

-

Headers:

<%= defined?(@response) ? @response.headers.inspect.gsub(',', ",\n") : 'None' %>
diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.html.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.html.erb new file mode 100644 index 0000000000..db219c8fa9 --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.html.erb @@ -0,0 +1,34 @@ +<% unless @exception.blamed_files.blank? %> + <% if (hide = @exception.blamed_files.length > 8) %> + Toggle blamed files + <% end %> + 
><%= @exception.describe_blame %>
+<% end %> + +<% + clean_params = @request.filtered_parameters.clone + clean_params.delete("action") + clean_params.delete("controller") + + request_dump = clean_params.empty? ? 'None' : clean_params.inspect.gsub(',', ",\n") + + def debug_hash(object) + object.to_hash.sort_by { |k, _| k.to_s }.map { |k, v| "#{k}: #{v.inspect rescue $!.message}" }.join("\n") + end unless self.class.method_defined?(:debug_hash) +%> + +

Request

+

Parameters:

<%= request_dump %>
+ +
+
Toggle session dump
+ +
+ +
+
Toggle env dump
+ +
+ +

Response

+

Headers:

<%= defined?(@response) ? @response.headers.inspect.gsub(',', ",\n") : 'None' %>
diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.text.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.text.erb new file mode 100644 index 0000000000..396768ecee --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_request_and_response.text.erb @@ -0,0 +1,23 @@ +<% + clean_params = @request.filtered_parameters.clone + clean_params.delete("action") + clean_params.delete("controller") + + request_dump = clean_params.empty? ? 'None' : clean_params.inspect.gsub(',', ",\n") + + def debug_hash(object) + object.to_hash.sort_by { |k, _| k.to_s }.map { |k, v| "#{k}: #{v.inspect rescue $!.message}" }.join("\n") + end unless self.class.method_defined?(:debug_hash) +%> + +Request parameters +<%= request_dump %> + +Session dump +<%= debug_hash @request.session %> + +Env dump +<%= debug_hash @request.env.slice(*@request.class::ENV_METHODS) %> + +Response headers +<%= defined?(@response) ? @response.headers.inspect.gsub(',', ",\n") : 'None' %> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.erb deleted file mode 100644 index b181909bff..0000000000 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.erb +++ /dev/null @@ -1,24 +0,0 @@ -<% - traces = { "Application Trace" => @application_trace, - "Framework Trace" => @framework_trace, - "Full Trace" => @full_trace } - names = traces.keys -%> - -

Rails.root: <%= defined?(Rails) && Rails.respond_to?(:root) ? Rails.root : "unset" %>

- -
- <% names.each do |name| %> - <% - show = "show('#{name.gsub(/\s/, '-')}');" - hide = (names - [name]).collect {|hide_name| "hide('#{hide_name.gsub(/\s/, '-')}');"} - %> - <%= name %> <%= '|' unless names.last == name %> - <% end %> - - <% traces.each do |name, trace| %> -
;"> - 
<%= trace.join "\n" %>
-
- <% end %> -
diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.html.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.html.erb new file mode 100644 index 0000000000..b181909bff --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.html.erb @@ -0,0 +1,24 @@ +<% + traces = { "Application Trace" => @application_trace, + "Framework Trace" => @framework_trace, + "Full Trace" => @full_trace } + names = traces.keys +%> + +

Rails.root: <%= defined?(Rails) && Rails.respond_to?(:root) ? Rails.root : "unset" %>

+ +
+ <% names.each do |name| %> + <% + show = "show('#{name.gsub(/\s/, '-')}');" + hide = (names - [name]).collect {|hide_name| "hide('#{hide_name.gsub(/\s/, '-')}');"} + %> + <%= name %> <%= '|' unless names.last == name %> + <% end %> + + <% traces.each do |name, trace| %> +
;"> + 
<%= trace.join "\n" %>
+
+ <% end %> +
diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.text.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.text.erb new file mode 100644 index 0000000000..d4af5c9b06 --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_trace.text.erb @@ -0,0 +1,15 @@ +<% + traces = { "Application Trace" => @application_trace, + "Framework Trace" => @framework_trace, + "Full Trace" => @full_trace } +%> + +Rails.root: <%= defined?(Rails) && Rails.respond_to?(:root) ? Rails.root : "unset" %> + +<% traces.each do |name, trace| %> +<% if trace.any? %> +<%= name %> +<%= trace.join("\n") %> + +<% end %> +<% end %> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.erb deleted file mode 100644 index 5c016e544e..0000000000 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.erb +++ /dev/null @@ -1,7 +0,0 @@ -
-

Template is missing

-
- -
-

<%= h @exception.message %>

-
diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.html.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.html.erb new file mode 100644 index 0000000000..5c016e544e --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.html.erb @@ -0,0 +1,7 @@ +
+

Template is missing

+
+ +
+

<%= h @exception.message %>

+
diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.text.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.text.erb new file mode 100644 index 0000000000..ae62d9eb02 --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/missing_template.text.erb @@ -0,0 +1,3 @@ +Template is missing + +<%= @exception.message %> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.erb deleted file mode 100644 index 7e9cedb95e..0000000000 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.erb +++ /dev/null @@ -1,30 +0,0 @@ -
-

Routing Error

-
-
-

<%= h @exception.message %>

- <% unless @exception.failures.empty? %> -

-

Failure reasons:

-
    - <% @exception.failures.each do |route, reason| %> -
  1. <%= route.inspect.delete('\\') %> failed because <%= reason.downcase %>
    2. - <% end %> -
-

- <% end %> - - <%= render template: "rescues/_trace" %> - - <% if @routes_inspector %> -

- Routes -

- -

- Routes match in priority from top to bottom -

- - <%= @routes_inspector.format(ActionDispatch::Routing::HtmlTableFormatter.new(self)) %> - <% end %> -
diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.html.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.html.erb new file mode 100644 index 0000000000..7e9cedb95e --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.html.erb @@ -0,0 +1,30 @@ +
+

Routing Error

+
+
+

<%= h @exception.message %>

+ <% unless @exception.failures.empty? %> +

+

Failure reasons:

+
    + <% @exception.failures.each do |route, reason| %> +
  1. <%= route.inspect.delete('\\') %> failed because <%= reason.downcase %>
    2. + <% end %> +
+

+ <% end %> + + <%= render template: "rescues/_trace" %> + + <% if @routes_inspector %> +

+ Routes +

+ +

+ Routes match in priority from top to bottom +

+ + <%= @routes_inspector.format(ActionDispatch::Routing::HtmlTableFormatter.new(self)) %> + <% end %> +
diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.text.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.text.erb new file mode 100644 index 0000000000..f6e4dac1f3 --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/routing_error.text.erb @@ -0,0 +1,11 @@ +Routing Error + +<%= @exception.message %> +<% unless @exception.failures.empty? %> +Failure reasons: +<% @exception.failures.each do |route, reason| %> + - <%= route.inspect.delete('\\') %> failed because <%= reason.downcase %> +<% end %> +<% end %> + +<%= render template: "rescues/_trace", format: :text %> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.erb deleted file mode 100644 index 027a0f5b3e..0000000000 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.erb +++ /dev/null @@ -1,43 +0,0 @@ -<% @source_extract = @exception.source_extract(0, :html) %> -
-

- <%= @exception.original_exception.class.to_s %> in - <%= @request.parameters["controller"].camelize if @request.parameters["controller"] %>#<%= @request.parameters["action"] %> -

-
- -
-

- Showing <%= @exception.file_name %> where line #<%= @exception.line_number %> raised: -

- 
<%= h @exception.message %>
- -
-
-

Extracted source (around line #<%= @exception.line_number %>):

-
-
- - - - - -
- 
-            <% @source_extract.keys.each do |line_number| %>
-<%= line_number -%>
-            <% end %>
-
- 		-
-<% @source_extract.each do |line, source| -%>
"><%= source -%>
<% end -%>
-
-
-
-
- -

<%= @exception.sub_template_message %>

- - <%= render template: "rescues/_trace" %> - <%= render template: "rescues/_request_and_response" %> -
diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.html.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.html.erb new file mode 100644 index 0000000000..027a0f5b3e --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.html.erb @@ -0,0 +1,43 @@ +<% @source_extract = @exception.source_extract(0, :html) %> +
+

+ <%= @exception.original_exception.class.to_s %> in + <%= @request.parameters["controller"].camelize if @request.parameters["controller"] %>#<%= @request.parameters["action"] %> +

+
+ +
+

+ Showing <%= @exception.file_name %> where line #<%= @exception.line_number %> raised: +

+ 
<%= h @exception.message %>
+ +
+
+

Extracted source (around line #<%= @exception.line_number %>):

+
+
+ + + + + +
+ 
+            <% @source_extract.keys.each do |line_number| %>
+<%= line_number -%>
+            <% end %>
+
+ 		+
+<% @source_extract.each do |line, source| -%>
"><%= source -%>
<% end -%>
+
+
+
+
+ +

<%= @exception.sub_template_message %>

+ + <%= render template: "rescues/_trace" %> + <%= render template: "rescues/_request_and_response" %> +
diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.text.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.text.erb new file mode 100644 index 0000000000..5da21d9784 --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/template_error.text.erb @@ -0,0 +1,8 @@ +<% @source_extract = @exception.source_extract(0, :html) %> +<%= @exception.original_exception.class.to_s %> in <%= @request.parameters["controller"].camelize if @request.parameters["controller"] %>#<%= @request.parameters["action"] %> + +Showing <%= @exception.file_name %> where line #<%= @exception.line_number %> raised: +<%= @exception.message %> +<%= @exception.sub_template_message %> +<%= render template: "rescues/_trace", format: :text %> +<%= render template: "rescues/_request_and_response", format: :text %> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/unknown_action.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/unknown_action.erb deleted file mode 100644 index 259fb2bb3b..0000000000 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/unknown_action.erb +++ /dev/null @@ -1,6 +0,0 @@ -
-

Unknown action

-
-
-

<%= h @exception.message %>

-
diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/unknown_action.html.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/unknown_action.html.erb new file mode 100644 index 0000000000..259fb2bb3b --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/unknown_action.html.erb @@ -0,0 +1,6 @@ +
+

Unknown action

+
+
+

<%= h @exception.message %>

+
diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/unknown_action.text.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/unknown_action.text.erb new file mode 100644 index 0000000000..83973addcb --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/unknown_action.text.erb @@ -0,0 +1,3 @@ +Unknown action + +<%= @exception.message %> diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb index ff0baccd76..3045a07ad6 100644 --- a/actionpack/test/dispatch/debug_exceptions_test.rb +++ b/actionpack/test/dispatch/debug_exceptions_test.rb @@ -128,6 +128,47 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest assert_match(/ActionController::ParameterMissing/, body) end + test "rescue with text error for xhr request" do + @app = DevelopmentApp + xhr_request_env = {'action_dispatch.show_exceptions' => true, 'HTTP_X_REQUESTED_WITH' => 'XMLHttpRequest'} + + get "/", {}, xhr_request_env + assert_response 500 + assert_no_match(//, body) + assert_equal response.content_type, "text/plain" + assert_match(/puke/, body) + + get "/not_found", {}, xhr_request_env + assert_response 404 + assert_no_match(//, body) + assert_equal response.content_type, "text/plain" + assert_match(/#{AbstractController::ActionNotFound.name}/, body) + + get "/method_not_allowed", {}, xhr_request_env + assert_response 405 + assert_no_match(//, body) + assert_equal response.content_type, "text/plain" + assert_match(/ActionController::MethodNotAllowed/, body) + + get "/unknown_http_method", {}, xhr_request_env + assert_response 405 + assert_no_match(//, body) + assert_equal response.content_type, "text/plain" + assert_match(/ActionController::UnknownHttpMethod/, body) + + get "/bad_request", {}, xhr_request_env + assert_response 400 + assert_no_match(//, body) + assert_equal response.content_type, "text/plain" + assert_match(/ActionController::BadRequest/, body) + + get "/parameter_missing", {}, xhr_request_env + assert_response 400 + assert_no_match(//, body) + assert_equal response.content_type, "text/plain" + assert_match(/ActionController::ParameterMissing/, body) + end + test "does not show filtered parameters" do @app = DevelopmentApp -- cgit v1.2.3