Rack::CommonLogger: docs are unclear what "logger" is supposed to be #412

Closed
mfn opened this Issue Jul 20, 2012 · 5 comments

Comments

Projects
None yet
6 participants
@mfn

mfn commented Jul 20, 2012

I think the docs of http://rack.rubyforge.org/doc/classes/Rack/CommonLogger.html are a bit unclear what logger is supposed to be.

It's called logger everywhere, however it can't actually be a plain ruby stdlib Logger class because the object is expected to respond to a #write method ( see https://github.com/rack/rack/blob/master/lib/rack/commonlogger.rb#L33 ).

Since the fallback of the logger is env['rack.errors'], which is per http://rack.rubyforge.org/doc/files/SPEC.html specified as "Error Stream ... must respond to puts, write and flush" I think this fact or a direct reference to the rack spec would help discover the nature of how this is supposed to work.

@rkh rkh closed this in 1d70ecd Oct 25, 2012

raggi added a commit that referenced this issue Jan 4, 2013

@kenichi

This comment has been minimized.

Show comment Hide comment
@kenichi

kenichi Feb 27, 2013

this is perhaps even more confusing now, since it explicitly says:

  # +logger+ can be any class, including the standard library Logger, and is
  # expected to have a +write+ method, which accepts the CommonLogger::FORMAT.

...but stdlib Logger does not have a #write method?

kenichi commented Feb 27, 2013

this is perhaps even more confusing now, since it explicitly says:

  # +logger+ can be any class, including the standard library Logger, and is
  # expected to have a +write+ method, which accepts the CommonLogger::FORMAT.

...but stdlib Logger does not have a #write method?

@bkimble

This comment has been minimized.

Show comment Hide comment
@bkimble

bkimble Mar 12, 2013

Agree with @kenichi , it seems like to get it to work, you need to do something like:

logger = Logger.new(Rails.root.join("log/common.log"))
logger.instance_eval do
  def write(msg); self.send(:<<, msg); end
end
config.middleware.use Rack::CommonLogger, logger

Or use alias method on the class, though I don't like modifying all instances of the logger just to get around this issue.

bkimble commented Mar 12, 2013

Agree with @kenichi , it seems like to get it to work, you need to do something like:

logger = Logger.new(Rails.root.join("log/common.log"))
logger.instance_eval do
  def write(msg); self.send(:<<, msg); end
end
config.middleware.use Rack::CommonLogger, logger

Or use alias method on the class, though I don't like modifying all instances of the logger just to get around this issue.

@kenichi

This comment has been minimized.

Show comment Hide comment
@kenichi

kenichi Mar 12, 2013

@bkimble here's yet another approach, but it seems smelly and i don't like how it bypasses level settings...

https://github.com/kenichi/rack/compare/stdlib_logger

thoughts?

kenichi commented Mar 12, 2013

@bkimble here's yet another approach, but it seems smelly and i don't like how it bypasses level settings...

https://github.com/kenichi/rack/compare/stdlib_logger

thoughts?

@urielka

This comment has been minimized.

Show comment Hide comment
@urielka

urielka Jul 29, 2013

Contributor

@kenichi @bkimble I agree with you it doesn't support standard logger .
I think it used to support std logger when it used << instead of write ( in commit 761c624)
Since stdlib logger supports <<.

@rkh I think this bug is open and very alive - CommonLogger just doesn't support stdlib logger - it only supports a File/IO.
And in the documentation it says it does and that is very confusing....

Contributor

urielka commented Jul 29, 2013

@kenichi @bkimble I agree with you it doesn't support standard logger .
I think it used to support std logger when it used << instead of write ( in commit 761c624)
Since stdlib logger supports <<.

@rkh I think this bug is open and very alive - CommonLogger just doesn't support stdlib logger - it only supports a File/IO.
And in the documentation it says it does and that is very confusing....

@rkh rkh reopened this Jul 29, 2013

urielka added a commit to urielka/rack that referenced this issue Aug 12, 2013

urielka added a commit to urielka/rack that referenced this issue Aug 19, 2013

urielka added a commit to urielka/rack that referenced this issue Dec 7, 2013

urielka added a commit to urielka/rack that referenced this issue Dec 7, 2013

raggi added a commit that referenced this issue Dec 8, 2013

@raggi raggi added this to the Rack 1.5.3 milestone Jul 12, 2014

@raggi

This comment has been minimized.

Show comment Hide comment
@raggi

raggi Jul 13, 2014

Owner

This was fixed sometime ago. The docs now read responds to write or <<, and either is used.

Owner

raggi commented Jul 13, 2014

This was fixed sometime ago. The docs now read responds to write or <<, and either is used.

@raggi raggi closed this Jul 13, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment