Skip to content

Commit

Permalink
Deprecate exception#original_exception in favor of exception#cause
Browse files Browse the repository at this point in the history
  • Loading branch information
yuki24 committed Feb 3, 2015
1 parent 826ed62 commit 8b447f6
Show file tree
Hide file tree
Showing 29 changed files with 141 additions and 107 deletions.
11 changes: 3 additions & 8 deletions actionpack/lib/action_controller/metal/exceptions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,9 @@ class ActionControllerError < StandardError #:nodoc:
end

class BadRequest < ActionControllerError #:nodoc:
attr_reader :original_exception

def initialize(type = nil, e = nil)
return super() unless type && e

super("Invalid #{type} parameters: #{e.message}")
@original_exception = e
set_backtrace e.backtrace
def original_exception
ActiveSupport::Deprecation.warn("#original_exception is deprecated. Use #cause instead.", caller)
cause
end
end

Expand Down
6 changes: 2 additions & 4 deletions actionpack/lib/action_controller/metal/rescue.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,8 @@ module Rescue
include ActiveSupport::Rescuable

def rescue_with_handler(exception)
if (exception.respond_to?(:original_exception) &&
(orig_exception = exception.original_exception) &&
handler_for_rescue(orig_exception))
exception = orig_exception
if exception.cause && handler_for_rescue(exception.cause)
exception = exception.cause
end
super(exception)
end
Expand Down
4 changes: 2 additions & 2 deletions actionpack/lib/action_dispatch/http/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -311,15 +311,15 @@ def session_options=(options)
def GET
@env["action_dispatch.request.query_parameters"] ||= Utils.deep_munge(normalize_encode_params(super || {}))
rescue Rack::Utils::ParameterTypeError, Rack::Utils::InvalidParameterError => e
raise ActionController::BadRequest.new(:query, e)
raise ActionController::BadRequest, "Invalid query parameters: #{e.message}", e.backtrace
end
alias :query_parameters :GET

# Override Rack's POST method to support indifferent access
def POST
@env["action_dispatch.request.request_parameters"] ||= Utils.deep_munge(normalize_encode_params(super || {}))
rescue Rack::Utils::ParameterTypeError, Rack::Utils::InvalidParameterError => e
raise ActionController::BadRequest.new(:request, e)
raise ActionController::BadRequest, "Invalid request parameters: #{e.message}", e.backtrace
end
alias :request_parameters :POST

Expand Down
10 changes: 3 additions & 7 deletions actionpack/lib/action_dispatch/middleware/exception_wrapper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def initialize(env, exception)
@env = env
@exception = original_exception(exception)

expand_backtrace if exception.is_a?(SyntaxError) || exception.try(:original_exception).try(:is_a?, SyntaxError)
expand_backtrace if exception.is_a?(SyntaxError) || exception.cause.is_a?(SyntaxError)
end

def rescue_template
Expand Down Expand Up @@ -103,17 +103,13 @@ def backtrace
end

def original_exception(exception)
if registered_original_exception?(exception)
exception.original_exception
if @@rescue_responses.has_key?(exception.cause.class.name)
exception.cause
else
exception
end
end

def registered_original_exception?(exception)
exception.respond_to?(:original_exception) && @@rescue_responses.has_key?(exception.original_exception.class.name)
end

def clean_backtrace(*args)
if backtrace_cleaner
backtrace_cleaner.clean(backtrace, *args)
Expand Down
13 changes: 7 additions & 6 deletions actionpack/lib/action_dispatch/middleware/params_parser.rb
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
require 'active_support/core_ext/hash/conversions'
require 'action_dispatch/http/request'
require 'active_support/core_ext/hash/indifferent_access'
require "active_support/core_ext/module/delegation"

module ActionDispatch
class ParamsParser
class ParseError < StandardError
attr_reader :original_exception
delegate :to_s, to: :cause

def initialize(message, original_exception)
super(message)
@original_exception = original_exception
def original_exception
ActiveSupport::Deprecation.warn("#original_exception is deprecated. Use #cause instead.", caller)
cause
end
end

Expand Down Expand Up @@ -47,10 +48,10 @@ def parse_formatted_parameters(env)
else
false
end
rescue => e # JSON or Ruby code block errors
rescue # JSON or Ruby code block errors
logger(env).debug "Error occurred while parsing request parameters.\nContents:\n\n#{request.raw_post}"

raise ParseError.new(e.message, e)
raise ParseError
end

def logger(env)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,22 @@
require 'rack/session/abstract/id'
require 'action_dispatch/middleware/cookies'
require 'action_dispatch/request/session'
require "active_support/core_ext/module/delegation"

module ActionDispatch
module Session
class SessionRestoreError < StandardError #:nodoc:
attr_reader :original_exception

def initialize(const_error)
@original_exception = const_error
delegate :backtrace, to: :cause

def initialize(message)
super("Session contains objects whose class definition isn't available.\n" +
"Remember to require the classes for all objects kept in the session.\n" +
"(Original exception: #{const_error.message} [#{const_error.class}])\n")
"(Original exception: #{message})\n")
end

def original_exception
ActiveSupport::Deprecation.warn("#original_exception is deprecated. Use #cause instead.", caller)
cause
end
end

Expand Down Expand Up @@ -55,7 +59,7 @@ def stale_session_check!
# Note that the regexp does not allow $1 to end with a ':'
$1.constantize
rescue LoadError, NameError => e
raise ActionDispatch::Session::SessionRestoreError, e, e.backtrace
raise ActionDispatch::Session::SessionRestoreError, "#{e.message} [#{e.class}]"
end
retry
else
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<header>
<h1>
<%= @exception.original_exception.class.to_s %> in
<%= @exception.cause.class.to_s %> in
<%= @request.parameters["controller"].camelize if @request.parameters["controller"] %>#<%= @request.parameters["action"] %>
</h1>
</header>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<%= @exception.original_exception.class.to_s %> in <%= @request.parameters["controller"].camelize if @request.parameters["controller"] %>#<%= @request.parameters["action"] %>
<%= @exception.cause.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 %>
Expand Down
12 changes: 10 additions & 2 deletions actionpack/test/controller/rescue_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,19 @@ def missing_template
end

def io_error_in_view
raise ActionView::TemplateError.new(nil, IOError.new('this is io error'))
begin
raise IOError.new('this is io error')
rescue
raise ActionView::TemplateError.new(nil)
end
end

def zero_division_error_in_view
raise ActionView::TemplateError.new(nil, ZeroDivisionError.new('this is zero division error'))
begin
raise ZeroDivisionError.new('this is zero division error')
rescue
raise ActionView::TemplateError.new(nil)
end
end

protected
Expand Down
10 changes: 7 additions & 3 deletions actionpack/test/dispatch/debug_exceptions_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,11 @@ def call(env)
when "/unprocessable_entity"
raise ActionController::InvalidAuthenticityToken
when "/not_found_original_exception"
raise ActionView::Template::Error.new('template', AbstractController::ActionNotFound.new)
begin
raise AbstractController::ActionNotFound.new
rescue
raise ActionView::Template::Error.new('template')
end
when "/missing_template"
raise ActionView::MissingTemplate.new(%w(foo), 'foo/index', %w(foo), false, 'mailer')
when "/bad_request"
Expand All @@ -56,12 +60,12 @@ def call(env)
when "/syntax_error_into_view"
begin
eval 'broke_syntax ='
rescue Exception => e
rescue Exception
template = ActionView::Template.new(File.read(__FILE__),
__FILE__,
ActionView::Template::Handlers::Raw.new,
{})
raise ActionView::Template::Error.new(template, e)
raise ActionView::Template::Error.new(template)
end
when "/framework_raises"
method_that_raises
Expand Down
4 changes: 2 additions & 2 deletions actionpack/test/dispatch/request/json_params_parsing_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ def teardown
$stderr = StringIO.new # suppress the log
json = "[\"person]\": {\"name\": \"David\"}}"
exception = assert_raise(ActionDispatch::ParamsParser::ParseError) { post "/parse", json, {'CONTENT_TYPE' => 'application/json', 'action_dispatch.show_exceptions' => false} }
assert_equal JSON::ParserError, exception.original_exception.class
assert_equal exception.original_exception.message, exception.message
assert_equal JSON::ParserError, exception.cause.class
assert_equal exception.cause.message, exception.message
ensure
$stderr = STDERR
end
Expand Down
4 changes: 2 additions & 2 deletions actionpack/test/dispatch/request_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -970,8 +970,8 @@ class RequestParameters < BaseRequestTest
request.parameters
end

assert e.original_exception
assert_equal e.original_exception.backtrace, e.backtrace
assert_not_nil e.cause
assert_equal e.cause.backtrace, e.backtrace
end
end

Expand Down
12 changes: 10 additions & 2 deletions actionpack/test/dispatch/show_exceptions_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,21 @@ def call(env)
when "/not_found"
raise AbstractController::ActionNotFound
when "/bad_params"
raise ActionDispatch::ParamsParser::ParseError.new("", StandardError.new)
begin
raise StandardError.new
rescue
raise ActionDispatch::ParamsParser::ParseError.new("")
end
when "/method_not_allowed"
raise ActionController::MethodNotAllowed
when "/unknown_http_method"
raise ActionController::UnknownHttpMethod
when "/not_found_original_exception"
raise ActionView::Template::Error.new('template', AbstractController::ActionNotFound.new)
begin
raise AbstractController::ActionNotFound.new
rescue
raise ActionView::Template::Error.new('template')
end
else
raise "puke!"
end
Expand Down
26 changes: 11 additions & 15 deletions actionview/lib/action_view/template.rb
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,17 @@ def render(view, locals, buffer=nil, &block)
view.send(method_name, locals, buffer, &block)
end
rescue => e
handle_render_error(view, e)
if e.is_a?(Template::Error)
e.sub_template_of(self)
raise e
else
template = self
unless template.source
template = refresh(view)
template.encode!
end
raise Template::Error.new(template)
end
end

def type
Expand Down Expand Up @@ -297,20 +307,6 @@ def #{method_name}(local_assigns, output_buffer)
ObjectSpace.define_finalizer(self, Finalizer[method_name, mod])
end

def handle_render_error(view, e) #:nodoc:
if e.is_a?(Template::Error)
e.sub_template_of(self)
raise e
else
template = self
unless template.source
template = refresh(view)
template.encode!
end
raise Template::Error.new(template, e)
end
end

def locals_code #:nodoc:
# Double assign to suppress the dreaded 'assigned but unused variable' warning
@locals.each_with_object('') { |key, code| code << "#{key} = #{key} = local_assigns[:#{key}];" }
Expand Down
14 changes: 8 additions & 6 deletions actionview/lib/action_view/template/error.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require "active_support/core_ext/enumerable"
require "active_support/core_ext/module/delegation"

module ActionView
# = Action View Errors
Expand Down Expand Up @@ -58,14 +59,15 @@ class Template
# precise exception message.
class Error < ActionViewError #:nodoc:
SOURCE_CODE_RADIUS = 3
delegate :backtrace, :to_s, to: :cause

attr_reader :original_exception
def initialize(template)
@template, @sub_templates = template, nil
end

def initialize(template, original_exception)
super(original_exception.message)
@template, @original_exception = template, original_exception
@sub_templates = nil
set_backtrace(original_exception.backtrace)
def original_exception
ActiveSupport::Deprecation.warn("#original_exception is deprecated. Use #cause instead.", caller)
cause
end

def file_name
Expand Down
8 changes: 4 additions & 4 deletions actionview/test/template/render_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -346,8 +346,8 @@ def test_render_partial_without_object_or_collection_does_not_generate_partial_n
exception = assert_raises ActionView::Template::Error do
@controller_view.render("partial_name_local_variable")
end
assert_instance_of NameError, exception.original_exception
assert_equal :partial_name_local_variable, exception.original_exception.name
assert_instance_of NameError, exception.cause
assert_equal :partial_name_local_variable, exception.cause.name
end

# TODO: The reason for this test is unclear, improve documentation
Expand Down Expand Up @@ -584,14 +584,14 @@ def test_render_utf8_template_with_default_external_encoding
def test_render_utf8_template_with_incompatible_external_encoding
with_external_encoding Encoding::SHIFT_JIS do
e = assert_raises(ActionView::Template::Error) { @view.render(:file => "test/utf8", :formats => [:html], :layouts => "layouts/yield") }
assert_match 'Your template was not saved as valid Shift_JIS', e.original_exception.message
assert_match 'Your template was not saved as valid Shift_JIS', e.cause.message
end
end

def test_render_utf8_template_with_partial_with_incompatible_encoding
with_external_encoding Encoding::SHIFT_JIS do
e = assert_raises(ActionView::Template::Error) { @view.render(:file => "test/utf8_magic_with_bare_partial", :formats => [:html], :layouts => "layouts/yield") }
assert_match 'Your template was not saved as valid Shift_JIS', e.original_exception.message
assert_match 'Your template was not saved as valid Shift_JIS', e.cause.message
end
end

Expand Down
25 changes: 20 additions & 5 deletions actionview/test/template/template_error_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,34 @@

class TemplateErrorTest < ActiveSupport::TestCase
def test_provides_original_message
error = ActionView::Template::Error.new("test", Exception.new("original"))
error = begin
raise Exception.new("original")
rescue Exception
raise ActionView::Template::Error.new("test") rescue $!
end

assert_equal "original", error.message
end

def test_provides_original_backtrace
original_exception = Exception.new
original_exception.set_backtrace(%W[ foo bar baz ])
error = ActionView::Template::Error.new("test", original_exception)
error = begin
original_exception = Exception.new
original_exception.set_backtrace(%W[ foo bar baz ])
raise original_exception
rescue Exception
raise ActionView::Template::Error.new("test") rescue $!
end

assert_equal %W[ foo bar baz ], error.backtrace
end

def test_provides_useful_inspect
error = ActionView::Template::Error.new("test", Exception.new("original"))
error = begin
raise Exception.new("original")
rescue Exception
raise ActionView::Template::Error.new("test") rescue $!
end

assert_equal "#<ActionView::Template::Error: original>", error.inspect
end
end
Loading

0 comments on commit 8b447f6

Please sign in to comment.