Merge pull request #2580 from ericproulx/revisit_endpoint_helpers_and_new

Revisit endpoint helpers and Grape::Middleware::Error helpers

Author: dblock

CHANGELOG.md

@@ -6,6 +6,7 @@
 * [#2573](https://github.com/ruby-grape/grape/pull/2573): Clean up deprecated code - [@ericproulx](https://github.com/ericproulx).
 * [#2575](https://github.com/ruby-grape/grape/pull/2575): Refactor Api description class - [@ericproulx](https://github.com/ericproulx).
 * [#2577](https://github.com/ruby-grape/grape/pull/2577): Deprecate `return` in endpoint execution - [@ericproulx](https://github.com/ericproulx).
+* [#13](https://github.com/ericproulx/grape/pull/13): Refactor endpoint helpers and error middleware integration - [@ericproulx](https://github.com/ericproulx).
 * Your contribution here.
 
 #### Fixes

lib/grape/endpoint.rb

@@ -11,17 +11,12 @@ class Endpoint
     include Grape::DSL::Headers
     include Grape::DSL::InsideRoute
 
-    attr_accessor :block, :source, :options
-    attr_reader :env, :request
+    attr_reader :env, :request, :source, :options
 
     def_delegators :request, :params, :headers, :cookies
     def_delegator :cookies, :response_cookies
 
     class << self
-      def new(...)
-        self == Endpoint ? Class.new(Endpoint).new(...) : super
-      end
-
       def before_each(new_setup = false, &block)
         @before_each ||= []
         if new_setup == false
@@ -82,17 +77,19 @@ def initialize(new_settings, options = {}, &block)
       @stream = nil
       @body = nil
 
-      return unless block
-
-      @source = block
-      @block = lambda do |endpoint_instance|
-        ActiveSupport::Notifications.instrument('endpoint_render.grape', endpoint: endpoint_instance) do
-          endpoint_instance.instance_exec(&block)
-        rescue LocalJumpError => e
-          Grape.deprecator.warn 'Using `return` in an endpoint has been deprecated.'
-          return e.exit_value
+      if block
+        @source = block
+        @block = lambda do |endpoint_instance|
+          ActiveSupport::Notifications.instrument('endpoint_render.grape', endpoint: endpoint_instance) do
+            endpoint_instance.instance_exec(&block)
+          rescue LocalJumpError => e
+            Grape.deprecator.warn 'Using `return` in an endpoint has been deprecated.'
+            return e.exit_value
+          end
         end
       end
+
+      @helpers = build_helpers
     end
 
     # Update our settings from a given set of stackable parameters. Used when
@@ -194,6 +191,8 @@ def call(env)
     def call!(env)
       env[Grape::Env::API_ENDPOINT] = self
       @env = env
+      # this adds the helpers only to the instance
+      singleton_class.include(@helpers) if @helpers
       @app.call(env)
     end
 
@@ -259,19 +258,13 @@ def execute
       @block&.call(self)
     end
 
-    def helpers
-      lazy_initialize! && @helpers
-    end
-
     def lazy_initialize!
       return true if @lazy_initialized
 
       @lazy_initialize_lock.synchronize do
         return true if @lazy_initialized
 
-        @helpers = build_helpers&.tap { |mod| self.class.include mod }
-        @app = options[:app] || build_stack(@helpers)
-
+        @app = options[:app] || build_stack
         @lazy_initialized = true
       end
     end
@@ -323,16 +316,15 @@ def options?
 
     private
 
-    def build_stack(helpers)
+    def build_stack
       stack = Grape::Middleware::Stack.new
 
       content_types = namespace_stackable_with_hash(:content_types)
       format = namespace_inheritable(:format)
 
       stack.use Rack::Head
       stack.use Rack::Lint if lint?
-      stack.use Class.new(Grape::Middleware::Error),
-                helpers: helpers,
+      stack.use Grape::Middleware::Error,
                 format: format,
                 content_types: content_types,
                 default_status: namespace_inheritable(:default_error_status),

lib/grape/middleware/error.rb

@@ -16,11 +16,6 @@ class Error < Base
         }.freeze
       }.freeze
 
-      def initialize(app, **options)
-        super
-        self.class.include(options[:helpers]) if options[:helpers]
-      end
-
       def call!(env)
         @env = env
         error_response(catch(:error) { return @app.call(@env) })
@@ -103,12 +98,7 @@ def rescue_handler_for_any_class(klass)
       end
 
       def run_rescue_handler(handler, error, endpoint)
-        if handler.instance_of?(Symbol)
-          raise NoMethodError, "undefined method '#{handler}'" unless respond_to?(handler)
-
-          handler = public_method(handler)
-        end
-
+        handler = endpoint.public_method(handler) if handler.instance_of?(Symbol)
         response = catch(:error) do
           handler.arity.zero? ? endpoint.instance_exec(&handler) : endpoint.instance_exec(error, &handler)
         end

spec/grape/api_spec.rb

@@ -2369,7 +2369,7 @@ def rescue_no_method_error
       subject.rescue_from :all, with: :not_exist_method
       subject.get('/rescue_method') { raise StandardError }
 
-      expect { get '/rescue_method' }.to raise_error(NoMethodError, /^undefined method 'not_exist_method'/)
+      expect { get '/rescue_method' }.to raise_error(NameError, /^undefined method [`']not_exist_method'/)
     end
 
     it 'correctly chooses exception handler if :all handler is specified' do

spec/grape/endpoint_spec.rb

@@ -735,18 +735,9 @@ def handle_argument_error
 
   describe 'NameError' do
     context 'when referencing an undefined local variable or method' do
-      let(:error_message) do
-        if Gem::Version.new(RUBY_VERSION).release <= Gem::Version.new('3.2')
-          %r{undefined local variable or method `undefined_helper' for #<Class:0x[0-9a-fA-F]+> in '/hey' endpoint}
-        else
-          opening_quote = Gem::Version.new(RUBY_VERSION).release >= Gem::Version.new('3.4') ? "'" : '`'
-          /undefined local variable or method #{opening_quote}undefined_helper' for/
-        end
-      end
-
       it 'raises NameError but stripping the internals of the Grape::Endpoint class and including the API route' do
         subject.get('/hey') { undefined_helper }
-        expect { get '/hey' }.to raise_error(NameError, error_message)
+        expect { get '/hey' }.to raise_error(NameError, /^undefined local variable or method ['`]undefined_helper' for/)
       end
     end
   end