From 519577ecc3662fa4645c222f08e5d5c9812218b7 Mon Sep 17 00:00:00 2001 From: Hartley McGuire Date: Sun, 25 Jun 2023 22:52:14 -0400 Subject: [PATCH] Make the log level in DebugExceptions configurable This middleware has been logging at a FATAL level since the first [commit][1] in Rails (the code originally lived in actionpack/lib/action_controller/rescue.rb). However, FATAL is documented in the Ruby Logger [docs][2] as being for "An unhandleable error that results in a program crash.", which does not really apply to this case since DebugExceptions is handling the error. A more appropriate level would be ERROR, which the Ruby Logger docs describe as "A handleable error condition." This commit introduces a new configuration for the DebugExceptions log level so that new apps will have it set to ERROR by default and ERROR can eventually be made the default. [1]: db045dbbf60b53dbe013ef25554fd013baf88134 [2]: https://ruby-doc.org/3.2.1/stdlibs/logger/Logger/Severity.html --- .../middleware/debug_exceptions.rb | 10 ++++--- actionpack/lib/action_dispatch/railtie.rb | 1 + .../test/dispatch/debug_exceptions_test.rb | 13 ++++++++ guides/source/configuring.md | 13 ++++++++ railties/CHANGELOG.md | 8 +++++ railties/lib/rails/application.rb | 1 + .../lib/rails/application/configuration.rb | 1 + .../new_framework_defaults_7_1.rb.tt | 4 +++ .../test/application/configuration_test.rb | 30 +++++++++++++++++++ 9 files changed, 77 insertions(+), 4 deletions(-) diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index fa24fa8bed1b5..187678d18a324 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -145,16 +145,18 @@ def log_error(request, wrapper) message << " " message.concat(trace) - log_array(logger, message) + log_array(logger, message, request) end - def log_array(logger, lines) + def log_array(logger, lines, request) return if lines.empty? + level = request.get_header("action_dispatch.debug_exception_log_level") + if logger.formatter && logger.formatter.respond_to?(:tags_text) - logger.fatal lines.join("\n#{logger.formatter.tags_text}") + logger.add(level, lines.join("\n#{logger.formatter.tags_text}")) else - logger.fatal lines.join("\n") + logger.add(level, lines.join("\n")) end end diff --git a/actionpack/lib/action_dispatch/railtie.rb b/actionpack/lib/action_dispatch/railtie.rb index 61ff10d878210..33a87d14a11fe 100644 --- a/actionpack/lib/action_dispatch/railtie.rb +++ b/actionpack/lib/action_dispatch/railtie.rb @@ -26,6 +26,7 @@ class Railtie < Rails::Railtie # :nodoc: config.action_dispatch.perform_deep_munge = true config.action_dispatch.request_id_header = "X-Request-Id" config.action_dispatch.log_rescued_responses = true + config.action_dispatch.debug_exception_log_level = :fatal config.action_dispatch.default_headers = { "X-Frame-Options" => "SAMEORIGIN", diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb index 542096a9a1e38..d9ad35baf5bb7 100644 --- a/actionpack/test/dispatch/debug_exceptions_test.rb +++ b/actionpack/test/dispatch/debug_exceptions_test.rb @@ -456,6 +456,19 @@ def call(env) assert_match(/puke/, output.rewind && output.read) end + test "logs at configured log level" do + @app = DevelopmentApp + output = StringIO.new + logger = Logger.new(output) + logger.level = Logger::WARN + + get "/", headers: { "action_dispatch.show_exceptions" => :all, "action_dispatch.logger" => logger, "action_dispatch.debug_exception_log_level" => Logger::INFO } + assert_no_match(/puke/, output.rewind && output.read) + + get "/", headers: { "action_dispatch.show_exceptions" => :all, "action_dispatch.logger" => logger, "action_dispatch.debug_exception_log_level" => Logger::ERROR } + assert_match(/puke/, output.rewind && output.read) + end + test "logs only what is necessary" do @app = DevelopmentApp io = StringIO.new diff --git a/guides/source/configuring.md b/guides/source/configuring.md index 2147fb622b811..03964dd510930 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -61,6 +61,7 @@ Below are the default values associated with each target version. In cases of co #### Default Values for Target Version 7.1 - [`config.action_controller.allow_deprecated_parameters_hash_equality`](#config-action-controller-allow-deprecated-parameters-hash-equality): `false` +- [`config.action_dispatch.debug_exception_log_level`](#config-action-dispatch-debug-exception-log-level): `:error` - [`config.action_dispatch.default_headers`](#config-action-dispatch-default-headers): `{ "X-Frame-Options" => "SAMEORIGIN", "X-XSS-Protection" => "0", "X-Content-Type-Options" => "nosniff", "X-Permitted-Cross-Domain-Policies" => "none", "Referrer-Policy" => "strict-origin-when-cross-origin" }` - [`config.action_view.sanitizer_vendor`](#config-action-view-sanitizer-vendor): `Rails::HTML::Sanitizer.best_supported_vendor` - [`config.active_job.use_big_decimal_serializer`](#config-active-job-use-big-decimal-serializer): `true` @@ -1689,6 +1690,18 @@ The default value depends on the `config.load_defaults` target version: | (original) | `:marshal` | | 7.0 | `:json` | +#### `config.action_dispatch.debug_exception_log_level` + +Configure the log level used by the DebugExceptions middleware when logging +uncaught exceptions during requests + +The default value depends on the `config.load_defaults` target version: + +| Starting with version | The default value is | +| --------------------- | -------------------- | +| (original) | `:fatal` | +| 7.1 | `:error` | + #### `config.action_dispatch.default_headers` Is a hash with HTTP headers that are set by default in each response. diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index ae5c06557f76b..171c156e08288 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,11 @@ +* Add `config.action_dispatch.debug_exception_log_level` to configure the log + level used by `ActionDispatch::DebugExceptions`. + + The default is `:fatal`, but with `load_defaults "7.1"` the default will be + `:error`. + + *Hartley McGuire* + * The new method `config.autoload_lib(ignore:)` provides a simple way to autoload from `lib`: diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index 47ae12bfea363..a6b67505135fb 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -303,6 +303,7 @@ def env_config "action_dispatch.show_exceptions" => config.action_dispatch.show_exceptions, "action_dispatch.show_detailed_exceptions" => config.consider_all_requests_local, "action_dispatch.log_rescued_responses" => config.action_dispatch.log_rescued_responses, + "action_dispatch.debug_exception_log_level" => ActiveSupport::Logger.const_get(config.action_dispatch.debug_exception_log_level.to_s.upcase), "action_dispatch.logger" => Rails.logger, "action_dispatch.backtrace_cleaner" => Rails.backtrace_cleaner, "action_dispatch.key_generator" => key_generator, diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index a10fb545736ae..5b64c6289568f 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -297,6 +297,7 @@ def load_defaults(target_version) "X-Permitted-Cross-Domain-Policies" => "none", "Referrer-Policy" => "strict-origin-when-cross-origin" } + action_dispatch.debug_exception_log_level = :error end if respond_to?(:active_job) diff --git a/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_7_1.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_7_1.rb.tt index 3e78878c35312..e63851ecc732b 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_7_1.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_7_1.rb.tt @@ -190,3 +190,7 @@ # In previous versions of Rails, Action View always used `Rails::HTML4::Sanitizer`. # # Rails.application.config.action_view.sanitizer_vendor = Rails::HTML::Sanitizer.best_supported_vendor + +# Configure the log level used by the DebugExceptions middleware when logging +# uncaught exceptions during requests +# Rails.application.config.action_dispatch.debug_exception_log_level = :error diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index 9f83f7fe987eb..8ae8307255252 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -2622,6 +2622,36 @@ def index assert_equal :default, Rails.configuration.debug_exception_response_format end + test "debug_exception_log_level is :fatal by default for upgraded apps" do + make_basic_app + + class ::OmgController < ActionController::Base + def index + render plain: request.env["action_dispatch.debug_exception_log_level"] + end + end + + get "/" + + assert_equal "4", last_response.body + end + + test "debug_exception_log_level is :error for new apps" do + make_basic_app do |app| + app.config.load_defaults "7.1" + end + + class ::OmgController < ActionController::Base + def index + render plain: request.env["action_dispatch.debug_exception_log_level"] + end + end + + get "/" + + assert_equal "3", last_response.body + end + test "ActiveRecord::Base.has_many_inversing is true by default for new apps" do app "development"