Merge pull request #1240 from namusyaka/call-after-block-on-error

Call #after of middleware on error

Author: dblock

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

README.md

@@ -73,6 +73,7 @@
 - [Before and After](#before-and-after)
 - [Anchoring](#anchoring)
 - [Using Custom Middleware](#using-custom-middleware)
+  - [Grape Middleware](#grape-middleware)
   - [Rails Middleware](#rails-middleware)
   - [Remote IP](#remote-ip)
 - [Writing Tests](#writing-tests)
@@ -2557,6 +2558,33 @@ part.
 
 ## Using Custom Middleware
 
+### Grape Middleware
+
+You can make a custom middleware by using `Grape::Middleware::Base`.
+It's inherited from some grape official middlewares in fact.
+
+For example, you can write a middleware to log application exception.
+
+```ruby
+class LoggingError < Grape::Middleware::Base
+  def after
+    if @api_response[0] == 500
+      env['rack.logger'].error("Raised error on #{env['PATH_INFO']}")
+    end
+  end
+end
+```
+
+Your middleware can overwrite application response as follows, except error case.
+
+```ruby
+class Overwriter < Grape::Middleware::Base
+  def after
+    [200, { 'Content-Type' => 'text/plain' }, ['Overwrited.']]
+  end
+end
+```
+
 ### Rails Middleware
 
 Note that when you're using Grape mounted on Rails you don't have to use Rails middleware because it's already included into your middleware stack.

UPGRADING.md

@@ -1,6 +1,25 @@
 Upgrading Grape
 ===============
 
+### Upgrading to >= 0.15.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 ErrorMiddleware < Grape::Middleware::Base
+  def after
+    if @api_response[0] == 500
+      env['rack.logger'].debug("Raised error on #{env['PATH_INFO']}")
+    end
+  end
+end
+```
+
+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.
+
 ### Upgrading to >= 0.14.0
 
 #### Changes to availability of DSL methods in filters

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)