aserafin / grape_logging

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

Make sure response status is always available #10

Closed terceiro closed 7 years ago

terceiro commented 9 years ago

Sometimes response is not what you think it is. @app_response is always a proper Rack::Response object so it's safe to get its status.

See https://github.com/intridea/grape/issues/1059

fidelisrafael commented 9 years ago

:+1:

sebastiansier commented 9 years ago

:+1:

grozen commented 9 years ago

This actually failed for me (specifically, for requests that fail due to wrong basic http authorization credentials). @app_response, in that case, was not a Rack::Response at all but instead was the array that response expects.

I ended up doing: status: @app_response.try(:status) || response.status in a middleware I have, and that seems to both work and be accurate.

Also, I have to admit I didn't fully understand whether or not this problem will go away once this fix is available in the actual grape gem.

terceiro commented 9 years ago

the grape fix seems to be the correct one ...

guizmaii commented 8 years ago

Your fix does not work. For example if an exception is thrown, a HTTP 500 answer will be sended and the @app_response will be nil. I'm currently searching a solution for that problem and asked for help in the grape repo : https://github.com/ruby-grape/grape/issues/1301

aserafin commented 7 years ago

@terceiro I'm closing this - I believe #33 should solve the issue 👍