From c6b72d830d494f4b54bcb48ecb043d8bebccac2c Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Thu, 10 Jul 2025 21:51:06 +0200 Subject: [PATCH] Add CHANGELOG Fix spec --- CHANGELOG.md | 1 + lib/grape/endpoint.rb | 42 ++++++++++++++--------------------- lib/grape/middleware/error.rb | 12 +--------- spec/grape/api_spec.rb | 2 +- spec/grape/endpoint_spec.rb | 11 +-------- 5 files changed, 21 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 83c226ec2..25132a6c9 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb index 6af3bed53..d4910e79a 100644 --- a/lib/grape/endpoint.rb +++ b/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,7 +316,7 @@ def options? private - def build_stack(helpers) + def build_stack stack = Grape::Middleware::Stack.new content_types = namespace_stackable_with_hash(:content_types) @@ -331,8 +324,7 @@ def build_stack(helpers) 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), diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb index 77b66d6d2..4735ec4bb 100644 --- a/lib/grape/middleware/error.rb +++ b/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 diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index ad5515d05..e5cc2a51c 100644 --- a/spec/grape/api_spec.rb +++ b/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 diff --git a/spec/grape/endpoint_spec.rb b/spec/grape/endpoint_spec.rb index 29c332e8e..eecb9102f 100644 --- a/spec/grape/endpoint_spec.rb +++ b/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 # 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