From d05f0ecea56d38005a1b278c66c90757e6f1976d Mon Sep 17 00:00:00 2001 From: Drew Nichols Date: Mon, 27 Jan 2025 22:33:34 -0800 Subject: [PATCH 1/4] added http status to the format_error parameter list making it accessible to ErrorFormatter children --- CHANGELOG.md | 2 ++ README.md | 8 ++++---- lib/grape/error_formatter/base.rb | 2 +- lib/grape/middleware/error.rb | 8 ++++---- spec/grape/api_spec.rb | 10 +++++----- 5 files changed, 16 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c88b65f80..1380d7456 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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): Passed status to ErrorFormatter - [@drewnichols](https://github.com/drewnichols). + * Your contribution here. #### Fixes diff --git a/README.md b/README.md index c6fa674d8..541ec47b6 100644 --- a/README.md +++ b/README.md @@ -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) { + "error: #{message} status: #{status} from #{backtrace}" } end ``` @@ -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 diff --git a/lib/grape/error_formatter/base.rb b/lib/grape/error_formatter/base.rb index f2cf223c6..06b2430e9 100644 --- a/lib/grape/error_formatter/base.rb +++ b/lib/grape/error_formatter/base.rb @@ -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) # rubocop:disable Metrics/ParameterLists merge_backtrace = backtrace.present? && options.dig(:rescue_options, :backtrace) merge_original_exception = original_exception && options.dig(:rescue_options, :original_exception) diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb index f201ee8ef..af42ef8c4 100644 --- a/lib/grape/middleware/error.rb +++ b/lib/grape/middleware/error.rb @@ -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, @@ -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) @@ -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 diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index 20193ccb3..b1a80681d 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -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) # rubocop:disable Metrics/ParameterLists + "message: #{message} status: #{status} @backtrace" end end end @@ -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 @@ -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 @@ -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 From 5c868d4a199f5bca9a2cba8ba194d121aa48907e Mon Sep 17 00:00:00 2001 From: Drew Nichols Date: Tue, 28 Jan 2025 09:34:28 -0800 Subject: [PATCH 2/4] fixed CHANGELOG.md tense Co-authored-by: Daniel (dB.) Doubrovkine --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1380d7456..be958ba2a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ * [#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): Passed status to ErrorFormatter - [@drewnichols](https://github.com/drewnichols). +* [#2528](https://github.com/ruby-grape/grape/pull/2528): Pass `status` into `ErrorFormatter` - [@drewnichols](https://github.com/drewnichols). * Your contribution here. From b5d5cae9145854966c954f07809a5e5bd98fe132 Mon Sep 17 00:00:00 2001 From: Drew Nichols Date: Tue, 28 Jan 2025 10:38:02 -0800 Subject: [PATCH 3/4] moving rubocop violations to the todo list --- .rubocop_todo.yml | 9 +++++++-- lib/grape/error_formatter/base.rb | 2 +- spec/grape/api_spec.rb | 2 +- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 11eb9f51e..bef6ce52d 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -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 @@ -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 @@ -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: diff --git a/lib/grape/error_formatter/base.rb b/lib/grape/error_formatter/base.rb index 06b2430e9..9cef2ead6 100644 --- a/lib/grape/error_formatter/base.rb +++ b/lib/grape/error_formatter/base.rb @@ -4,7 +4,7 @@ module Grape module ErrorFormatter class Base class << self - def call(message, backtrace, options = {}, env = nil, original_exception = nil, _status = nil) # rubocop:disable Metrics/ParameterLists + 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) diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index b1a80681d..6c410af09 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -2509,7 +2509,7 @@ def rescue_all_errors context 'class' do let(:custom_error_formatter) do Class.new do - def self.call(message, _backtrace, _options, _env, _original_exception, status) # rubocop:disable Metrics/ParameterLists + def self.call(message, _backtrace, _options, _env, _original_exception, status) "message: #{message} status: #{status} @backtrace" end end From 86945868d1933d4257163f2d589b580557182ebd Mon Sep 17 00:00:00 2001 From: Drew Nichols Date: Tue, 28 Jan 2025 10:59:41 -0800 Subject: [PATCH 4/4] fixing spec --- spec/grape/middleware/exception_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/grape/middleware/exception_spec.rb b/spec/grape/middleware/exception_spec.rb index b209b726f..2b69525fc 100644 --- a/spec/grape/middleware/exception_spec.rb +++ b/spec/grape/middleware/exception_spec.rb @@ -211,7 +211,7 @@ def call(_env) rescue_all: true, format: :custom, error_formatters: { - custom: lambda do |message, _backtrace, _options, _env, _original_exception| + custom: lambda do |message, _backtrace, _options, _env, _original_exception, _status| { custom_formatter: message }.inspect end }