Skip to content

Commit

Permalink
[#1719] Fix missing headers before error! call (#1869)
Browse files Browse the repository at this point in the history
  • Loading branch information
anaumov authored and dblock committed Mar 12, 2019
1 parent b79bd9c commit ab184d1
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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)
# @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
Original file line number Diff line number Diff line change
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

0 comments on commit ab184d1

Please sign in to comment.