Fixed issues related to invalid accept headers #913
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
croeck
force-pushed
the
invalid-accept-header-not-processed-by-handlers
branch
from
ff71366 to
f5ea8a0
Compare
| @@ -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). | |||
| * [#913](https://github.com/intridea/grape/pull/913): Fix: Invalid accept headers are (1) not processed by rescue handlers and (2) result in 500 when http_codes are defines - [@croeck](https://github.com/croeck). | |||
There was a problem hiding this comment.
Typo: defineD, right?
You can split this into two separate lines, to avoid the whole (1) (2) thing even if it means repeating #913.
|
Solid, nice work @croeck. Amend by my nitpicky comments and I'll merge. |
…dlers and (2) result in status 500 (no method) when http_codes are defined.
croeck
force-pushed
the
invalid-accept-header-not-processed-by-handlers
branch
from
f5ea8a0 to
0dea522
Compare
|
Changes are now included, thanks for your comments! |
|
Merged rebased via 046450e. Thank you. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixed that invalid accept headers are (1) not processed by rescue handlers and (2) result in status 500 (no method) when http_codes are defined. The fix is based on changing the former
throwto raising a new error, which then can be processed.To (1):
The actual behavior of cascading the request remains unchanged as the headers are included in the raised error. As long as the error is not rescued it results in the same error messages (404 or 406).
The error message that is generated consists of two parts: The problem
Invalid accept headerand what actually went wrong, eg.API vendor or version not found. The cause of the problem was included as the resolution of the problem, as it always describes what the user did wrong:Rack::Accept::MediaType.new env['HTTP_ACCEPT']failsTo (2):
As commented in the source code,
lib/grape/error_formatter/base.rbdoes raise an error if the failure originated from the middleware and not a route. All thethrowstatements that were used before have caused those errors if http_codes were defined in the route description.Please let me know if there are any issues with these changes.
Cedric