Skip to content

Commit d978262

Browse files
authored
Fix double wrap on json errors (#2771)
* Add failing spec for additional envelope wrapping errors * Fix for wrap_message incorrectly wrapping SimpleDelegator-wrapped Hash * Update changelog
1 parent 7ac57ff commit d978262

3 files changed

Lines changed: 33 additions & 8 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
* [#2767](https://github.com/ruby-grape/grape/pull/2767): Update rubocop to 1.88.0 and rubocop-rspec to 3.10.2 - [@ericproulx](https://github.com/ericproulx).
1111
* [#2770](https://github.com/ruby-grape/grape/pull/2770): Avoid per-entry array allocation in `Request#build_headers` - [@ericproulx](https://github.com/ericproulx).
12+
* [#2771](https://github.com/ruby-grape/grape/pull/2771): Fix double wrap on json errors - [@MattHall](https://github.com/MattHall).
1213
* Your contribution here.
1314

1415
### 3.3.0 (2026-06-20)

lib/grape/error_formatter/json.rb

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,16 @@ def format_structured_message(structured_message)
1111
private
1212

1313
def wrap_message(message)
14-
case message
15-
when Hash
16-
message
17-
when Exceptions::ValidationErrors
18-
message.as_json
19-
else
20-
{ error: ensure_utf8(message) }
21-
end
14+
# Use +is_a?+ rather than +case/when+ here. +case/when Hash+ matches via
15+
# +Module#===+, a C-level real-class check that ignores delegation, so a
16+
# +SimpleDelegator+ wrapping a Hash (e.g. the +OutputBuilder+ returned by
17+
# +Grape::Entity#serializable_hash+ when an error is presented via an entity)
18+
# would fall through and be wrapped in a spurious +{ error: ... }+ envelope.
19+
# +is_a?+ is forwarded by the delegator to the wrapped Hash, so it matches.
20+
return message if message.is_a?(Hash)
21+
return message.as_json if message.is_a?(Exceptions::ValidationErrors)
22+
23+
{ error: ensure_utf8(message) }
2224
end
2325

2426
def ensure_utf8(message)

spec/integration/grape_entity/entity_spec.rb

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -417,5 +417,27 @@ def static
417417
expect(subject.body).to eql({ code: 408, static: 'some static text' }.to_json)
418418
end
419419
end
420+
421+
context 'when the format is :json' do
422+
let(:app) do
423+
Class.new(Grape::API) do
424+
format :json
425+
426+
desc 'some desc', http_codes: [[408, 'Unauthorized', ErrorPresenter]]
427+
get '/exception' do
428+
error!({ code: 408 }, 408)
429+
end
430+
end
431+
end
432+
433+
# Regression: a presented error hash must not be wrapped in an extra
434+
# `{ "error": ... }` envelope. `Grape::Entity#serializable_hash` returns a
435+
# `SimpleDelegator` around a Hash, which `Json#wrap_message`'s `case/when Hash`
436+
# (via `Module#===`) fails to match, so it falls through to the `else` branch.
437+
it 'is presented without an extra error envelope' do
438+
expect(subject).to be_request_timeout
439+
expect(JSON(subject.body)).to eql('code' => 408, 'static' => 'some static text')
440+
end
441+
end
420442
end
421443
end

0 commit comments

Comments
 (0)