From cd9d28d6fdff6819dac3c6643fe882eb568b5a39 Mon Sep 17 00:00:00 2001 From: lest Date: Thu, 24 Nov 2011 22:37:48 +0300 Subject: middlewares should use logger from env --- .../lib/action_dispatch/middleware/params_parser.rb | 6 +++--- .../lib/action_dispatch/middleware/show_exceptions.rb | 16 ++++++++++------ actionpack/test/abstract_unit.rb | 4 ++-- .../test/dispatch/request/json_params_parsing_test.rb | 16 ++++++---------- .../test/dispatch/request/xml_params_parsing_test.rb | 16 ++++++---------- actionpack/test/dispatch/show_exceptions_test.rb | 7 +++++++ railties/lib/rails/application.rb | 3 ++- railties/test/application/configuration_test.rb | 1 + 8 files changed, 37 insertions(+), 32 deletions(-) diff --git a/actionpack/lib/action_dispatch/middleware/params_parser.rb b/actionpack/lib/action_dispatch/middleware/params_parser.rb index 84e3dd16dd..6ded9dbfed 100644 --- a/actionpack/lib/action_dispatch/middleware/params_parser.rb +++ b/actionpack/lib/action_dispatch/middleware/params_parser.rb @@ -52,7 +52,7 @@ module ActionDispatch false end rescue Exception => e # YAML, XML or Ruby code block errors - logger.debug "Error occurred while parsing request parameters.\nContents:\n\n#{request.raw_post}" + logger(env).debug "Error occurred while parsing request parameters.\nContents:\n\n#{request.raw_post}" raise e end @@ -68,8 +68,8 @@ module ActionDispatch nil end - def logger - defined?(Rails.logger) ? Rails.logger : Logger.new($stderr) + def logger(env) + env['action_dispatch.logger'] || Logger.new($stderr) end end end diff --git a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb index 52dce4cc81..8dc2820d37 100644 --- a/actionpack/lib/action_dispatch/middleware/show_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/show_exceptions.rb @@ -63,7 +63,7 @@ module ActionDispatch private def render_exception(env, exception) - log_error(exception) + log_error(env, exception) exception = original_exception(exception) if env['action_dispatch.show_detailed_exceptions'] == true @@ -124,14 +124,14 @@ module ActionDispatch defined?(Rails.public_path) ? Rails.public_path : 'public_path' end - def log_error(exception) - return unless logger + def log_error(env, exception) + return unless logger(env) ActiveSupport::Deprecation.silence do message = "\n#{exception.class} (#{exception.message}):\n" message << exception.annoted_source_code.to_s if exception.respond_to?(:annoted_source_code) message << " " << application_trace(exception).join("\n ") - logger.fatal("#{message}\n\n") + logger(env).fatal("#{message}\n\n") end end @@ -153,8 +153,12 @@ module ActionDispatch exception.backtrace end - def logger - defined?(Rails.logger) ? Rails.logger : Logger.new($stderr) + def logger(env) + env['action_dispatch.logger'] || stderr_logger + end + + def stderr_logger + Logger.new($stderr) end def original_exception(exception) diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index 24d071df39..cbb8968496 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -333,9 +333,9 @@ module ActionDispatch "#{FIXTURE_LOAD_PATH}/public" end - remove_method :logger + remove_method :stderr_logger # Silence logger - def logger + def stderr_logger nil end end diff --git a/actionpack/test/dispatch/request/json_params_parsing_test.rb b/actionpack/test/dispatch/request/json_params_parsing_test.rb index d481a2df13..ad44b4b16a 100644 --- a/actionpack/test/dispatch/request/json_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/json_params_parsing_test.rb @@ -32,16 +32,12 @@ class JsonParamsParsingTest < ActionDispatch::IntegrationTest test "logs error if parsing unsuccessful" do with_test_routing do - begin - $stderr = StringIO.new - json = "[\"person]\": {\"name\": \"David\"}}" - post "/parse", json, {'CONTENT_TYPE' => 'application/json', 'action_dispatch.show_exceptions' => true} - assert_response :error - $stderr.rewind && err = $stderr.read - assert err =~ /Error occurred while parsing request parameters/ - ensure - $stderr = STDERR - end + output = StringIO.new + json = "[\"person]\": {\"name\": \"David\"}}" + post "/parse", json, {'CONTENT_TYPE' => 'application/json', 'action_dispatch.show_exceptions' => true, 'action_dispatch.logger' => Logger.new(output)} + assert_response :error + output.rewind && err = output.read + assert err =~ /Error occurred while parsing request parameters/ end end diff --git a/actionpack/test/dispatch/request/xml_params_parsing_test.rb b/actionpack/test/dispatch/request/xml_params_parsing_test.rb index 65a25557b7..d8fa751548 100644 --- a/actionpack/test/dispatch/request/xml_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/xml_params_parsing_test.rb @@ -54,16 +54,12 @@ class XmlParamsParsingTest < ActionDispatch::IntegrationTest test "logs error if parsing unsuccessful" do with_test_routing do - begin - $stderr = StringIO.new - xml = "David#{ActiveSupport::Base64.encode64('ABC')}" - post "/parse", xml, default_headers.merge('action_dispatch.show_exceptions' => true) - assert_response :error - $stderr.rewind && err = $stderr.read - assert err =~ /Error occurred while parsing request parameters/ - ensure - $stderr = STDERR - end + output = StringIO.new + xml = "David#{ActiveSupport::Base64.encode64('ABC')}" + post "/parse", xml, default_headers.merge('action_dispatch.show_exceptions' => true, 'action_dispatch.logger' => Logger.new(output)) + assert_response :error + output.rewind && err = output.read + assert err =~ /Error occurred while parsing request parameters/ end end diff --git a/actionpack/test/dispatch/show_exceptions_test.rb b/actionpack/test/dispatch/show_exceptions_test.rb index 9f4d6c530f..5875725b5d 100644 --- a/actionpack/test/dispatch/show_exceptions_test.rb +++ b/actionpack/test/dispatch/show_exceptions_test.rb @@ -128,4 +128,11 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest get "/", {}, {'action_dispatch.show_exceptions' => true} assert_equal "text/html; charset=utf-8", response.headers["Content-Type"] end + + test 'uses logger from env' do + @app = ProductionApp + output = StringIO.new + get "/", {}, {'action_dispatch.show_exceptions' => true, 'action_dispatch.logger' => Logger.new(output)} + assert_match(/puke/, output.rewind && output.read) + end end diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index 847445d29f..25ff74506a 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -123,7 +123,8 @@ module Rails @env_config ||= super.merge({ "action_dispatch.parameter_filter" => config.filter_parameters, "action_dispatch.secret_token" => config.secret_token, - "action_dispatch.show_exceptions" => config.action_dispatch.show_exceptions + "action_dispatch.show_exceptions" => config.action_dispatch.show_exceptions, + "action_dispatch.logger" => Rails.logger }) end diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index f356805d6e..f37a024a0b 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -524,6 +524,7 @@ module ApplicationTests assert_equal app.env_config['action_dispatch.parameter_filter'], app.config.filter_parameters assert_equal app.env_config['action_dispatch.secret_token'], app.config.secret_token assert_equal app.env_config['action_dispatch.show_exceptions'], app.config.action_dispatch.show_exceptions + assert_equal app.env_config['action_dispatch.logger'], Rails.logger end end end -- cgit v1.2.3