Skip to content

Commit 126f561

Browse files
committed
Merge pull request #1285 from gregormelhorn/after_callback_warning
After callback warning
2 parents 07f464a + 7455f65 commit 126f561

File tree

3 files changed

+26
-1
lines changed

3 files changed

+26
-1
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
* [#1252](https://github.com/ruby-grape/grape/pull/1252): Allow default to be a subset or equal to allowed values without raising IncompatibleOptionValues - [@jeradphelps](https://github.com/jeradphelps).
1212
* [#1255](https://github.com/ruby-grape/grape/pull/1255): Allow param type definition in `route_param` - [@namusyaka](https://github.com/namusyaka).
1313
* [#1257](https://github.com/ruby-grape/grape/pull/1257): Allow Proc, Symbol or String in `rescue_from with: ...` - [@namusyaka](https://github.com/namusyaka).
14+
* [#1285](https://github.com/ruby-grape/grape/pull/1285): Add a warning for errors appearing in `after` callbacks - [@gregormelhorn](https://github.com/gregormelhorn).
1415
* Your contribution here.
1516

1617
#### Fixes

lib/grape/middleware/base.rb

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,12 @@ def call!(env)
2929
begin
3030
@app_response = @app.call(@env)
3131
ensure
32-
after_response = after
32+
begin
33+
after_response = after
34+
rescue StandardError => e
35+
warn "caught error of type #{e.class} in after callback inside #{self.class.name} : #{e.message}"
36+
raise e
37+
end
3338
end
3439

3540
response = after_response || @app_response

spec/grape/middleware/base_spec.rb

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,25 @@
4848
end
4949
end
5050

51+
context 'after callback with errors' do
52+
it 'does not overwrite the application response' do
53+
expect(subject.call({})).to eq([200, {}, 'Hi there.'])
54+
end
55+
56+
context 'with patched warnings' do
57+
before do
58+
@warnings = warnings = []
59+
allow_any_instance_of(Grape::Middleware::Base).to receive(:warn) { |m| warnings << m }
60+
allow(subject).to receive(:after).and_raise(StandardError)
61+
end
62+
63+
it 'does show a warning' do
64+
expect { subject.call({}) }.to raise_error(StandardError)
65+
expect(@warnings).not_to be_empty
66+
end
67+
end
68+
end
69+
5170
it 'is able to access the response' do
5271
subject.call({})
5372
expect(subject.response).to be_kind_of(Rack::Response)

0 commit comments

Comments
 (0)