search

aserafin / grape_logging

Request logging for Grape!
MIT License
147 stars 76 forks source link

Log fail status code #29

Closed guavabot closed 7 years ago

guavabot commented 8 years ago

Is there a way to log the status code for a fail? So instead of getting :status=>"fail" I would like to see :status=>405 or whatever.

aserafin commented 8 years ago

Thanks for the report!

As you can see here fail status is only displayed when we don't have response object. Can you maybe provide quick sample project or describe in details when this happens to you?

jnardone commented 8 years ago

This happens on any non-success from grape -- any 400-level error. https://github.com/ridiculous/grape-middleware-logger handles this correctly but i haven't yet figured out how to reconcile the two implementations.

guizmaii commented 8 years ago

My bad. I wasn't able the access the grappe error in the middleware, so I wasn't able to know which error happened, so as an ugly patch I put fail. I talk about that problem in this grappe issue: https://github.com/ruby-grape/grape/issues/1301

I do not have the time to work on that for now. Look at @ridiculous work, it's maybe better.

ridiculous commented 8 years ago

Yeah, the grape-middleware-logger handles that by wrapping the entire request: logging when the request comes in and when it finishes. This means it has access to the response object here and is able to get the status.

This also means requests can look "split" in the log when running multiple instances in production. I should add the ability to buffer and flush the log, but not one's asked for it and I don't even use Grape anymore...

stanhu commented 7 years ago

@strnadj We at GitLab starting use grape-logging and noticed the same issue here. I noticed you added https://github.com/Strnadj/grape_logging/commit/f94a56f2940826cfe020c8bdf7df9a950bdb9e71 to your fork. Would it be possible to submit that as a PR to this repository?

aserafin commented 7 years ago

@stanhu I just merged this PR, fixed tests and also added the datetime to lograge formatter. The 1.7.0 version is pushed to rubygems.

@Strnadj - thank you very much for fixing this issue!

stanhu commented 7 years ago

@aserafin @Strnadj Thanks so much!