From 7805dd0076f2b21e2f861665c2774ef1e112190d Mon Sep 17 00:00:00 2001
From: namusyaka <namusyaka@gmail.com>
Date: Fri, 10 Jun 2016 12:36:50 +0900
Subject: [PATCH] Avoid polluting Grape::Middleware::Error (#1421)

---
 CHANGELOG.md                  |  1 +
 lib/grape/endpoint.rb         |  7 ++++---
 lib/grape/middleware/error.rb | 21 ++++++---------------
 spec/grape/api_spec.rb        | 30 ++++++++++++++++++++++++++++++
 4 files changed, 41 insertions(+), 18 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 32b7e0f52a..95778516bd 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -17,6 +17,7 @@
 * [#1384](https://github.com/ruby-grape/grape/pull/1384): Fix parameter validation with an empty optional nested `Array` - [@ipkes](https://github.com/ipkes).
 * [#1414](https://github.com/ruby-grape/grape/pull/1414): Fix multiple version definitions for path versioning - [@304](https://github.com/304).
 * [#1415](https://github.com/ruby-grape/grape/pull/1415): Fix `declared(params, include_parent_namespaces: false)` - [@304](https://github.com/304).
+* [#1421](https://github.com/ruby-grape/grape/pull/1421): Avoid polluting `Grape::Middleware::Error` - [@namusyaka](https://github.com/namusyaka).
 
 0.16.2 (4/12/2016)
 ==================
diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb
index 6de3364cba..18d7888c17 100644
--- a/lib/grape/endpoint.rb
+++ b/lib/grape/endpoint.rb
@@ -244,11 +244,12 @@ def run
       end
     end
 
-    def build_stack
+    def build_stack(helpers)
       stack = Grape::Middleware::Stack.new
 
       stack.use Rack::Head
-      stack.use Grape::Middleware::Error,
+      stack.use Class.new(Grape::Middleware::Error),
+                helpers: helpers,
                 format: namespace_inheritable(:format),
                 content_types: namespace_stackable_with_hash(:content_types),
                 default_status: namespace_inheritable(:default_error_status),
@@ -300,10 +301,10 @@ def lazy_initialize!
       @lazy_initialize_lock.synchronize do
         return true if @lazy_initialized
 
-        @app = options[:app] || build_stack
         @helpers = build_helpers.tap do |mod|
           self.class.send(:include, mod)
         end
+        @app = options[:app] || build_stack(@helpers)
 
         @lazy_initialized = true
       end
diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb
index 1e4176f053..5e0290627b 100644
--- a/lib/grape/middleware/error.rb
+++ b/lib/grape/middleware/error.rb
@@ -8,6 +8,7 @@ def default_options
           default_status: 500, # default status returned on error
           default_message: '',
           format: :txt,
+          helpers: nil,
           formatters: {},
           error_formatters: {},
           rescue_all: false, # true to rescue all exceptions
@@ -19,11 +20,14 @@ def default_options
         }
       end
 
+      def initialize(app, options = {})
+        super
+        self.class.send(:include, @options[:helpers]) if @options[:helpers]
+      end
+
       def call!(env)
         @env = env
 
-        inject_helpers!
-
         begin
           error_response(catch(:error) do
             return @app.call(@env)
@@ -67,19 +71,6 @@ def exec_handler(e, &handler)
         end
       end
 
-      def inject_helpers!
-        return if helpers_available?
-        endpoint = @env['api.endpoint']
-        self.class.instance_eval do
-          include endpoint.send(:helpers)
-        end if endpoint.is_a?(Grape::Endpoint)
-        @helpers = true
-      end
-
-      def helpers_available?
-        @helpers
-      end
-
       def error!(message, status = options[:default_status], headers = {}, backtrace = [])
         headers = headers.reverse_merge(Grape::Http::Headers::CONTENT_TYPE => content_type)
         rack_response(format_message(message, backtrace), status, headers)
diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb
index 1efd414871..e9a97eda73 100644
--- a/spec/grape/api_spec.rb
+++ b/spec/grape/api_spec.rb
@@ -1456,6 +1456,36 @@ def custom_error!(name)
       expect(last_response.body).to eq 'hello bob'
     end
 
+    context 'with multiple apis' do
+      let(:a) { Class.new(Grape::API) }
+      let(:b) { Class.new(Grape::API) }
+
+      before do
+        a.helpers do
+          def foo
+            error!('foo', 401)
+          end
+        end
+        a.rescue_from(:all) { foo }
+        a.get { raise 'boo' }
+        b.helpers do
+          def foo
+            error!('bar', 401)
+          end
+        end
+        b.rescue_from(:all) { foo }
+        b.get { raise 'boo' }
+      end
+
+      it 'avoids polluting global namespace' do
+        env = Rack::MockRequest.env_for('/')
+
+        expect(a.call(env)[2].body).to eq(['foo'])
+        expect(b.call(env)[2].body).to eq(['bar'])
+        expect(a.call(env)[2].body).to eq(['foo'])
+      end
+    end
+
     it 'rescues all errors if rescue_from :all is called' do
       subject.rescue_from :all
       subject.get '/exception' do