Customized Errors No Longer Including CORS Related Headers Upgrading from 1.0 to 1.3 #2078
Comments
|
Here is an example of what's happening in my specs. All of my specs that return a 200 response have the correct headers. For example this spec passes: it 'returns a 200 with the [redacted] if the user is [redacted]' do
#...
get_as_user path
expect(response.status).to eq(200)
body = JSON.parse(response.body)
expect(body.length).to eq 2
result_ids = [body[0]['uuid'], body[1]['uuid']]
expect(result_ids.include?(first_event.uuid)).to eq true
expect(result_ids.include?(second_event.uuid)).to eq true
expect(response.headers['Content-Type']).to match('json')
expect(response.headers['Access-Control-Allow-Origin']).to eq('*')
expect(response.headers['Access-Control-Request-Method']).to eq('*')
endHowever, any spec expecting an error gets the right content-type, error code, etc.. but the additional CORS headers are no longer present: it 'returns a 404 if the user does not have permission' do
get_as_user path
expect(response.status).to eq(404)
expect(response.headers['Content-Type']).to match('json')
expect(response.headers['Access-Control-Allow-Origin']).to eq('*')
expect(response.headers['Access-Control-Request-Method']).to eq('*')
end |
|
At the very least I would say that the regression is not expected. Do you think you could turn this spec into one in the Grape project that was succeeding < 1.3.3? |
|
Sure. Is there a specific set of specs I should look at when writing one for the library to test this? Let me know and I will work on a PR. |
|
@dblock you know it looks like this must just be an issue with 1.3.3. I upgraded to 1.4 by installing directly from master and my specs pass when I use the headers option on rescue_from :all do |error|
error! error.message, FilmCal::Base.status_code_for(error, error.class.to_s), {
'Access-Control-Allow-Origin' => '*',
'Access-Control-Request-Method' => '*'
}
endMy work around returning a rack response directly still returns an "Invalid Response" error but that is not a big deal as it was a work around anyway. |
|
I am a little worried that we introduced a regression, then fixed it, but still don't have specs for this. I don't see anything in CHANGELOG that says something like this should be fixed. Do appreciate if you could write a set of specs for this behavior, add a new file in spec/grape/api and we can see if we already have something similar later. Once you have a failing spec on 1.3.3 try bisecting it down to a fix in 1.4.0. |
Steps to reproduce
What we need to do to see your problem or bug?
I've upgraded Grape from 1.0 to 1.3.3 and can no longer add
Access-Control-Allow-OriginandAccess-Control-Request-Methodto the errors thrown by grape. I am using rescue_from :all to generate the desired status code for errors thrown by active record validations and other causes via this method as well.The errors throw with the correct status code in my specs BUT the customized headers are not being set.
The more detailed the issue, the more likely that we will fix it ASAP.
To get around the issue earlier on Grape 1.0 -- I returned a custom Rack::Response but AFTER upgrading to 1.3.3 this resulted in all of my specs receiving a 500 error with the message "Invalid Response":
So my thought was maybe this work around was no longer needed. However, I cannot get the standard
error!method to honor the customized headers. Without them my client does not receive the errors returned by the server as they are no longer CORS.Expected behavior
Tell us what should happen
The error response should have 'Access-Control-Allow-Origin' = '' and 'Access-Control-Request-Method' = ''.
Actual behavior
Tell us what happens instead
The headers were not present in the response with or without the explicit customization in the call to
error!. All other responses (successfull responses) are receiving the custom headers via:It's only the errors that are not.
System configuration
You can help us to understand your problem if you will share some very
useful information about your project environment (don't forget to
remove any confidential data if it exists).
config.ru
Ruby version: ``
ruby '2.7.1'
Gemfile.lock:
Gemfile.lock content
The text was updated successfully, but these errors were encountered: