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

Fix 500 error for xml format when method is not allowed #1283

Merged
merged 1 commit into from
Feb 22, 2016

Conversation

304
Copy link
Contributor

@304 304 commented Feb 21, 2016

Problem:

All requests to Not Allowed methods are returning 500 error instead of
405 when the format is xml.

Cause:

Route definitions for not allowed methods are returning an empty string
as a response body. But the empty string fails to convert to xml and
raises InvalidFormatter error.

Solution:

Define an exception for "method is not allowed" case.
Return an error message "405 Not Allowed" instead of empty string.

@dblock
Copy link
Member

dblock commented Feb 22, 2016

This is much better. Can you squash these two and rebase please?

Problem:

All requests to Not Allowed methods are returning 500 error instead of
405 when the format is xml.

Cause:

Route definitions for not allowed methods are returning an empty string
as a response body. But the empty string fails to convert to xml and
raises InvalidFormatter error.

Solution:

Define an exception for "method is not allowed" case.
Return an error message "405 Not Allowed" instead of empty string.
@304 304 force-pushed the fix_405_error_for_xml_format branch from 5438812 to 27a14c9 Compare February 22, 2016 15:46
@304
Copy link
Contributor Author

304 commented Feb 22, 2016

@dblock, done.

dblock added a commit that referenced this pull request Feb 22, 2016
Fix 500 error for xml format when method is not allowed
@dblock dblock merged commit 07f464a into ruby-grape:master Feb 22, 2016
@dblock
Copy link
Member

dblock commented Feb 22, 2016

Merged thanks.

@dblock
Copy link
Member

dblock commented Mar 7, 2016

This introduced a behavior change that I am documenting in UPGRADING, because Grape::Exceptions::MethodNotAllowed is now raised the exception is being rescued in rescue_from :all and therefore needs to be handled explicitly. I think the following restores old behavior:

        rescue_from Grape::Exceptions::Base do |e|
          error! e.message, e.status, e.headers
        end

dblock added a commit that referenced this pull request Mar 7, 2016
@dblock
Copy link
Member

dblock commented Mar 7, 2016

252bfd2

przbadu pushed a commit to przbadu/grape that referenced this pull request Apr 5, 2016
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