Skip to content

Commit

Permalink
Log RecordNotFound as 404
Browse files Browse the repository at this point in the history
  • Loading branch information
pxlpnk committed Feb 7, 2015
1 parent 940cba8 commit 682e873
Show file tree
Hide file tree
Showing 7 changed files with 25 additions and 9 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### Fixed
* Make sure rack_cache[:verbose] can be set #103
* Follow hash syntax for logstash-event v1.4.x #75
* Log RecordNotFound as 404 #27, #110, #112

### Other
* Use https in Gemfile #104
Expand Down
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ gemspec

group :test do
gem 'actionpack'
gem 'activerecord'
# logstash does not release any gems on rubygems, but they have two gemspecs within their repo.
# Using the tag is an attempt of having a stable version to test against where we can ensure that
# we test against the correct code.
Expand Down
1 change: 1 addition & 0 deletions gemfiles/Gemfile.actionpack3.2
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ gemspec :path => ".."

group :test do
gem 'actionpack', '~> 3.2.0'
gem 'activerecord', '~> 3.2.0'
# logstash does not release any gems on rubygems, but they have two gemspecs within their repo.
# Using the tag is an attempt of having a stable version to test against where we can ensure that
# we test against the correct code.
Expand Down
1 change: 1 addition & 0 deletions gemfiles/Gemfile.actionpack4.0
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ gemspec :path => ".."

group :test do
gem 'actionpack', '~> 4.0.0'
gem 'activerecord', '~> 4.0.0'
# logstash does not release any gems on rubygems, but they have two gemspecs within their repo.
# Using the tag is an attempt of having a stable version to test against where we can ensure that
# we test against the correct code.
Expand Down
2 changes: 2 additions & 0 deletions gemfiles/Gemfile.actionpack4.2
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ gemspec :path => ".."

group :test do
gem 'actionpack', '~> 4.2.0'
gem 'activerecord', '~> 4.2.0'

# logstash does not release any gems on rubygems, but they have two gemspecs within their repo.
# Using the tag is an attempt of having a stable version to test against where we can ensure that
# we test against the correct code.
Expand Down
13 changes: 9 additions & 4 deletions lib/lograge/log_subscriber.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
require 'json'

require 'active_support/core_ext/class/attribute'
require 'active_support/log_subscriber'

Expand All @@ -10,7 +9,7 @@ def process_action(event)

payload = event.payload

data = extract_request(payload)
data = extract_request(payload)
extract_status(data, payload)
runtimes(data, event)
location(data)
Expand Down Expand Up @@ -62,13 +61,19 @@ def extract_status(data, payload)
data[:status] = status.to_i
elsif (error = payload[:exception])
exception, message = error
data[:status] = 500
data[:error] = "#{exception}:#{message}"
data[:status] = get_error_status_code(exception)
data[:error] = "#{exception}: #{message}"
else
data[:status] = 0
end
end

def get_error_status_code(exception)
exception_object = exception.constantize.new
exception_wrapper = ::ActionDispatch::ExceptionWrapper.new({}, exception_object)
exception_wrapper.status_code
end

def custom_options(data, event)
options = Lograge.custom_options(event)
data.merge! options if options
Expand Down
15 changes: 10 additions & 5 deletions spec/lograge_logsubscriber_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
require 'active_support/notifications'
require 'active_support/core_ext/string'
require 'logger'
require 'active_record/errors'
require 'rails'

describe Lograge::RequestLogSubscriber do
let(:log_output) { StringIO.new }
Expand Down Expand Up @@ -120,14 +122,17 @@
expect(log_output.string).to match(/db=0.02/)
end

it 'adds a 500 status when an exception occurred' do
it 'adds a 404 status when an exception occurred' do
error_double = double(ActionDispatch::ExceptionWrapper, status_code: 404)
expect(ActionDispatch::ExceptionWrapper).to(receive(:new).with({}, ActiveRecord::RecordNotFound.new)
.and_return(error_double))

event.payload[:exception] = ['ActiveRecord::RecordNotFound', 'Record not found']
event.payload[:status] = nil
event.payload[:exception] = ['AbstractController::ActionNotFound', 'Route not found']
subscriber.process_action(event)

expect(log_output.string).to match(/status=500 /)
expect(log_output.string).to match(/status=404 /)
expect(log_output.string).to match(
/error='AbstractController::ActionNotFound:Route not found' /
/error='ActiveRecord::RecordNotFound: Record not found' /
)
end

Expand Down

0 comments on commit 682e873

Please sign in to comment.