From d747e80c5cf979684d0ed29e3280ba03c3baf58c Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Thu, 10 Sep 2026 22:10:37 +0200 Subject: [PATCH] Skip three redundant steps on every error response - `Middleware::Error#rack_response` built a `Grape::Util::Header` from the headers only to hand it to `Rack::Response.new`, which copies its argument into a Headers of its own on every supported Rack. Rack 2.2's `HeaderHash[]` adopts only a Hash that already is one, and 3.x always builds a fresh `Rack::Headers`. #2911 kept this copy because callers write into what they are given, but here the writer is Rack::Response, which writes into its own copy, and all three callers build the Hash fresh for this response. The copy #2911 kept in `API::Instance#call`, which can be handed a mounted app's frozen Hash, stays. - `run_rescue_handler` ran every handler through `instance_exec`, turning a Method into a Proc first. The middleware's own handlers and the endpoint's for a `with:` Symbol are Methods, already bound to a receiver that `instance_exec` cannot change, so they are now called as they are. Blocks still run as the endpoint. - `ErrorFormatter::Json#ensure_utf8` re-encoded every String message, which for one that already is valid UTF-8 only copies it, for about 260 ns. Such a message is now returned as it is; anything else is converted as before. What `ensure_utf8` does with a message that is not valid UTF-8 had no spec; dropping the encoding half of the new check passed the suite. The new spec pins all three cases: UTF-8, binary and malformed. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 1 + lib/grape/error_formatter/json.rb | 3 +++ lib/grape/middleware/error.rb | 20 +++++++++++++++-- spec/grape/error_formatter/json_spec.rb | 30 +++++++++++++++++++++++++ 4 files changed, 52 insertions(+), 2 deletions(-) create mode 100644 spec/grape/error_formatter/json_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index fdc7d3a3e..5d84bbbd6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ * [#2918](https://github.com/ruby-grape/grape/pull/2918): Skip the dry-types round trip when a value already is the declared type - [@ericproulx](https://github.com/ericproulx). * [#2917](https://github.com/ruby-grape/grape/pull/2917): Read path captures out of the router's union match instead of re-running the route's pattern - [@ericproulx](https://github.com/ericproulx). * [#2921](https://github.com/ruby-grape/grape/pull/2921): Pin the router's request-time isolation regressions through requests instead of its instance variables - [@ericproulx](https://github.com/ericproulx). +* [#2929](https://github.com/ruby-grape/grape/pull/2929): Skip a header copy, a message re-encode and a Proc conversion on every error response - [@ericproulx](https://github.com/ericproulx). * Your contribution here. ### 4.0.0 (2026-09-07) diff --git a/lib/grape/error_formatter/json.rb b/lib/grape/error_formatter/json.rb index fe34a904b..5b3ef469a 100644 --- a/lib/grape/error_formatter/json.rb +++ b/lib/grape/error_formatter/json.rb @@ -23,8 +23,11 @@ def wrap_message(message) { error: ensure_utf8(message) } end + # Re-encoding a String that already is valid UTF-8 only copies it, for + # about 260 ns on every error response. def ensure_utf8(message) return message unless message.respond_to? :encode + return message if message.is_a?(String) && message.encoding == Encoding::UTF_8 && message.valid_encoding? message.encode('UTF-8', invalid: :replace, undef: :replace) end diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb index a2dd7c48a..09774a40a 100644 --- a/lib/grape/middleware/error.rb +++ b/lib/grape/middleware/error.rb @@ -66,9 +66,13 @@ def call!(env) private + # +headers+ is handed over as it is: Rack::Response copies it into a + # Headers of its own on every supported Rack (Rack 2.2 through + # +HeaderHash[]+, which only adopts a Hash that already is one), and every + # caller builds it fresh for this response. def rack_response(status, headers, message) body = html_content_type?(headers[Rack::CONTENT_TYPE]) ? Rack::Utils.escape_html(message) : message - Rack::Response.new(Array.wrap(body), Rack::Utils.status_code(status), Grape::Util::Header.new.merge!(headers)) + Rack::Response.new(Array.wrap(body), Rack::Utils.status_code(status), headers) end # Escaping must key off the media type only, case-insensitively. Comparing @@ -268,7 +272,7 @@ def rescue_handler_for_any_class(klass) def run_rescue_handler(handler, error, endpoint, redispatched: false) callable = handler.is_a?(Symbol) ? endpoint.public_method(handler) : handler response = catch(:error) do - callable.arity.zero? ? endpoint.instance_exec(&callable) : endpoint.instance_exec(error, &callable) + call_rescue_handler(callable, error, endpoint) rescue StandardError => e return redispatch(e, endpoint, redispatched) end @@ -279,6 +283,18 @@ def run_rescue_handler(handler, error, endpoint, redispatched: false) run_rescue_handler(method(:default_rescue_handler), Grape::Exceptions::InvalidResponse.new, endpoint) end + # A +rescue_from+ block runs as the endpoint. A Method (the middleware's + # own handlers, or the endpoint's for a +with:+ Symbol) is already bound + # to a receiver that instance_exec cannot change, so it is called as it + # is rather than turned into a Proc first. + def call_rescue_handler(callable, error, endpoint) + if callable.is_a?(Method) + callable.arity.zero? ? callable.call : callable.call(error) + else + callable.arity.zero? ? endpoint.instance_exec(&callable) : endpoint.instance_exec(error, &callable) + end + end + # Route an exception raised inside a +rescue_from+ block. # # * If we have already redispatched once (the redispatched handler diff --git a/spec/grape/error_formatter/json_spec.rb b/spec/grape/error_formatter/json_spec.rb new file mode 100644 index 000000000..2b9676a4a --- /dev/null +++ b/spec/grape/error_formatter/json_spec.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +describe Grape::ErrorFormatter::Json do + let(:app) do + Class.new(Grape::API) do + format :json + get('/utf8') { error!('café', 400) } + get('/binary') { error!("caf\xC3\xA9".b, 400) } + get('/malformed') { error!("caf\xC3", 400) } + end + end + + # A String message goes out as UTF-8 whatever it arrived as: one that already + # is valid UTF-8 as it is, anything else converted, with what does not + # convert replaced rather than failing the response. + it 'renders a UTF-8 message as it is' do + get '/utf8' + expect(JSON.parse(last_response.body)).to eq('error' => 'café') + end + + it 'converts a binary message, replacing the bytes it cannot map' do + get '/binary' + expect(JSON.parse(last_response.body)).to eq('error' => 'caf��') + end + + it 'replaces the invalid bytes of a malformed UTF-8 message' do + get '/malformed' + expect(JSON.parse(last_response.body)).to eq('error' => 'caf�') + end +end