lumber icon indicating copy to clipboard operation
lumber copied to clipboard

Compatibility with rails 4.1.8 in development mode. Fix issue#12

Open bestmike007 opened this issue 10 years ago • 6 comments

bestmike007 avatar Dec 05 '14 10:12 bestmike007

Coverage Status

Coverage increased (+0.0%) when pulling 27cf31cd441ee3fb83c811a52df4858dc531fd06 on bestmike007:master into 6a483ea44f496d4e98f5698590be59941b58fe9b on wr0ngway:master.

coveralls avatar Dec 05 '14 10:12 coveralls

Coverage Status

Coverage increased (+0.0%) when pulling 13191a375b12bac593a4dc6dd06bb61061794726 on bestmike007:master into 6a483ea44f496d4e98f5698590be59941b58fe9b on wr0ngway:master.

coveralls avatar Dec 08 '14 03:12 coveralls

Thank you for pointing this out. Placing the monkey fix in Lumber.init is actually wrong. The main issue that causes the error is that Lumber has replaced Rails.logger with a Log4r::Logger instance, which does not provide an instance method called formatter (well, formatter is not a Logger thing, it belongs to outputter.), and rails Rack server use the formatter defined by Rails.logger to send output to stdout. See the source

I've moved the monkey patch to Lumber::Railtie and committed a patch. And also, instead of overriding the instance method for Log4r::Logger, I use Rails.logger.extend to patch the instance Rails.logger only.

bestmike007 avatar Dec 11 '14 03:12 bestmike007

BTW. Log4r on master branch has already added a formatter method to Log4r::Logger (view source), which means that we do not need to define the formatter method since Log4r 1.1.11 (not published to rubygems yet for several unsolved issues). However, Log4r on master branch does not work with rails (using lumber) either. This is how formatter method is defined:

    def formatter
      stderr_outputter = nil
      @outputters.each do |outputter|
        if outputter.class == StderrOutputter
          stderr_outputter = outputter
          break
        end
      end
      if stderr_outputter.nil?
        raise 'unable to locate stderr outputter'
      else
        return stderr_outputter.formatter
      end
    end

There's no StderrOutputter among Rails.logger's outputters by default. So the patch could possibly be:

        if Rails.env == 'development' && Rails.logger.instance_of?(Log4r::Logger)
          if Rails.logger.respond_to? :formatter # latest Log4r ~> 1.1.11
            if Rails.logger.outputters.find{|o|o.class == Log4r::StderrOutputter}.nil?
              outputter = Log4r::StderrOutputter.new "rack_patch"
              outputter.level = Log4r::LNAMES.count - 1
              outputter.extend Module.new {
                def formatter
                  Rails.logger.remove self
                  nil
                end
              }
              Rails.logger.add outputter
            end
          else
            Rails.logger.extend Module.new { def formatter; nil; end }
          end
        end

bestmike007 avatar Dec 11 '14 04:12 bestmike007

Coverage Status

Coverage remained the same when pulling 96f474eacc4a45c952cdbec2d58cc3549c5b137e on bestmike007:master into 6a483ea44f496d4e98f5698590be59941b58fe9b on wr0ngway:master.

coveralls avatar Dec 11 '14 10:12 coveralls

Its also worth mentioning that another fix is to simply define a new outputter of type StderrOutputter and then make sure the 'rails' logger includes that outputter. You can use the 'level: ERROR' filter in that outputter so that debug/info logs also don't end up being printed to stderr. This solved the issue for me.

masha256 avatar Aug 16 '15 05:08 masha256