diff --git a/CHANGELOG.md b/CHANGELOG.md index b634e96e..d44af1dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/Gemfile b/Gemfile index adf59b00..f1cfde74 100644 --- a/Gemfile +++ b/Gemfile @@ -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. diff --git a/gemfiles/Gemfile.actionpack3.2 b/gemfiles/Gemfile.actionpack3.2 index 0ba9c17a..cd9f1861 100644 --- a/gemfiles/Gemfile.actionpack3.2 +++ b/gemfiles/Gemfile.actionpack3.2 @@ -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. diff --git a/gemfiles/Gemfile.actionpack4.0 b/gemfiles/Gemfile.actionpack4.0 index 42cdf51b..608b2e5c 100644 --- a/gemfiles/Gemfile.actionpack4.0 +++ b/gemfiles/Gemfile.actionpack4.0 @@ -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. diff --git a/gemfiles/Gemfile.actionpack4.2 b/gemfiles/Gemfile.actionpack4.2 index e9914f42..74f0d6a2 100644 --- a/gemfiles/Gemfile.actionpack4.2 +++ b/gemfiles/Gemfile.actionpack4.2 @@ -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. diff --git a/lib/lograge/log_subscriber.rb b/lib/lograge/log_subscriber.rb index deefdc88..d5d1e31d 100644 --- a/lib/lograge/log_subscriber.rb +++ b/lib/lograge/log_subscriber.rb @@ -1,5 +1,4 @@ require 'json' - require 'active_support/core_ext/class/attribute' require 'active_support/log_subscriber' @@ -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) @@ -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 diff --git a/spec/lograge_logsubscriber_spec.rb b/spec/lograge_logsubscriber_spec.rb index 242e4fad..0b9f1119 100644 --- a/spec/lograge_logsubscriber_spec.rb +++ b/spec/lograge_logsubscriber_spec.rb @@ -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 } @@ -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