Skip to content

Commit

Permalink
Merge pull request #7660 from senny/7646_wrong_status_code_on_not_found
Browse files Browse the repository at this point in the history
log 404 status when ActiveRecord::RecordNotFound was raised (#7646)
  • Loading branch information
rafaelfranca committed Sep 17, 2012
2 parents 3c48b32 + dadfa9a commit 8f0d332
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 8 deletions.
16 changes: 10 additions & 6 deletions actionpack/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,17 +1,21 @@
## Rails 4.0.0 (unreleased) ##

* Fix #7646, the log now displays the correct status code when an exception is raised.

*Yves Senn*

* Allow pass couple extensions to ActionView::Template.register_template_handler call. *Tima Maslyuchenko*

* Sprockets integration has been extracted from Action Pack and the `sprockets-rails`
* Sprockets integration has been extracted from Action Pack and the `sprockets-rails`
gem should be added to Gemfile (under the assets group) in order to use Rails asset
pipeline in future versions of Rails.
pipeline in future versions of Rails.

*Guillermo Iguaran*

* `ActionDispatch::Session::MemCacheStore` now uses `dalli` instead of the deprecated
`memcache-client` gem. As side effect the autoloading of unloaded classes objects
saved as values in session isn't supported anymore when mem_cache session store is
used, this can have an impact in apps only when config.cache_classes is false.
* `ActionDispatch::Session::MemCacheStore` now uses `dalli` instead of the deprecated
`memcache-client` gem. As side effect the autoloading of unloaded classes objects
saved as values in session isn't supported anymore when mem_cache session store is
used, this can have an impact in apps only when config.cache_classes is false.

*Arun Agrawal + Guillermo Iguaran*

Expand Down
3 changes: 2 additions & 1 deletion actionpack/lib/action_controller/log_subscriber.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ def process_action(event)

status = payload[:status]
if status.nil? && payload[:exception].present?
status = ActionDispatch::ExceptionWrapper.new({}, payload[:exception]).status_code
exception_class_name = payload[:exception].first
status = ActionDispatch::ExceptionWrapper.status_code_for_exception(exception_class_name)
end
message = "Completed #{status} #{Rack::Utils::HTTP_STATUS_CODES[status]} in %.0fms" % event.duration
message << " (#{additions.join(" | ")})" unless additions.blank?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def rescue_template
end

def status_code
Rack::Utils.status_code(@@rescue_responses[@exception.class.name])
self.class.status_code_for_exception(@exception.class.name)
end

def application_trace
Expand All @@ -52,6 +52,10 @@ def full_trace
clean_backtrace(:all)
end

def self.status_code_for_exception(class_name)
Rack::Utils.status_code(@@rescue_responses[class_name])
end

private

def original_exception(exception)
Expand Down
15 changes: 15 additions & 0 deletions actionpack/test/controller/log_subscriber_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ def with_exception
def with_rescued_exception
raise SpecialException
end

def with_action_not_found
raise AbstractController::ActionNotFound
end
end
end

Expand Down Expand Up @@ -225,6 +229,17 @@ def test_process_action_with_rescued_exception_includes_http_status_code
assert_match(/Completed 406/, logs.last)
end

def test_process_action_with_with_action_not_found_logs_404
begin
get :with_action_not_found
wait
rescue AbstractController::ActionNotFound
end

assert_equal 2, logs.size
assert_match(/Completed 404/, logs.last)
end

def logs
@logs ||= @logger.logged(:info)
end
Expand Down

0 comments on commit 8f0d332

Please sign in to comment.