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