Call #after of middleware on error

Add a test for the bevhavior of returning Rack response by after callback

Author: namusyaka

CHANGELOG.md

@@ -6,6 +6,7 @@
 * [#1227](https://github.com/ruby-grape/grape/pull/1227): Store `message_key` on Grape::Exceptions::Validation - [@stjhimy](https://github.com/sthimy).
 * [#1232](https://github.com/ruby-grape/grape/pull/1232): Helpers are now available inside `rescue_from` - [@namusyaka](https://github.com/namusyaka).
 * [#1237](https://github.com/ruby-grape/grape/pull/1237): Allow multiple parameters in `given`, which behaves as if the scopes were nested in the inputted order - [@ochagata](https://github.com/ochagata).
+* [#1238](https://github.com/ruby-grape/grape/pull/1238): Call `after` of middleware on error - [@namusyaka](https://github.com/namusyaka).
 * Your contribution here.
 
 #### Fixes

UPGRADING.md

@@ -3,6 +3,30 @@ Upgrading Grape
 
 ### Upgrading to >= 0.14.0
 
+#### Changes to behavior of `after` method of middleware on error
+
+The `after` method of the middleware is now called on error.
+The following code would work correctly.
+
+```ruby
+class CustomMiddleware < Grape::Middleware::Base
+  def after
+    puts "Yay!"
+  end
+end
+
+class API < Grape::API
+  use CustomMiddleware
+
+  get { raise StandardError }
+end
+
+mock = Rack::MockRequest.env_for(?/)
+API.new.call(mock) #=. Displaying the message correctly.
+```
+
+See [#1147](https://github.com/ruby-grape/grape/issues/1147) and [#1240](https://github.com/ruby-grape/grape/issues/1240) for discussion of the issues.
+
 #### Changes to availability of DSL methods in filters
 
 The `#declared` method of the route DSL is no longer available in the `before` filter.  Using `declared` in a `before` filter will now raise `Grape::DSL::InsideRoute::MethodNotYetAvailable`.

lib/grape/middleware/base.rb

@@ -22,8 +22,12 @@ def call(env)
       def call!(env)
         @env = env
         before
-        @app_response = @app.call(@env)
-        after || @app_response
+        begin
+          @app_response = @app.call(@env)
+        ensure
+          after_response = after
+        end
+        after_response || @app_response
       end
 
       # @abstract

lib/grape/middleware/formatter.rb

@@ -19,6 +19,7 @@ def before
       end
 
       def after
+        return unless @app_response
         status, headers, bodies = *@app_response
 
         if Rack::Utils::STATUS_WITH_NO_ENTITY_BODY.include?(status)

spec/grape/middleware/base_spec.rb

@@ -29,6 +29,25 @@
     after { subject.call!({}) }
   end
 
+  context 'callbacks on error' do
+    let(:blank_app) { ->(_) { fail StandardError } }
+
+    it 'calls #after' do
+      expect(subject).to receive(:after)
+      expect { subject.call({}) }.to raise_error(StandardError)
+    end
+  end
+
+  context 'after callback' do
+    before do
+      allow(subject).to receive(:after).and_return([200, {}, 'Hello from after callback'])
+    end
+
+    it 'overwrites application response' do
+      expect(subject.call!({}).last).to eq('Hello from after callback')
+    end
+  end
+
   it 'is able to access the response' do
     subject.call({})
     expect(subject.response).to be_kind_of(Rack::Response)