kemalcr / kemal

Fast, Effective, Simple Web Framework
https://kemalcr.com
MIT License
3.62k stars 188 forks source link

Fix multiple logger handlers when custom logger is used #653

Closed aravindavk closed 1 year ago

aravindavk commented 1 year ago

When logger function is used, it adds logger handler after the static file handler. This causes 404 errors for static files. Default logger is also added this makes two logger handlers per every request.

Reproducer:

require "kemal"

public_folder "./public"

get "/hello" do |env|
  "Hello!"
end

class MyCustomLogger < Kemal::BaseLogHandler
  def call(context)
    puts "I'm logging some custom stuff here."
    call_next(context) # => This calls the next handler
  end

  # This is used from `log` method.
  def write(message)
    STDERR.puts message # => Logs the output to STDERR
  end
end

# This works
# Kemal.config.logger = MyCustomLogger.new

# This fails while serving static files
logger MyCustomLogger.new

Kemal.run

With this PR, logger function only updates the Kemal.config and Kemal.run will take care of adding logger handler.

This PR also fixes the warning when tried to run the example code for adding custom logger.

 10 | def call(env)
               ^--
Warning: positional parameter 'env' corresponds to
parameter 'context' of the overridden method
Kemal::BaseLogHandler#call(context : HTTP::Server::Context),
which has a different name and may affect named argument passing

Signed-off-by: Aravinda Vishwanathapura mail@aravindavk.in

Description of the Change

Alternate Designs

Benefits

Possible Drawbacks

aravindavk commented 1 year ago

CI failures(fmt and lint issues) are not related to this PR. Please look into this.