Merge pull request #3747 from lest/middleware-logger

middlewares should have configurable logger

actionpack/lib/action_dispatch/middleware/params_parser.rb

@@ -52,7 +52,7 @@ def parse_formatted_parameters(env)
           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 @@ def content_type_from_legacy_post_data_format_header(env)
         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

actionpack/lib/action_dispatch/middleware/show_exceptions.rb

@@ -63,7 +63,7 @@ def call(env)
 
     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 @@ def public_path
         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 @@ def clean_backtrace(exception, *args)
           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)

actionpack/test/abstract_unit.rb

@@ -333,9 +333,9 @@ def public_path
         "#{FIXTURE_LOAD_PATH}/public"
       end
 
-      remove_method :logger
+      remove_method :stderr_logger
       # Silence logger
-      def logger
+      def stderr_logger
         nil
       end
   end

actionpack/test/dispatch/request/json_params_parsing_test.rb

@@ -32,16 +32,12 @@ def teardown
 
   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
 

actionpack/test/dispatch/request/xml_params_parsing_test.rb

@@ -54,16 +54,12 @@ def call(env)
 
   test "logs error if parsing unsuccessful" do
     with_test_routing do
-      begin
-        $stderr = StringIO.new
-        xml = "<person><name>David</name><avatar type='file' name='me.jpg' content_type='image/jpg'>#{ActiveSupport::Base64.encode64('ABC')}</avatar></pineapple>"
-        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 = "<person><name>David</name><avatar type='file' name='me.jpg' content_type='image/jpg'>#{ActiveSupport::Base64.encode64('ABC')}</avatar></pineapple>"
+      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
 

actionpack/test/dispatch/show_exceptions_test.rb

@@ -128,4 +128,11 @@ def call(env)
     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

railties/lib/rails/application.rb

@@ -123,7 +123,8 @@ def env_config
       @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
 

railties/test/application/configuration_test.rb

@@ -524,6 +524,7 @@ def index
       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