From f0243cb9ee95d6e6e7a05b11a15d07c65fdf31b9 Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Thu, 1 Oct 2026 11:41:36 +0200 Subject: [PATCH] Raise from error! instead of throwing :error error! threw :error, and a throw is not an exception. Active Record commits a transaction that a throw leaves, so error! inside a transaction saved what the block had written while the client was told the request failed. A throw cannot leave the fiber it was thrown in either, so error! inside an Async task answered 500. error! now raises Grape::Exceptions::Halt, a StandardError carrying the same ErrorResponse. The error middleware takes it inside its catch, so it is rendered where a thrown error is and never reaches rescue_from. Its backtrace is preset empty, as Validation's is, so raise skips capturing one. Raising costs more than throwing, so ErrorResponse is now a plain frozen class rather than a Data. Every error response builds at least two of them, and Data's constructor takes its keywords as a Hash it then validates: about 450 ns each and 700 ns for the copy #with made, against about 120 ns. It answers only what Grape reads off it, its readers, and stays frozen; error_response builds its copy with new. Nothing compared, hashed, converted or pattern-matched one, so the rest of Data's interface is dropped, as UPGRADING says. Halt spells its keywords out for the same reason. With both, an error! response is at least as fast as before, and other error responses are faster. throw :error from middleware and rescue_from handlers keeps working. Fixes #2197 Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + README.md | 13 +++++- UPGRADING.md | 26 +++++++++++ lib/grape/dsl/inside_route.rb | 3 +- lib/grape/exceptions/error_response.rb | 30 +++++++++--- lib/grape/exceptions/halt.rb | 39 ++++++++++++++++ lib/grape/middleware/error.rb | 13 +++++- spec/grape/dsl/inside_route_spec.rb | 26 +++++------ spec/grape/endpoint_spec.rb | 48 ++++++++++++++++++++ spec/grape/exceptions/error_response_spec.rb | 27 ++--------- 10 files changed, 179 insertions(+), 47 deletions(-) create mode 100644 lib/grape/exceptions/halt.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c3612143..b3eb53bb8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -81,6 +81,7 @@ * [#3016](https://github.com/ruby-grape/grape/pull/3016): Stop copying the error middleware on requests that do not fail - [@ericproulx](https://github.com/ericproulx). * [#3017](https://github.com/ruby-grape/grape/pull/3017): Stop copying the header versioner per request for an Accept header it answers from its table - [@ericproulx](https://github.com/ericproulx). * [#3018](https://github.com/ruby-grape/grape/pull/3018): Stop copying the auth middleware per request - [@ericproulx](https://github.com/ericproulx). +* [#3023](https://github.com/ruby-grape/grape/pull/3023): Raise from `error!` instead of throwing `:error`, so a transaction it leaves rolls back - [@ericproulx](https://github.com/ericproulx). * Your contribution here. ### 4.0.1 (2026-09-15) diff --git a/README.md b/README.md index 062f9ad66..2a378f627 100644 --- a/README.md +++ b/README.md @@ -2731,6 +2731,17 @@ You can set additional headers for the response. They will be merged with header error!('Something went wrong', 500, 'X-Error-Detail' => 'Invalid token.') ``` +`error!` raises a `Grape::Exceptions::Halt`, which Grape renders as the response without handing it to `rescue_from`. Since it is an exception, a database transaction it leaves is rolled back. It also reaches Grape from a fiber or thread whose result the route waits for, such as an `Async` task. + +```ruby +Order.transaction do + order = Order.create!(declared(params)) + error!('Out of stock', 409) unless order.product.in_stock? # the order is rolled back +end +``` + +It is a `StandardError`, so a `rescue => e` around `error!` catches it as well. Rescue `Grape::Exceptions::Halt` first and re-raise it to let it through. + You can present documented errors with a Grape entity using the [grape-entity](https://github.com/ruby-grape/grape-entity) gem. ```ruby @@ -2878,7 +2889,7 @@ 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. 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: +Custom error formatters for existing and additional types can be defined with a proc. The formatter receives a frozen `Grape::Exceptions::ErrorResponse` as `error:` (read its `status`, `message`, `headers`, `backtrace` and `original_exception`), 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 diff --git a/UPGRADING.md b/UPGRADING.md index 407ff3e87..02050cd28 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -3,6 +3,32 @@ Upgrading Grape ### Upgrading to >= 4.1.0 +#### `error!` raises `Grape::Exceptions::Halt` instead of throwing `:error` + +`error!` used to `throw :error`, and a throw is not an exception. Active Record commits a transaction that a throw leaves, so `error!` inside `transaction` saved what the block had written while the client was told the request failed ([#2197](https://github.com/ruby-grape/grape/issues/2197)). A throw cannot leave the fiber or thread it was thrown in either, so `error!` inside an `Async` task answered 500 instead of its status. + +`error!` now raises `Grape::Exceptions::Halt` ([#3023](https://github.com/ruby-grape/grape/pull/3023)), a `StandardError` carrying the same response (`#response`, also `#status` and `#headers`). Grape renders it as it rendered the thrown error, without handing it to a `rescue_from` handler, `rescue_from :all` included. + +* A `rescue => e` or `rescue StandardError` around `error!` now catches it, whether in a route, a helper, a filter or a middleware added with `use`. Narrow the rescue, or let `Halt` through: + + ```ruby + rescue Grape::Exceptions::Halt + raise + rescue StandardError => e + # ... + ``` + +* Code that wraps `error!` in `catch(:error)`, a spec for instance, now gets the exception. Rescue `Grape::Exceptions::Halt` and read `#response`, the `Grape::Exceptions::ErrorResponse` that used to be thrown. +* An `ActiveSupport::Notifications` subscriber to `endpoint_run.grape`, `endpoint_render.grape` or `endpoint_run_filters.grape` now finds `:exception` and `:exception_object` in the payload when `error!` ends the request, as it does for any exception. + +`throw :error` from your own middleware or `rescue_from` handlers still works. + +#### `Grape::Exceptions::ErrorResponse` is no longer a `Data` + +Every error response builds at least two `ErrorResponse`s, and building a `Data` costs several times what building a plain object does. It is now a plain frozen class ([#3023](https://github.com/ruby-grape/grape/pull/3023)). It is still built with keywords, each optional, and still answers its readers. + +It no longer copies itself with `#with`, compares by value (`==`, `eql?`, `hash`), converts with `#to_h`, lists `members`, takes positional arguments or takes part in pattern matching. Build a new one with `ErrorResponse.new` instead of calling `#with`. To check a payload, read its fields, for instance with `have_attributes(status: 404, message: 'Not Found')` in a spec. + #### `Grape::Middleware::Versioner::Header` answers a common Accept header without `#before` `Middleware::Base#call` copied the header versioner for every request so that `#before` could keep the env and the Accept header it scrubs in instance variables. When the API is not strict and the Accept header is one the versioner worked out when it was built (a media type it declares, `*/*` or none), it now records the media type and calls the app from the one instance the stack built ([#3017](https://github.com/ruby-grape/grape/pull/3017)). Any other request still goes through `#before`. diff --git a/lib/grape/dsl/inside_route.rb b/lib/grape/dsl/inside_route.rb index d7a2c9653..e74952740 100644 --- a/lib/grape/dsl/inside_route.rb +++ b/lib/grape/dsl/inside_route.rb @@ -26,10 +26,11 @@ def configuration # @param additional_headers [Hash] Additional headers for the response. # @param backtrace [Array] The backtrace of the exception that caused the error. # @param original_exception [Exception] The original exception that caused the error. + # @raise [Grape::Exceptions::Halt] always, carrying the response. def error!(message, status = nil, additional_headers = nil, backtrace = nil, original_exception = nil) resolved_status = self.status(status || inheritable_setting.default_error_status) headers = additional_headers.present? ? header.merge(additional_headers) : header - throw :error, Grape::Exceptions::ErrorResponse.new( + raise Grape::Exceptions::Halt.new( message:, status: resolved_status, headers:, backtrace:, original_exception: ) end diff --git a/lib/grape/exceptions/error_response.rb b/lib/grape/exceptions/error_response.rb index cb835bdf5..d687f5b67 100644 --- a/lib/grape/exceptions/error_response.rb +++ b/lib/grape/exceptions/error_response.rb @@ -2,13 +2,29 @@ module Grape module Exceptions - # Value object representing the payload thrown via `throw :error, ...` - # and consumed by `Middleware::Error#error_response`. Replaces the - # implicit-schema Hash that previously circulated between throw sites - # and the error middleware. - ErrorResponse = Data.define(:status, :message, :headers, :backtrace, :original_exception) do + # The frozen payload thrown via `throw :error, ...` (or raised by `error!` + # inside a `Grape::Exceptions::Halt`) and consumed by + # `Middleware::Error#error_response`. Replaces the implicit-schema Hash + # that previously circulated between throw sites and the error middleware. + # + # A plain frozen class rather than a +Data+: every error response builds + # at least two (the payload, then the one +error_response+ fills the + # defaults into), and +Data+'s constructor takes its keywords as a Hash it + # then validates, about 450 ns apiece. A Ruby method keeps keyword + # arguments on the stack. It answers only what is read off it: its + # readers. + class ErrorResponse + MEMBERS = %i[status message headers backtrace original_exception].freeze + + attr_reader(*MEMBERS) + def initialize(status: nil, message: nil, headers: nil, backtrace: nil, original_exception: nil) - super + @status = status + @message = message + @headers = headers + @backtrace = backtrace + @original_exception = original_exception + freeze end def to_s @@ -38,7 +54,7 @@ def self.coerce(input) when Grape::Exceptions::Base from_exception(input) when Hash - new(**input.slice(:status, :message, :headers, :backtrace, :original_exception)) + new(**input.slice(*MEMBERS)) else new end diff --git a/lib/grape/exceptions/halt.rb b/lib/grape/exceptions/halt.rb new file mode 100644 index 000000000..980a84124 --- /dev/null +++ b/lib/grape/exceptions/halt.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +module Grape + module Exceptions + # Raised by +error!+ to end the request with an error response. It carries + # the ErrorResponse a +throw :error+ carries, and +Middleware::Error+ + # renders it the same way: as it is, without consulting +rescue_from+. + # + # It is raised rather than thrown so that it leaves a block the way an + # exception does. Active Record rolls a transaction back only when an + # exception leaves its block, and commits it on a +throw+. A +throw+ cannot + # leave the fiber or thread it was thrown in either, while +Async::Task#wait+ + # and +Thread#value+ re-raise an exception in the request's own. + # + # A StandardError on purpose: async treats any other exception leaving a + # task as fatal and stops its reactor. It is not a Grape::Exceptions::Base, + # so a +rescue+ of those around +error!+ still lets it through. + class Halt < StandardError + extend Forwardable + + EMPTY_BACKTRACE = [].freeze + + attr_reader :response + + def_delegators :response, :status, :headers + + # Keywords spelled out rather than forwarded with **, which would gather + # them into a Hash on every error! only to spread it again. + def initialize(status: nil, message: nil, headers: nil, backtrace: nil, original_exception: nil) + @response = ErrorResponse.new(status:, message:, headers:, backtrace:, original_exception:) + super(message) + # Pre-seed the backtrace so Ruby's raise skips capture, as + # Grape::Exceptions::Validation does: +error!+ is how a route answers + # with an error, and the backtrace would only point at that route. + set_backtrace(EMPTY_BACKTRACE) + end + end + end +end diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb index fe653d17c..9f6d3e906 100644 --- a/lib/grape/middleware/error.rb +++ b/lib/grape/middleware/error.rb @@ -109,12 +109,18 @@ def render_raised(env, exception) # the +catch+ block: a +return+ there leaves the method through Kernel#catch, # which unwinds like a throw and allocates for it, on every request that # did not fail. + # + # +error!+ raises a Halt carrying what a +throw :error+ carries. It is + # taken inside the +catch+, so it is rendered where a thrown error is and + # never reaches a +rescue_from+ handler, the +:all+ one included. def respond(env) answered = false response = catch(:error) do app_response = @app.call(env) answered = true app_response + rescue Grape::Exceptions::Halt => e + e.response end answered ? response : dup.render_thrown(env, response) rescue Exception => e # rubocop:disable Lint/RescueException @@ -169,11 +175,12 @@ def error_response(error = nil) raw = Grape::Exceptions::ErrorResponse.coerce(error) headers = { Rack::CONTENT_TYPE => content_type } headers.merge!(raw.headers) if raw.headers.is_a?(Hash) - payload = raw.with( + payload = Grape::Exceptions::ErrorResponse.new( status: raw.status || default_status, message: raw.message || default_message, headers:, - backtrace: resolved_backtrace(raw) + backtrace: resolved_backtrace(raw), + original_exception: raw.original_exception ) env[Grape::Env::API_ENDPOINT].status(payload.status) # error! may not have been called render_response(payload) @@ -330,6 +337,8 @@ def run_rescue_handler(handler, error, endpoint, redispatched: false) callable = handler.is_a?(Symbol) ? endpoint.public_method(handler) : handler response = catch(:error) do call_rescue_handler(callable, error, endpoint) + rescue Grape::Exceptions::Halt => e + e.response rescue StandardError => e return redispatch(e, endpoint, redispatched) end diff --git a/spec/grape/dsl/inside_route_spec.rb b/spec/grape/dsl/inside_route_spec.rb index 818726a0b..e8cbc3f16 100644 --- a/spec/grape/dsl/inside_route_spec.rb +++ b/spec/grape/dsl/inside_route_spec.rb @@ -37,33 +37,33 @@ def header(key = nil, val = nil) end describe '#error!' do - it 'throws :error' do - expect { subject.error! 'Not Found', 404 }.to throw_symbol(:error) + it 'raises a Grape::Exceptions::Halt carrying the response' do + expect { subject.error! 'Not Found', 404, 'X-Error' => 'detail' }.to raise_error(Grape::Exceptions::Halt) do |halt| + expect(halt.response).to have_attributes(status: 404, message: 'Not Found', headers: { 'X-Error' => 'detail' }) + end end - describe 'thrown' do - before do - catch(:error) { subject.error! 'Not Found', 404 } - end + # async hands a StandardError raised in a task back to the task waiting on + # it, and stops its reactor on any other exception. + it 'raises a StandardError' do + expect { subject.error! 'Not Found', 404 }.to raise_error(StandardError) + end - it 'sets status' do - expect(subject.status).to eq 404 - end + it 'sets status' do + expect { subject.error! 'Not Found', 404 }.to raise_error(Grape::Exceptions::Halt) + expect(subject.status).to eq 404 end describe 'default_error_status' do before do subject.inheritable_setting.default_error_status = 500 - catch(:error) { subject.error! 'Unknown' } end it 'sets status to default_error_status' do + expect { subject.error! 'Unknown' }.to raise_error(Grape::Exceptions::Halt) expect(subject.status).to eq 500 end end - - # self.status(status || settings[:default_error_status]) - # throw :error, message: message, status: self.status, headers: headers end describe '#redirect' do diff --git a/spec/grape/endpoint_spec.rb b/spec/grape/endpoint_spec.rb index 72e16c967..e412b558f 100644 --- a/spec/grape/endpoint_spec.rb +++ b/spec/grape/endpoint_spec.rb @@ -705,6 +705,54 @@ def self.represent(_object, options = {}) expect(last_response.body).to eq('{"version":"v1"}') end end + + # Shaped like Active Record's transaction: it rolls back when an exception + # leaves the block, and commits on any other exit, a throw included. + it 'leaves an enclosing block as an exception, so a transaction around it rolls back' do + outcome = nil + transaction = lambda do |&block| + block.call + rescue Exception # rubocop:disable Lint/RescueException + outcome = :rolled_back + raise + ensure + outcome ||= :committed + end + subject.get('/hey') { transaction.call { error!('out of stock', 409) } } + + get '/hey' + expect(last_response.status).to eq(409) + expect(last_response.body).to eq('out of stock') + expect(outcome).to eq(:rolled_back) + end + + # As from a task under async, whose #wait resumes the request's fiber with + # what the task raised. + it 'answers from a fiber the route resumes' do + subject.get('/hey') { Fiber.new { error!('out of stock', 409) }.resume } + + get '/hey' + expect(last_response.status).to eq(409) + expect(last_response.body).to eq('out of stock') + end + + it 'is not handed to a rescue_from :all handler' do + subject.rescue_from(:all) { error!('rescued', 500) } + subject.get('/hey') { error!('out of stock', 409) } + + get '/hey' + expect(last_response.status).to eq(409) + expect(last_response.body).to eq('out of stock') + end + + it 'is not handed to a rescue_from StandardError handler' do + subject.rescue_from(StandardError) { error!('rescued', 500) } + subject.get('/hey') { error!('out of stock', 409) } + + get '/hey' + expect(last_response.status).to eq(409) + expect(last_response.body).to eq('out of stock') + end end describe '#redirect' do diff --git a/spec/grape/exceptions/error_response_spec.rb b/spec/grape/exceptions/error_response_spec.rb index b3da66f76..938c98d49 100644 --- a/spec/grape/exceptions/error_response_spec.rb +++ b/spec/grape/exceptions/error_response_spec.rb @@ -27,6 +27,10 @@ expect(payload.backtrace).to be_nil expect(payload.original_exception).to be_nil end + + it 'is frozen' do + expect(described_class.new(status: 422)).to be_frozen + end end describe '#to_s' do @@ -38,29 +42,6 @@ end end - describe '#==' do - let(:exception) { StandardError.new('inner') } - let(:attrs) { { status: 422, message: 'boom', headers: { 'X-Foo' => 'bar' }, backtrace: ['line 1'], original_exception: exception } } - let(:payload) { described_class.new(**attrs) } - let(:twin) { described_class.new(**attrs) } - - it 'is equal when every attribute matches' do - expect(payload).to eq(twin) - end - - it 'is not equal when any attribute differs' do - expect(payload).not_to eq(described_class.new(**attrs, status: 500)) - end - - it 'is not equal to a non-ErrorResponse with the same shape' do - expect(described_class.new(status: 422)).not_to eq(Object.new) - end - - it 'returns the same hash for equal instances' do - expect(payload.hash).to eq(twin.hash) - end - end - describe '.from_exception' do it 'extracts status, message, headers, and backtrace from a Grape exception' do exception = Grape::Exceptions::Base.new(status: 418, message: 'teapot', headers: { 'X-T' => '1' })