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

Invalid body parse error not rescued by handlers #906

Conversation

croeck
Copy link
Contributor

@croeck croeck commented Jan 21, 2015

Please see issue #904 for a more detailed description. The spec with content_type application/json fails and is not processed by rescue_from :all.

@croeck croeck force-pushed the invalid-body-parse-error-not-rescued-by-handlers branch 3 times, most recently from 4a2ddac to 6edfbea Compare January 28, 2015 10:07
@@ -81,6 +81,9 @@ def read_rack_input(body)
end
env['rack.request.form_input'] = env['rack.input']
end
rescue MultiJson::ParseError
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is specific to the JSON parser, so it should be in it, not here. Then you can raise a Grape::Exception::InvalidMessageBody error and re-rescue and just re-raise anything that is Grape::Exception::Base here.

You might want to look at other parsers while you're at it, maybe the XML one has a similar problem and can be fixed in the same way?

@dblock
Copy link
Member

dblock commented Jan 29, 2015

This is good work, see my comments.

@croeck croeck force-pushed the invalid-body-parse-error-not-rescued-by-handlers branch 2 times, most recently from ef1fd15 to f69ded9 Compare January 30, 2015 14:36
@croeck
Copy link
Contributor Author

croeck commented Jan 30, 2015

Found (and fixed) the same issue to appear when using the XML parser.

@@ -9,6 +9,7 @@ Next Release
* [#901](https://github.com/intridea/grape/pull/901): Fix: callbacks defined in a version block are only called for the routes defined in that block - [@kushkella](https://github.com/kushkella).
* [#886](https://github.com/intridea/grape/pull/886): Group of parameters made to require an explicit type of Hash or Array - [@jrichter1](https://github.com/jrichter1).
* [#912](https://github.com/intridea/grape/pull/912): Extended the `:using` feature for param documentation to `optional` fields - [@croeck](https://github.com/croeck).
* [#906](https://github.com/intridea/grape/pull/906): Fix: invalid body parse errors are not rescued by handlers [@croeck](https://github.com/croeck).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: this is missing a dash. before your github name.

@dblock
Copy link
Member

dblock commented Jan 30, 2015

See my minor comments. Will merge after, great work.

… error that translates into a customized bad request response (+spec)
@croeck croeck force-pushed the invalid-body-parse-error-not-rescued-by-handlers branch from f69ded9 to 3479ba0 Compare January 30, 2015 21:33
@croeck
Copy link
Contributor Author

croeck commented Jan 30, 2015

Ok, fixed your comments and the build just passed :)

dblock added a commit that referenced this pull request Jan 31, 2015
…ued-by-handlers

Invalid body parse error not rescued by handlers
@dblock dblock merged commit 6d8b803 into ruby-grape:master Jan 31, 2015
@dblock
Copy link
Member

dblock commented Jan 31, 2015

Merged, thanks.

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

Successfully merging this pull request may close these issues.

2 participants