Skip to content
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# This configuration was generated by
# `rubocop --auto-gen-config`
# on 2024-10-06 16:00:59 UTC using RuboCop version 1.66.1.
# on 2025-01-28 18:36:21 UTC using RuboCop version 1.66.1.
# The point is for the user to remove these configuration records
# one by one as the offenses are removed from the code base.
# Note that changes in the inspected code, or installation of new
Expand All @@ -12,6 +12,11 @@ Metrics/MethodLength:
Exclude:
- 'lib/grape/endpoint.rb'

# Offense count: 2
# Configuration parameters: CountKeywordArgs, MaxOptionalParameters.
Metrics/ParameterLists:
Max: 6

# Offense count: 18
# Configuration parameters: EnforcedStyle, CheckMethodNames, CheckSymbols, AllowedIdentifiers, AllowedPatterns.
# SupportedStyles: snake_case, normalcase, non_integer
Expand Down Expand Up @@ -60,7 +65,7 @@ RSpec/IndexedLet:
- 'spec/grape/presenters/presenter_spec.rb'
- 'spec/shared/versioning_examples.rb'

# Offense count: 38
# Offense count: 39
# Configuration parameters: AssignmentOnly.
RSpec/InstanceVariable:
Exclude:
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
* [#2514](https://github.com/ruby-grape/grape/pull/2514): Add rails 8.0 to CI - [@ericproulx](https://github.com/ericproulx).
* [#2516](https://github.com/ruby-grape/grape/pull/2516): Dynamic registration for parsers, formatters, versioners - [@ericproulx](https://github.com/ericproulx).
* [#2518](https://github.com/ruby-grape/grape/pull/2518): Add ruby 3.4 to CI - [@ericproulx](https://github.com/ericproulx).
* [#2528](https://github.com/ruby-grape/grape/pull/2528): Pass `status` into `ErrorFormatter` - [@drewnichols](https://github.com/drewnichols).

* Your contribution here.

#### Fixes
Expand Down
8 changes: 4 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2772,8 +2772,8 @@ Custom error formatters for existing and additional types can be defined with a

```ruby
class Twitter::API < Grape::API
error_formatter :txt, ->(message, backtrace, options, env, original_exception) {
"error: #{message} from #{backtrace}"
error_formatter :txt, ->(message, backtrace, options, env, original_exception, status) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a breaking change? Meaning if I had the old code and upgrade Grape, will this fail with an invalid number of parameters?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Best I could tell this calls Grape::Middleware::Error.format_message where I defaulted status to nil. If someone created a custom Middleware there could be a problem. Fairpoint though.. I can check this with my own app and also look into making some more specs to verify.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we document custom middleware this would be a breaking change :(

"error: #{message} status: #{status} from #{backtrace}"
}
end
```
Expand All @@ -2782,8 +2782,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(message, backtrace, options, env, original_exception, status)
{ message: message, status: status, backtrace: backtrace }
end
end

Expand Down
2 changes: 1 addition & 1 deletion lib/grape/error_formatter/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ module Grape
module ErrorFormatter
class Base
class << self
def call(message, backtrace, options = {}, env = nil, original_exception = nil)
def call(message, backtrace, options = {}, env = nil, original_exception = nil, _status = nil)
merge_backtrace = backtrace.present? && options.dig(:rescue_options, :backtrace)
merge_original_exception = original_exception && options.dig(:rescue_options, :original_exception)

Expand Down
8 changes: 4 additions & 4 deletions lib/grape/middleware/error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@ 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(message, backtrace, original_exception = nil, status = nil)
format = env[Grape::Env::API_FORMAT] || options[:format]
formatter = Grape::ErrorFormatter.formatter_for(format, options[:error_formatters], options[:default_error_formatter])
return formatter.call(message, backtrace, options, env, original_exception) if formatter
return formatter.call(message, backtrace, options, env, original_exception, status) if formatter

throw :error,
status: 406,
Expand All @@ -71,7 +71,7 @@ def error_response(error = {})
end
backtrace = error[:backtrace] || error[:original_exception]&.backtrace || []
original_exception = error.is_a?(Exception) ? error : error[:original_exception] || nil
rack_response(status, headers, format_message(message, backtrace, original_exception))
rack_response(status, headers, format_message(message, backtrace, original_exception, status))
end

def default_rescue_handler(exception)
Expand Down Expand Up @@ -132,7 +132,7 @@ def run_rescue_handler(handler, error, endpoint)
def error!(message, status = options[:default_status], headers = {}, backtrace = [], original_exception = nil)
rack_response(
status, headers.reverse_merge(Rack::CONTENT_TYPE => content_type),
format_message(message, backtrace, original_exception)
format_message(message, backtrace, original_exception, status)
)
end

Expand Down
10 changes: 5 additions & 5 deletions spec/grape/api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2509,8 +2509,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(message, _backtrace, _options, _env, _original_exception, status)
"message: #{message} status: #{status} @backtrace"
end
end
end
Expand All @@ -2521,7 +2521,7 @@ def self.call(message, _backtrace, _options, _env, _original_exception)
subject.get('/exception') { raise 'rain!' }

get '/exception'
expect(last_response.body).to eq('message: rain! @backtrace')
expect(last_response.body).to eq('message: rain! status: 500 @backtrace')
end

it 'returns a custom error format (using keyword :with)' do
Expand All @@ -2530,7 +2530,7 @@ def self.call(message, _backtrace, _options, _env, _original_exception)
subject.get('/exception') { raise 'rain!' }

get '/exception'
expect(last_response.body).to eq('message: rain! @backtrace')
expect(last_response.body).to eq('message: rain! status: 500 @backtrace')
end

it 'returns a modified error with a custom error format' do
Expand All @@ -2542,7 +2542,7 @@ def self.call(message, _backtrace, _options, _env, _original_exception)
raise 'rain!'
end
get '/exception'
expect(last_response.body).to eq('message: raining dogs and cats @backtrace')
expect(last_response.body).to eq('message: raining dogs and cats status: 418 @backtrace')
end
end

Expand Down
Loading