Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wrong status code 500 logged #45

Open
thedarkside opened this issue Jan 29, 2018 · 3 comments
Open

wrong status code 500 logged #45

thedarkside opened this issue Jan 29, 2018 · 3 comments

Comments

@thedarkside
Copy link

I am using the newest gem versions. It looks like the logger is executed before the rescue_from which sets the statuscode.

  logger.formatter = GrapeLogging::Formatters::Default.new
  use GrapeLogging::Middleware::RequestLogger, logger: logger, include: [ GrapeLogging::Loggers::Response.new,
                                                                               GrapeLogging::Loggers::FilterParameters.new,
                                                                               GrapeLogging::Loggers::ClientEnv.new,
                                                                               GrapeLogging::Loggers::RequestHeaders.new ]

  rescue_from WineBouncer::Errors::OAuthUnauthorizedError do |e|
    error! e, 401
  end
@thedarkside
Copy link
Author

Simple fix:

  insert_before Grape::Middleware::Error, GrapeLogging::Middleware::RequestLogger, logger: logger, include: [ GrapeLogging::Loggers::Response.new,
                                                                               GrapeLogging::Loggers::FilterParameters.new,
                                                                               GrapeLogging::Loggers::ClientEnv.new,
                                                                               GrapeLogging::Loggers::RequestHeaders.new ]

Instead of use ... just insert_before Grape::Middleware::Error,... and the exceptions hit the logger after the rescue_from handlers. Now the logger logs the right status code.

I am using grape v1.0.2

Maybe update the readme?

@aserafin
Copy link
Owner

aserafin commented Feb 1, 2018

@thedarkside thanks for the report! could you please create PR for the readme?

filipalacerda pushed a commit to gitlabhq/gitlabhq that referenced this issue May 18, 2018
As described in aserafin/grape_logging#45, if
a Grape error is caught by the handlers and a different return code
is returned, then the api_json.log would have a 500 error code
instead of the right value. Inserting the GrapeLogging middleware
after the Grape middleware fixes this problem.

Seen in https://gitlab.com/gitlab-com/infrastructure/issues/4249
@thedarkside
Copy link
Author

Seems already partially done with #74

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants