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

[#1719] Fix missing headers before error! call #1869

Merged
merged 2 commits into from Mar 12, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -4,6 +4,7 @@

* Your contribution here.
* [#1864](https://github.com/ruby-grape/grape/pull/1864): Adds `finally` on the API - [@myxoh](https://github.com/myxoh).
* [#1869](https://github.com/ruby-grape/grape/pull/1869): Fix issue with empty headers after `error!` method call - [@anaumov](https://github.com/anaumov).

#### Fixes

Expand Down
8 changes: 7 additions & 1 deletion README.md
Expand Up @@ -1815,7 +1815,7 @@ You can set a response header with `header` inside an API.
header 'X-Robots-Tag', 'noindex'
```

When raising `error!`, pass additional headers as arguments.
When raising `error!`, pass additional headers as arguments. Additional headers will be merged with headers set before `error!` call.

```ruby
error! 'Unauthorized', 401, 'X-Error-Detail' => 'Invalid token.'
Expand Down Expand Up @@ -2155,6 +2155,12 @@ instead of a message.
error!({ error: 'unexpected error', detail: 'missing widget' }, 500)
```

You can set additional headers for the response. They will be merged with headers set before `error!` call.

```ruby
error!('Something went wrong', 500, 'X-Error-Detail' => 'Invalid token.')
```

You can present documented errors with a Grape entity using the the [grape-entity](https://github.com/ruby-grape/grape-entity) gem.

```ruby
Expand Down
32 changes: 32 additions & 0 deletions UPGRADING.md
@@ -1,6 +1,38 @@
Upgrading Grape
===============

### Upgrading to >= 1.2.4

#### Headers in `error!` call

Headers in `error!` will be merged with `headers` hash. If any header need to be cleared on `error!` call, make sure to move it to the `after` block.

```ruby
class SampleApi < Grape::API
before do
header 'X-Before-Header', 'before_call'
end

get 'ping' do
header 'X-App-Header', 'on_call'
error! :pong, 400, 'X-Error-Details' => 'Invalid token'
end
end
```
**Former behaviour**
```ruby
response.headers['X-Before-Header'] # => nil
response.headers['X-App-Header'] # => nil
response.headers['X-Error-Details'] # => Invalid token
```

**Current behaviour**
```ruby
response.headers['X-Before-Header'] # => 'before_call'
response.headers['X-App-Header'] # => 'on_call'
response.headers['X-Error-Details'] # => Invalid token
```

### Upgrading to >= 1.2.1

#### Obtaining the name of a mounted class
Expand Down
4 changes: 3 additions & 1 deletion lib/grape/dsl/inside_route.rb
Expand Up @@ -134,8 +134,10 @@ def version
#
# @param message [String] The message to display.
# @param status [Integer] the HTTP Status Code. Defaults to default_error_status, 500 if not set.
def error!(message, status = nil, headers = nil)
Copy link
Member

Choose a reason for hiding this comment

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

I know it wasn't there, but lets document additional_headers above?

# @param additional_headers [Hash] Addtional headers for the response.
def error!(message, status = nil, additional_headers = nil)
self.status(status || namespace_inheritable(:default_error_status))
headers = additional_headers.present? ? header.merge(additional_headers) : header
throw :error, message: message, status: self.status, headers: headers
end

Expand Down
30 changes: 30 additions & 0 deletions spec/grape/endpoint_spec.rb
Expand Up @@ -1089,6 +1089,36 @@ def app
expect(last_response.headers['X-Custom']).to eq('value')
end

it 'merges additional headers with headers set before call' do
subject.before do
header 'X-Before-Test', 'before-sample'
end

subject.get '/hey' do
header 'X-Test', 'test-sample'
error!({ 'dude' => 'rad' }, 403, 'X-Error' => 'error')
end

get '/hey.json'
expect(last_response.headers['X-Before-Test']).to eq('before-sample')
expect(last_response.headers['X-Test']).to eq('test-sample')
expect(last_response.headers['X-Error']).to eq('error')
end

it 'does not merges additional headers with headers set after call' do
subject.after do
header 'X-After-Test', 'after-sample'
end

subject.get '/hey' do
error!({ 'dude' => 'rad' }, 403, 'X-Error' => 'error')
end

get '/hey.json'
expect(last_response.headers['X-Error']).to eq('error')
expect(last_response.headers['X-After-Test']).to be_nil
end

it 'sets the status code for the endpoint' do
memoized_endpoint = nil

Expand Down