From 45bd7be7f20e5da0dc9b81f28baaeca6306dc852 Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Wed, 13 May 2026 22:34:55 +0200 Subject: [PATCH] Switch error_formatter to keyword arguments; expose status and headers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The error_formatter contract previously took five positional arguments — `(message, backtrace, options, env, original_exception)` — which made it impossible to thread additional context through to custom formatters without breaking every existing override. Switch to keyword arguments and add two fields that have been asked for in #2527: def call(message:, backtrace:, options:, env:, status:, headers:, original_exception:) `status:` makes JSON:API-style error bodies straightforward (the spec embeds the HTTP status code in the response). `headers:` lets formatters react to the response content-type or trace the marker headers set by `error!`. Both were previously only reachable via `env[Grape::Env::API_ENDPOINT].status` and friends. This is a contract break — existing custom formatters that re-declare `call` with the positional signature will need to migrate. See UPGRADING.md for the before/after. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 1 + README.md | 10 ++--- UPGRADING.md | 49 +++++++++++++++++++++++++ lib/grape/dsl/request_response.rb | 26 ++++++------- lib/grape/dsl/rescue_options.rb | 24 ++++++++++++ lib/grape/endpoint.rb | 2 +- lib/grape/error_formatter/base.rb | 15 ++++++-- lib/grape/middleware/error.rb | 48 ++++++++++++++---------- spec/grape/api_spec.rb | 19 +++++++++- spec/grape/dsl/request_response_spec.rb | 12 +++--- spec/grape/middleware/exception_spec.rb | 10 ++--- 11 files changed, 158 insertions(+), 58 deletions(-) create mode 100644 lib/grape/dsl/rescue_options.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 61e3e6eca..e5b9a268a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ * [#2702](https://github.com/ruby-grape/grape/pull/2702): Add `oneof:` option for `requires`/`optional` to accept a Hash parameter matching one of several variant schemas (resolves [#2385](https://github.com/ruby-grape/grape/issues/2385)) - [@ericproulx](https://github.com/ericproulx). * [#2715](https://github.com/ruby-grape/grape/pull/2715): Normalize `==` / `eql?` aliasing across value-like classes - [@ericproulx](https://github.com/ericproulx). * [#2710](https://github.com/ruby-grape/grape/pull/2710): Tidy up `Grape::DeclaredParamsHandler` - [@ericproulx](https://github.com/ericproulx). +* [#2712](https://github.com/ruby-grape/grape/pull/2712): Pass a `Grape::Exceptions::ErrorResponse` value object to `error_formatter#call` instead of separate kwargs - [@ericproulx](https://github.com/ericproulx). * [#2714](https://github.com/ruby-grape/grape/pull/2714): Drop unused `Grape::Middleware::Globals` and its `grape.request*` env constants - [@ericproulx](https://github.com/ericproulx). * [#2717](https://github.com/ruby-grape/grape/pull/2717): Convert `Grape::Exceptions::ErrorResponse` to a `Data` value object - [@ericproulx](https://github.com/ericproulx). * Your contribution here. diff --git a/README.md b/README.md index 1ba89ffcb..963764b9b 100644 --- a/README.md +++ b/README.md @@ -2739,12 +2739,12 @@ end The error format will match the request format. See "Content-Types" below. -Custom error formatters for existing and additional types can be defined with a proc. +Custom error formatters for existing and additional types can be defined with a proc. The formatter receives a `Grape::Exceptions::ErrorResponse` value object as `error:` plus three context kwargs — `env:`, `include_backtrace:`, `include_original_exception:`. Pull just the keys you need with `**` to ignore the rest: ```ruby class Twitter::API < Grape::API - error_formatter :txt, ->(message, backtrace, options, env, original_exception) { - "error: #{message} from #{backtrace}" + error_formatter :txt, ->(error:, **) { + "error #{error.status}: #{error.message} from #{error.backtrace}" } end ``` @@ -2753,8 +2753,8 @@ You can also use a module or class. ```ruby module CustomFormatter - def self.call(message, backtrace, options, env, original_exception) - { message: message, backtrace: backtrace } + def self.call(error:, **) + { status: error.status, message: error.message, backtrace: error.backtrace } end end diff --git a/UPGRADING.md b/UPGRADING.md index 9b3e644af..6b45573ac 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -25,6 +25,55 @@ end The original implementation is preserved in git history at [`6b4111b3:lib/grape/middleware/globals.rb`](https://github.com/ruby-grape/grape/blob/6b4111b3/lib/grape/middleware/globals.rb). +#### `error_formatter` now receives a `Grape::Exceptions::ErrorResponse` value object + +Custom error formatters now receive a frozen `Grape::Exceptions::ErrorResponse` as the `error:` keyword argument, alongside three request-time context kwargs. The new signature: + +```ruby +def call(error:, env: nil, include_backtrace: false, include_original_exception: false) +``` + +`error` is the same value object the middleware uses internally, with `status` / `message` / `headers` / `backtrace` / `original_exception` accessors. The two `include_*` booleans are forwarded from the matching `rescue_from` options (previously buried inside `options[:rescue_options]`). + +Existing positional formatters break and need to be updated: + +```ruby +# Before +error_formatter :txt, ->(message, backtrace, options, env, original_exception) { ... } + +module CustomFormatter + def self.call(message, backtrace, options, env, original_exception) + ... + end +end + +# After — pick fields off `error` +error_formatter :txt, ->(error:, **) { "[#{error.status}] #{error.message}" } + +module CustomFormatter + def self.call(error:, **) + { status: error.status, message: error.message, backtrace: error.backtrace } + end +end +``` + +Migration: + +| Old positional arg | New | +| --- | --- | +| `message` | `error.message` | +| `backtrace` | `error.backtrace` | +| `original_exception` | `error.original_exception` | +| `options[:rescue_options][:backtrace]` | `include_backtrace` (kwarg) | +| `options[:rescue_options][:original_exception]` | `include_original_exception` (kwarg) | +| `env` | `env` (kwarg, still passed) | +| HTTP status | `error.status` (newly exposed) | +| Response headers | `error.headers` (newly exposed) | + +The remaining middleware-options keys (`default_status`, `format`, `rescue_handlers`, …) were framework-internal and have never been part of the documented contract. + +The change resolves [#2527](https://github.com/ruby-grape/grape/issues/2527): the HTTP `status` and the response `headers` are now part of the formatter contract, so JSON:API–style error bodies (which embed the status code) and header-aware formatters can be written without reaching into `env[Grape::Env::API_ENDPOINT]`. + #### `Grape::Middleware::Base#options` is now frozen `@options` is frozen at the end of `Grape::Middleware::Base#initialize` (after `merge_default_options`). The hash is initialized once and treated as immutable for the lifetime of the middleware. Custom middleware that mutates `options[...]` at runtime will now raise `FrozenError`. diff --git a/lib/grape/dsl/request_response.rb b/lib/grape/dsl/request_response.rb index 33e6d255e..f4963e0a7 100644 --- a/lib/grape/dsl/request_response.rb +++ b/lib/grape/dsl/request_response.rb @@ -83,12 +83,15 @@ def default_error_status(new_status = nil) # @param [Array] exception_classes A list of classes that you want to rescue, or # the symbol :all to rescue from all exceptions. # @param [Block] block Execution block to handle the given exception. - # @param [Hash] options Options for the rescue usage. - # @option options [Boolean] :backtrace Include a backtrace in the rescue response. - # @option options [Boolean] :rescue_subclasses Also rescue subclasses of exception classes - # @param [Proc] handler Execution proc to handle the given exception as an - # alternative to passing a block. - def rescue_from(*args, with: nil, **options, &block) + # @param [Proc] with Execution proc to handle the given exception as an alternative + # to passing a block. + # @param [Boolean] rescue_subclasses Also rescue subclasses of exception classes; + # defaults to +true+. + # @param [Boolean] backtrace Include the rescued exception's backtrace in the + # rescue response body. + # @param [Boolean] original_exception Include +inspect+ of the rescued exception + # in the rescue response body. + def rescue_from(*args, with: nil, rescue_subclasses: true, backtrace: false, original_exception: false, &block) handler = extract_handler(args, with:, block:) if args.include?(:all) @@ -101,18 +104,11 @@ def rescue_from(*args, with: nil, **options, &block) elsif args.include?(:internal_grape_exceptions) inheritable_setting.namespace_inheritable[:internal_grape_exceptions_rescue_handler] = handler else - handler_type = - case options[:rescue_subclasses] - when nil, true - :rescue_handlers - else - :base_only_rescue_handlers - end - + handler_type = rescue_subclasses ? :rescue_handlers : :base_only_rescue_handlers inheritable_setting.namespace_reverse_stackable[handler_type] = args.to_h { |arg| [arg, handler] } end - inheritable_setting.namespace_stackable[:rescue_options] = options + inheritable_setting.namespace_stackable[:rescue_options] = RescueOptions.new(backtrace:, original_exception:) end # Allows you to specify a default representation entity for a diff --git a/lib/grape/dsl/rescue_options.rb b/lib/grape/dsl/rescue_options.rb new file mode 100644 index 000000000..f44a27569 --- /dev/null +++ b/lib/grape/dsl/rescue_options.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +module Grape + module DSL + # Immutable value object holding the response-shaping booleans accepted + # by +Grape::DSL::RequestResponse#rescue_from+. Stored on the + # inheritable settings as +namespace_stackable[:rescue_options]+ and + # delegated to by +Grape::Middleware::Error+ (which forwards + # +backtrace+/+original_exception+ to the formatter as + # +include_backtrace+/+include_original_exception+). + # + # Defaults are duplicated on +#initialize+ here and on +#rescue_from+'s + # signature on purpose: keeping them on both sides means each entry point + # is self-documenting without needing to import a shared constant — the + # DSL signature shows what a user sees in the IDE, and the Data object + # has working defaults when constructed directly (middleware + # `DEFAULT_OPTIONS`, spec fixtures, etc.). The two must stay in lockstep. + RescueOptions = Data.define(:backtrace, :original_exception) do + def initialize(backtrace: false, original_exception: false) + super + end + end + end +end diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb index f65f28b47..2daa7d326 100644 --- a/lib/grape/endpoint.rb +++ b/lib/grape/endpoint.rb @@ -340,7 +340,7 @@ def error_middleware_options(format, content_types) rescue_grape_exceptions: ns_inh[:rescue_grape_exceptions], default_error_formatter: ns_inh[:default_error_formatter], error_formatters: ns_stack.namespace_stackable_with_hash(:error_formatters), - rescue_options: ns_stack.namespace_stackable_with_hash(:rescue_options), + rescue_options: ns_stack.namespace_stackable[:rescue_options]&.last, rescue_handlers:, base_only_rescue_handlers: ns_stack.namespace_stackable_with_hash(:base_only_rescue_handlers), all_rescue_handler: ns_inh[:all_rescue_handler], diff --git a/lib/grape/error_formatter/base.rb b/lib/grape/error_formatter/base.rb index 050ea0603..643332841 100644 --- a/lib/grape/error_formatter/base.rb +++ b/lib/grape/error_formatter/base.rb @@ -4,11 +4,18 @@ module Grape module ErrorFormatter class Base class << self - def call(message, backtrace, options = {}, env = nil, original_exception = nil) - wrapped_message = wrap_message(present(message, env)) + # Custom error formatters override +call+. The +error+ is a frozen + # {Grape::Exceptions::ErrorResponse} carrying +status+/+message+/ + # +headers+/+backtrace+/+original_exception+. +env+ is the Rack env + # (needed by entity-presenter resolution). +include_backtrace+ and + # +include_original_exception+ are the request-time toggles set by + # +rescue_from+; the base implementation embeds the corresponding + # fields in the response body when they are true. + def call(error:, env: nil, include_backtrace: false, include_original_exception: false) + wrapped_message = wrap_message(present(error.message, env)) if wrapped_message.is_a?(Hash) - wrapped_message[:backtrace] = backtrace if backtrace.present? && options.dig(:rescue_options, :backtrace) - wrapped_message[:original_exception] = original_exception.inspect if original_exception && options.dig(:rescue_options, :original_exception) + wrapped_message[:backtrace] = error.backtrace if include_backtrace && error.backtrace.present? + wrapped_message[:original_exception] = error.original_exception.inspect if include_original_exception && error.original_exception end format_structured_message(wrapped_message) diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb index 8bcdb1207..89b050e29 100644 --- a/lib/grape/middleware/error.rb +++ b/lib/grape/middleware/error.rb @@ -3,6 +3,7 @@ module Grape module Middleware class Error < Base + extend Forwardable include PrecomputedContentTypes DEFAULT_OPTIONS = { @@ -18,16 +19,19 @@ class Error < Base rescue_all: false, rescue_grape_exceptions: false, rescue_handlers: nil, - rescue_options: { - backtrace: false, - original_exception: false - }.freeze + rescue_options: Grape::DSL::RescueOptions.new }.freeze attr_reader :all_rescue_handler, :base_only_rescue_handlers, :default_error_formatter, :default_message, :default_status, :error_formatters, :format, :grape_exceptions_rescue_handler, :internal_grape_exceptions_rescue_handler, - :rescue_all, :rescue_grape_exceptions, :rescue_handlers + :rescue_all, :rescue_grape_exceptions, :rescue_handlers, :rescue_options + + # +:backtrace+ / +:original_exception+ on the rescue options become + # +#include_backtrace+ / +#include_original_exception+ on the middleware, + # which is what the formatter call site reads. + def_delegator :rescue_options, :backtrace, :include_backtrace + def_delegator :rescue_options, :original_exception, :include_original_exception def initialize(app, **options) super @@ -43,6 +47,7 @@ def initialize(app, **options) @rescue_all = @options[:rescue_all] @rescue_grape_exceptions = @options[:rescue_grape_exceptions] @rescue_handlers = @options[:rescue_handlers] + @rescue_options = @options[:rescue_options] || Grape::DSL::RescueOptions.new end def call!(env) @@ -59,16 +64,16 @@ def rack_response(status, headers, message) Rack::Response.new(Array.wrap(message), Rack::Utils.status_code(status), Grape::Util::Header.new.merge(headers)) end - def format_message(message, backtrace, original_exception = nil) + def format_message(error) current_format = env[Grape::Env::API_FORMAT] || format formatter = Grape::ErrorFormatter.formatter_for(current_format, error_formatters, default_error_formatter) - return formatter.call(message, backtrace, options, env, original_exception) if formatter + return formatter.call(error:, env:, include_backtrace:, include_original_exception:) if formatter throw :error, Grape::Exceptions::ErrorResponse.new( status: 406, message: "The requested format '#{current_format}' is not supported.", - backtrace:, - original_exception: + backtrace: error.backtrace, + original_exception: error.original_exception ) end @@ -80,15 +85,17 @@ def find_handler(klass) end def error_response(error = nil) - payload = Grape::Exceptions::ErrorResponse.coerce(error) - - status = payload.status || options[:default_status] - env[Grape::Env::API_ENDPOINT].status(status) # error! may not have been called - message = payload.message || options[:default_message] + raw = Grape::Exceptions::ErrorResponse.coerce(error) headers = { Rack::CONTENT_TYPE => content_type } - headers.merge!(payload.headers) if payload.headers.is_a?(Hash) - backtrace = payload.backtrace || payload.original_exception&.backtrace || [] - rack_response(status, headers, format_message(message, backtrace, payload.original_exception)) + headers.merge!(raw.headers) if raw.headers.is_a?(Hash) + payload = raw.with( + status: raw.status || default_status, + message: raw.message || default_message, + headers:, + backtrace: raw.backtrace || raw.original_exception&.backtrace || [] + ) + env[Grape::Env::API_ENDPOINT].status(payload.status) # error! may not have been called + rack_response(payload.status, payload.headers, format_message(payload)) end def default_rescue_handler(exception) @@ -188,10 +195,11 @@ def framework_default(endpoint) def error!(message, status = default_status, headers = {}, backtrace = [], original_exception = nil) env[Grape::Env::API_ENDPOINT].status(status) # not error! inside route - rack_response( - status, headers.reverse_merge(Rack::CONTENT_TYPE => content_type), - format_message(message, backtrace, original_exception) + merged_headers = headers.reverse_merge(Rack::CONTENT_TYPE => content_type) + error = Grape::Exceptions::ErrorResponse.new( + status:, message:, headers: merged_headers, backtrace:, original_exception: ) + rack_response(status, merged_headers, format_message(error)) end def error?(response) diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index c844d3906..7f7194615 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -2511,8 +2511,8 @@ def rescue_all_errors context 'class' do let(:custom_error_formatter) do Class.new do - def self.call(message, _backtrace, _options, _env, _original_exception) - "message: #{message} @backtrace" + def self.call(error:, **) + "message: #{error.message} @backtrace" end end end @@ -2548,6 +2548,21 @@ def self.call(message, _backtrace, _options, _env, _original_exception) end end + context 'with status and headers exposed (issue 2527)' do + it 'passes the HTTP status and headers into a custom error formatter' do + subject.format :txt + subject.error_formatter :txt, ->(error:, **) { "[#{error.status}] #{error.message} (#{error.headers['x-marker']})" } + subject.rescue_from :all do + error!('boom', 418, 'x-marker' => 'hit') + end + subject.get('/exception') { raise 'rain!' } + + get '/exception' + expect(last_response.status).to eq(418) + expect(last_response.body).to eq('[418] boom (hit)') + end + end + it 'rescues all errors and return :json' do subject.rescue_from :all subject.format :json diff --git a/spec/grape/dsl/request_response_spec.rb b/spec/grape/dsl/request_response_spec.rb index b1d5f8687..0ee449e20 100644 --- a/spec/grape/dsl/request_response_spec.rb +++ b/spec/grape/dsl/request_response_spec.rb @@ -199,37 +199,39 @@ end describe 'list of exceptions is passed' do + let(:default_rescue_options) { [Grape::DSL::RescueOptions.new] } + it 'sets hash of exceptions as rescue handlers' do subject.rescue_from StandardError expect(subject.inheritable_setting.namespace_reverse_stackable[:rescue_handlers]).to eq([{ StandardError => nil }]) - expect(subject.inheritable_setting.namespace_stackable[:rescue_options]).to eq([{}]) + expect(subject.inheritable_setting.namespace_stackable[:rescue_options]).to eq(default_rescue_options) end it 'rescues only base handlers if rescue_subclasses: false option is passed' do subject.rescue_from StandardError, rescue_subclasses: false expect(subject.inheritable_setting.namespace_reverse_stackable[:base_only_rescue_handlers]).to eq([{ StandardError => nil }]) - expect(subject.inheritable_setting.namespace_stackable[:rescue_options]).to eq([{ rescue_subclasses: false }]) + expect(subject.inheritable_setting.namespace_stackable[:rescue_options]).to eq(default_rescue_options) end it 'sets given proc as rescue handler for each key in hash' do rescue_handler_proc = proc {} subject.rescue_from StandardError, rescue_handler_proc expect(subject.inheritable_setting.namespace_reverse_stackable[:rescue_handlers]).to eq([{ StandardError => rescue_handler_proc }]) - expect(subject.inheritable_setting.namespace_stackable[:rescue_options]).to eq([{}]) + expect(subject.inheritable_setting.namespace_stackable[:rescue_options]).to eq(default_rescue_options) end it 'sets given block as rescue handler for each key in hash' do rescue_handler_proc = proc {} subject.rescue_from StandardError, &rescue_handler_proc expect(subject.inheritable_setting.namespace_reverse_stackable[:rescue_handlers]).to eq([{ StandardError => rescue_handler_proc }]) - expect(subject.inheritable_setting.namespace_stackable[:rescue_options]).to eq([{}]) + expect(subject.inheritable_setting.namespace_stackable[:rescue_options]).to eq(default_rescue_options) end it 'sets a rescue handler declared through :with option for each key in hash' do with_block = -> { 'hello' } subject.rescue_from StandardError, with: with_block expect(subject.inheritable_setting.namespace_reverse_stackable[:rescue_handlers]).to eq([{ StandardError => with_block }]) - expect(subject.inheritable_setting.namespace_stackable[:rescue_options]).to eq([{}]) + expect(subject.inheritable_setting.namespace_stackable[:rescue_options]).to eq(default_rescue_options) end end end diff --git a/spec/grape/middleware/exception_spec.rb b/spec/grape/middleware/exception_spec.rb index 6a34bfe4a..4c675a7e8 100644 --- a/spec/grape/middleware/exception_spec.rb +++ b/spec/grape/middleware/exception_spec.rb @@ -190,9 +190,7 @@ def call(_env) rescue_all: true, format: :custom, error_formatters: { - custom: lambda do |message, _backtrace, _options, _env, _original_exception| - { custom_formatter: message }.inspect - end + custom: ->(error:, **) { { custom_formatter: error.message }.inspect } } } end @@ -231,7 +229,7 @@ def call(_env) { rescue_all: true, format: :json, - rescue_options: { backtrace: true, original_exception: true } + rescue_options: Grape::DSL::RescueOptions.new(backtrace: true, original_exception: true) } end @@ -247,7 +245,7 @@ def call(_env) { rescue_all: true, format: :xml, - rescue_options: { backtrace: true, original_exception: true } + rescue_options: Grape::DSL::RescueOptions.new(backtrace: true, original_exception: true) } end @@ -263,7 +261,7 @@ def call(_env) { rescue_all: true, format: :txt, - rescue_options: { backtrace: true, original_exception: true } + rescue_options: Grape::DSL::RescueOptions.new(backtrace: true, original_exception: true) } end