Merge pull request #255 from epimorphics/hotfix/reduce-log-noise

Hotfix: Logging improvements

CHANGELOG.md

@@ -4,6 +4,21 @@ This app allows the user to explore HMLR price-paid open linked data.
 
 ## Changelog
 
+## 1.7.10 - 2024-10
+
+- (Jon) Updated the message reported to the error page while in development mode
+  to include the error message and the status code; also configured the helper
+  to display the actual rails stack trace in development mode
+- (Jon) Added the `log_error` helper to the `render_error_page` helper method in
+  the `search_controller` to apply the appropriate log level based on the status
+  code
+- (Jon) Added `RoutingError` and `MissingTemplate` to the list of exceptions to
+  be caught by the `render_error_page` helper method in the `search_controller`
+- (Jon) Wrapped the Internal Error Instrumentation in an `unless` block to
+  ensure the application does not report internal errors to the Prometheus
+  metrics when the error is a 404 or 422 thereby reducing the noise in the Slack
+  alerts channel
+
 ## 1.7.9 - 2024-09
 
 - (Jon) Updated the application exceptions controller to instrument the

Makefile

@@ -69,10 +69,8 @@ lint: assets
 	@./bin/bundle exec rubocop
 
 local:
-	@echo "Installing all packages ..."
-	@./bin/bundle install
 	@echo "Starting local server ..."
-	@./bin/rails server -p ${PORT}
+	@API_SERVICE_URL=${API_SERVICE_URL} ./bin/rails server -p ${PORT}
 
 publish: image
 	@echo Publishing image: ${REPO}:${TAG} ...
@@ -98,8 +96,8 @@ tag:
 	@echo ${TAG}
 
 test: assets
-	@echo "Running unit tests ..."
-	@./bin/rails test:unit
+	@echo "Running tests ..."
+	@./bin/rails test
 	@echo "Running system tests ..."
 	@./bin/rails test:system
 

app/controllers/application_controller.rb

@@ -36,15 +36,18 @@ def log_request_result
   # or attempt to render a generic error page if no specific error page exists
   unless Rails.application.config.consider_all_requests_local
     rescue_from StandardError do |e|
-      # Instrument ActiveSupport::Notifications for internal errors:
-      ActiveSupport::Notifications.instrument('internal_error.application', exception: e)
+      # Instrument ActiveSupport::Notifications for internal errors but only for non-404 errors:
+      unless e.is_a?(ActionController::RoutingError) || e.is_a?(ActionView::MissingTemplate)
+        ActiveSupport::Notifications.instrument('internal_error.application', exception: e)
+      end
+
       # Trigger the appropriate error handling method based on the exception
       case e.class
-      when ActiveRecord::RecordNotFound, ActionController::RoutingError, ActionView::MissingTemplate
+      when ActionController::RoutingError, ActionView::MissingTemplate
         :render404
       when ActionController::InvalidCrossOriginRequest
         :render403
-      when ActiveRecord::RecordInvalid, ActionController::ParameterMissing
+      when ActionController::BadRequest, ActionController::ParameterMissing
         :render400
       else
         :handle_internal_error
@@ -101,7 +104,6 @@ def reset_response
   # rubocop:disable Metrics/AbcSize, Metrics/MethodLength
   def detailed_request_log(duration)
     env = request.env
-
     log_fields = {
       duration: duration,
       request_id: env['X_REQUEST_ID'],
@@ -113,7 +115,7 @@ def detailed_request_log(duration)
       body: request.body.gets&.gsub("\n", '\n'),
       method: request.method,
       status: response.status,
-      message: Rack::Utils::HTTP_STATUS_CODES[response.status]
+      message: response.message || Rack::Utils::HTTP_STATUS_CODES[response.status]
     }
 
     case response.status

app/controllers/search_controller.rb

@@ -8,7 +8,7 @@ def index
     create
   end
 
-  def create # rubocop:disable Metrics/AbcSize, Metrics/MethodLength, Metrics/PerceivedComplexity
+  def create # rubocop:disable Metrics/MethodLength
     @preferences = UserPreferences.new(params)
 
     if @preferences.empty?
@@ -25,15 +25,22 @@ def create # rubocop:disable Metrics/AbcSize, Metrics/MethodLength, Metrics/Perc
     end
   rescue StandardError => e
     e = e.cause || e
+    rescue_standard_error(e)
+  end
 
-    status = case e
+  # Determine status symbol to pass to the error page
+  def rescue_standard_error(err)
+    status = case err
+             when RoutingError, MissingTemplate
+               :not_found
              when MalformedSearchError, ArgumentError
                :bad_request
              else
                :internal_server_error
              end
-
-    render_error_page(e, e.message, status) if !Rails.env.development?
+    # Display the actual rails error stack trace while in development
+    render_error_page(err, err.message, status) unless Rails.env.development?
+    # To check the error page in development, set the RAILS_ENV to production or test
   end
 
   def use_compact_json?
@@ -54,10 +61,10 @@ def render_error_page(err, message, status, template = 'ppd/error')
     @message = message
 
     # log the error with as much detail as possible in development to aid in resolving the issue
-    @message = "#{err.class.name} error: #{message}" if Rails.env.development?
+    @message = "#{Rack::Utils::SYMBOL_TO_STATUS_CODE[status]} ~ #{err.class.name} error: #{message}" if Rails.env.development?
 
     # Keep it simple silly in production!
-    Rails.logger.error message
+    log_error(Rack::Utils::SYMBOL_TO_STATUS_CODE[status], message)
 
     @error_message =
       [
@@ -68,4 +75,17 @@ def render_error_page(err, message, status, template = 'ppd/error')
     render(template: template, status: status)
   end
   # rubocop:enable Layout/LineLength, Metrics/MethodLength
+
+  # Log the error with the appropriate log level based on the status code
+  def log_error(status, message)
+    case status
+    when 500..599
+      Rails.logger.error(message)
+    when 400..499
+      Rails.logger.warn(message)
+    else
+      Rails.logger.info(message)
+    end
+  end
+
 end

app/lib/version.rb

@@ -3,7 +3,7 @@
 module Version
   MAJOR = 1
   MINOR = 7
-  PATCH = 9
+  PATCH = 10
   SUFFIX = nil
   VERSION = "#{MAJOR}.#{MINOR}.#{PATCH}#{SUFFIX && ".#{SUFFIX}"}"
 end