From 85e69e44966ba9259a7fb2636ccb5e7e7253a23f Mon Sep 17 00:00:00 2001 From: Juan Carlos Garcia Date: Sat, 9 Dec 2023 23:45:02 +0100 Subject: [PATCH 1/3] feat(#19): Manage exceptions by default and allow the gem's consumer to manage them by itself --- CHANGELOG.md | 3 +- README.md | 51 ++++++++++++++++- grape-idempotency.gemspec | 3 +- lib/grape/idempotency.rb | 73 +++++++++++++++++------- lib/grape/idempotency/version.rb | 2 +- spec/idempotent_spec.rb | 96 +++++++++++++++++++++++++------- 6 files changed, 184 insertions(+), 44 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dff0b34..36d8c29 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ All changes to `grape-idempotency` will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [1.0.1] - (Next) +## [1.1.0] - (Next) ### Fix @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Feature +* [#20](https://github.com/jcagarcia/grape-idempotency/pull/20): Manage `Redis` exceptions by default and allow the gem's consumer to manage them by itself - [@jcagarcia](https://github.com/jcagarcia). * Your contribution here. ## [1.0.0] - 2023-11-23 diff --git a/README.md b/README.md index fc9fe7a..5712e1c 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,8 @@ Topics covered in this README: - [Installation](#installation-) - [Basic Usage](#basic-usage-) - [How it works](#how-it-works-) -- [Making idempotency key header mandatory](#making-idempotency-key-header-mandatory-) + - [Making idempotency key header mandatory](#making-idempotency-key-header-mandatory-) + - [Redis Storage Connectivity Issue](#redis-storage-connectivity-issue) - [Configuration](#configuration-) - [Changelog](#changelog) - [Contributing](#contributing) @@ -81,7 +82,7 @@ Results are only saved if an API endpoint begins its execution. If incoming para Additionally, this gem automatically appends the `Original-Request` header and the `Idempotency-Key` header to your API's response, enabling you to trace back to the initial request that generated that specific response. -## Making idempotency key header mandatory ⚠️ +### Making idempotency key header mandatory ⚠️ For some endpoints, you want to enforce your consumers to provide idempotency key. So, when wrapping the code inside the `idempotent` method, you can mark it as `required`: @@ -113,6 +114,14 @@ If the Idempotency-Key request header is missing for a idempotent operation requ If you want to change the error message returned in this scenario, check [How to configure idempotency key missing error message](#mandatory_header_response) section. +### Redis Storage Connectivity Issue + +By default, the `grape-idempotency` gem is configured to handle potential `Redis` exceptions. + +Therefore, if an exception arises while attempting to read, write or delete data from the `Redis` storage, the gem will safeguard against a crash by returning empty results. Consequently, the associated `block` will execute without considering potential prior calls made with the provided idempotency key. As a result, the expected idempotent behavior will not be enforced. + +If you want to avoid this functionality, you have the option to configure the gem to refrain from handling these `Redis` exceptions. Please refer to the [manage_redis_exceptions](#manage_redis_exceptions) configuration property. + ## Configuration 🪚 In addition to the storage aspect, you have the option to supply additional configuration details to tailor the gem to the specific requirements of your project. @@ -195,6 +204,44 @@ I, [2023-11-23T22:41:39.148523 #1] DEBUG -- : [my-own-prefix] Request has been I, [2023-11-23T22:41:39.148537 #1] DEBUG -- : [my-own-prefix] Returning the response from the original request. ``` +### manage_redis_exceptions + +By default, the `grape-idempotency` gem is configured to handle potential `Redis` exceptions. + +However, this approach carries a certain level of risk. In the case that `Redis` experiences an outage, the idempotent functionality will be lost, and this issue may go unnoticed. + +If you prefer to take control of handling potential `Redis` exceptions, you have the option to configure the gem to abstain from managing Redis exceptions. + +```ruby +Grape::Idempotency.configure do |c| + c.storage = @storage + c.manage_redis_exceptions = false +end +``` + +Now, if a `Redis` exception arises while attempting to utilize the `Redis` storage, the gem will re-raise the identical exception to your application. Thus, you will be responsible for handling it within your own code, such as: + +```ruby +require 'grape' +require 'grape-idempotency' + +class API < Grape::API + post '/payments' do + begin + idempotent do + status 201 + Payment.create!({ + amount: params[:amount] + }) + end + rescue Redis::BaseError => e + error!("Redis error! Idempotency is very important here and we cannot continue.", 500) + end + end + end +end +``` + ### conflict_error_response When providing a `Idempotency-Key: ` header, this gem compares incoming parameters to those of the original request (if exists) and returns a `409 - Conflict` status code if they don't match, preventing accidental misuse. The response body returned by the gem looks like: diff --git a/grape-idempotency.gemspec b/grape-idempotency.gemspec index c05ee69..3daffc6 100644 --- a/grape-idempotency.gemspec +++ b/grape-idempotency.gemspec @@ -22,7 +22,8 @@ Gem::Specification.new do |spec| spec.required_ruby_version = '>= 2.6' - spec.add_runtime_dependency 'grape', '~> 1' + spec.add_runtime_dependency 'grape', '>= 1' + spec.add_runtime_dependency 'redis', '>= 4' spec.add_development_dependency 'bundler' spec.add_development_dependency 'rspec' diff --git a/lib/grape/idempotency.rb b/lib/grape/idempotency.rb index 3318aa1..2210bfd 100644 --- a/lib/grape/idempotency.rb +++ b/lib/grape/idempotency.rb @@ -1,4 +1,5 @@ require 'grape' +require 'redis' require 'logger' require 'securerandom' require 'grape/idempotency/version' @@ -33,21 +34,21 @@ def idempotent(grape, required: false, &block) grape.error!(configuration.mandatory_header_response, 400) if required && !idempotency_key return block.call if !idempotency_key - cached_request = get_from_cache(idempotency_key) - log(:debug, "Request has been found for the provided idempotency key => #{cached_request}") if cached_request - if cached_request && (cached_request["params"] != grape.request.params || cached_request["path"] != grape.request.path) - log(:debug, "Request has conflicts. Same params? => #{cached_request["params"] != grape.request.params}. Same path? => #{cached_request["path"] != grape.request.path}") + stored_request = get_from_storage(idempotency_key) + log(:debug, "Request has been found for the provided idempotency key => #{stored_request}") if stored_request + if stored_request && (stored_request["params"] != grape.request.params || stored_request["path"] != grape.request.path) + log(:debug, "Request has conflicts. Same params? => #{stored_request["params"] != grape.request.params}. Same path? => #{stored_request["path"] != grape.request.path}") log(:debug, "Returning conflict error response.") grape.error!(configuration.conflict_error_response, 422) - elsif cached_request && cached_request["processing"] == true + elsif stored_request && stored_request["processing"] == true log(:debug, "Returning processing error response.") grape.error!(configuration.processing_response, 409) - elsif cached_request + elsif stored_request log(:debug, "Returning the response from the original request.") - grape.status cached_request["status"] - grape.header(ORIGINAL_REQUEST_HEADER, cached_request["original_request"]) + grape.status stored_request["status"] + grape.header(ORIGINAL_REQUEST_HEADER, stored_request["original_request"]) grape.header(configuration.idempotency_key_header, idempotency_key) - return cached_request["response"] + return stored_request["response"] end log(:debug, "Previous request information has NOT been found for the provided idempotency key.") @@ -76,26 +77,32 @@ def idempotent(grape, required: false, &block) grape.header(ORIGINAL_REQUEST_HEADER, original_request_id) grape.body response + rescue Redis::BaseError => e + raise rescue => e log(:debug, "An unexpected error was raised when performing the block.") - if !cached_request && !response + if !stored_request && !response validate_config! log(:debug, "Storing error response.") original_request_id = get_request_id(grape.request.headers) stored_key = store_error_request(idempotency_key, grape.request.path, grape.request.params, grape.status, original_request_id, e) - log(:debug, "Error response stored.") - grape.header(ORIGINAL_REQUEST_HEADER, original_request_id) - grape.header(configuration.idempotency_key_header, stored_key) + if stored_key + log(:debug, "Error response stored.") + grape.header(ORIGINAL_REQUEST_HEADER, original_request_id) + grape.header(configuration.idempotency_key_header, stored_key) + end end log(:debug, "Re-raising the error.") raise ensure - if !cached_request && response + if !stored_request && response validate_config! log(:debug, "Storing response.") stored_key = store_request_response(idempotency_key, grape.request.path, grape.request.params, grape.status, original_request_id, response) - log(:debug, "Response stored.") - grape.header(configuration.idempotency_key_header, stored_key) + if stored_key + log(:debug, "Response stored.") + grape.header(configuration.idempotency_key_header, stored_key) + end end end @@ -113,6 +120,10 @@ def update_error_with_rescue_from_result(error, status, response) store_request_response(idempotency_key, path, params, status, original_request_id, response) storage.del(stored_error[:error_key]) + rescue Redis::BaseError => e + log(:error, "Storage error => #{e.message} - #{e}") + return if configuration.manage_redis_exceptions + raise end private @@ -122,7 +133,10 @@ def validate_config! end def valid_storage? - configuration.storage && configuration.storage.respond_to?(:set) + configuration.storage && + configuration.storage.respond_to?(:get) && + configuration.storage.respond_to?(:set) && + configuration.storage.respond_to?(:del) end def get_idempotency_key(headers) @@ -141,11 +155,15 @@ def get_request_id(headers) request_id || "req_#{SecureRandom.hex}" end - def get_from_cache(idempotency_key) + def get_from_storage(idempotency_key) value = storage.get(key(idempotency_key)) return unless value JSON.parse(value) + rescue Redis::BaseError => e + log(:error, "Storage error => #{e.message} - #{e}") + return if configuration.manage_redis_exceptions + raise end def store_processing_request(idempotency_key, path, params, request_id) @@ -157,6 +175,9 @@ def store_processing_request(idempotency_key, path, params, request_id) } storage.set(key(idempotency_key), body.to_json, ex: configuration.expires_in, nx: true) + rescue Redis::BaseError => e + return true if configuration.manage_redis_exceptions + raise end def store_request_response(idempotency_key, path, params, status, request_id, response) @@ -171,6 +192,10 @@ def store_request_response(idempotency_key, path, params, status, request_id, re storage.set(key(idempotency_key), body.to_json, ex: configuration.expires_in, nx: false) idempotency_key + rescue Redis::BaseError => e + log(:error, "Storage error => #{e.message} - #{e}") + return if configuration.manage_redis_exceptions + raise end def store_error_request(idempotency_key, path, params, status, request_id, error) @@ -188,6 +213,10 @@ def store_error_request(idempotency_key, path, params, status, request_id, error storage.set(error_key(idempotency_key), body, ex: 30, nx: false) idempotency_key + rescue Redis::BaseError => e + log(:error, "Storage error => #{e.message} - #{e}") + return if configuration.manage_redis_exceptions + raise end def get_error_request_for(error) @@ -207,6 +236,10 @@ def get_error_request_for(error) } end end.first + rescue Redis::BaseError => e + log(:error, "Storage error => #{e.message} - #{e}") + return if configuration.manage_redis_exceptions + raise end def is_an_error?(response) @@ -252,7 +285,8 @@ def configuration class Configuration attr_accessor :storage, :logger, :logger_level, :logger_prefix, :expires_in, :idempotency_key_header, - :request_id_header, :conflict_error_response, :processing_response, :mandatory_header_response + :request_id_header, :conflict_error_response, :processing_response, :mandatory_header_response, + :manage_redis_exceptions class Error < StandardError; end @@ -264,6 +298,7 @@ def initialize @expires_in = 216_000 @idempotency_key_header = "idempotency-key" @request_id_header = "x-request-id" + @manage_redis_exceptions = true @conflict_error_response = { "title" => "Idempotency-Key is already used", "detail" => "This operation is idempotent and it requires correct usage of Idempotency Key. Idempotency Key MUST not be reused across different payloads of this operation." diff --git a/lib/grape/idempotency/version.rb b/lib/grape/idempotency/version.rb index 6f43466..69a9b5a 100644 --- a/lib/grape/idempotency/version.rb +++ b/lib/grape/idempotency/version.rb @@ -2,6 +2,6 @@ module Grape module Idempotency - VERSION = '1.0.1' + VERSION = '1.1.0' end end \ No newline at end of file diff --git a/spec/idempotent_spec.rb b/spec/idempotent_spec.rb index 36c22e5..de006a5 100644 --- a/spec/idempotent_spec.rb +++ b/spec/idempotent_spec.rb @@ -22,13 +22,13 @@ it 'is registered as grape helper' do expected_response_body = { payments: [] } - + app.post('/payments') do idempotent do status 401 end end - + post 'payments', { amount: 100_00 }.to_json end @@ -37,19 +37,19 @@ context 'and all the parameters matches with the original request' do it 'returns the original response' do allow(SecureRandom).to receive(:random_number).and_return(1, 2) - + app.post('/payments') do idempotent do status 200 { amount_to: SecureRandom.random_number }.to_json end end - + header "idempotency-key", idempotency_key post 'payments?locale=es', { amount: 100_00 }.to_json expect(last_response.status).to eq(200) expect(last_response.body).to eq({ amount_to: 1 }.to_json) - + header "idempotency-key", idempotency_key post 'payments?locale=es', { amount: 100_00 }.to_json expect(last_response.status).to eq(200) @@ -65,10 +65,10 @@ end expect(storage).to receive(:set).twice.and_call_original - + header "idempotency-key", idempotency_key post 'payments?locale=es', { amount: 100_00 }.to_json - + header "idempotency-key", idempotency_key post 'payments?locale=es', { amount: 100_00 }.to_json end @@ -76,20 +76,20 @@ it 'includes the original request id and the idempotency key in the response headers' do original_request_id = "a-request-identifier" allow(SecureRandom).to receive(:random_number).and_return(1, 2) - + app.post('/payments') do idempotent do status 200 { amount_to: SecureRandom.random_number }.to_json end end - + header "idempotency-key", idempotency_key header "x-request-id", original_request_id post 'payments?locale=es', { amount: 100_00 }.to_json expect(last_response.headers).to include("original-request" => original_request_id) expect(last_response.headers).to include("idempotency-key" => idempotency_key) - + header "idempotency-key", idempotency_key header "x-request-id", 'another-request-id' post 'payments?locale=es', { amount: 100_00 }.to_json @@ -100,18 +100,18 @@ context 'when the original request does not include a request id header' do it 'generates a new one' do allow(SecureRandom).to receive(:hex).and_return("123456") - + app.post('/payments') do idempotent do status 200 { amount_to: SecureRandom.random_number }.to_json end end - + header "idempotency-key", idempotency_key post 'payments?locale=es', { amount: 100_00 }.to_json expect(last_response.headers).to include("original-request" => "req_123456") - + header "idempotency-key", idempotency_key header "x-request-id", 'another-request-id' post 'payments?locale=es', { amount: 100_00 }.to_json @@ -196,7 +196,7 @@ end end end - + context 'and there is NOT a response already stored in the storage' do it 'stores the the request as processing initially without overwritting and later stores the response' do expected_response_body = { error: "Internal Server Error" } @@ -264,12 +264,12 @@ end end end - + header "idempotency-key", idempotency_key post 'payments?locale=es', { amount: 100_00 }.to_json expect(last_response.status).to eq(500) expect(last_response.body).to eq("{:error=>\"Internal Server Error\", :message=>\"Unexpected error\"}") - + header "idempotency-key", idempotency_key post 'payments?locale=es', { amount: 100_00 }.to_json expect(last_response.status).to eq(500) @@ -294,12 +294,12 @@ { amount_to: 100_00 }.to_json end end - + header "idempotency-key", idempotency_key post 'payments?locale=es', { amount: 100_00 }.to_json expect(last_response.status).to eq(404) expect(last_response.body).to eq("{\"message\":\"Not found error\"}") - + header "idempotency-key", idempotency_key post 'payments?locale=es', { amount: 100_00 }.to_json expect(last_response.status).to eq(404) @@ -315,18 +315,74 @@ expected_response_body.to_json end end - + header "idempotency-key", idempotency_key post 'payments?locale=undefined', { amount: 100_00 }.to_json expect(last_response.headers).to include("idempotency-key" => idempotency_key) end end + + context 'and a Redis exception appears in any of the redis methods' do + before do + allow(storage).to receive(:get).and_raise(Redis::CannotConnectError) + allow(storage).to receive(:set).and_raise(Redis::CannotConnectError) + allow(storage).to receive(:del).and_raise(Redis::CannotConnectError) + end + + it 'manages the exeception and the endpoint behaves as no idempotent' do + allow(SecureRandom).to receive(:random_number).and_return(1, 2) + + app.post('/payments') do + idempotent do + status 201 + { amount_to: SecureRandom.random_number }.to_json + end + end + + header "idempotency-key", idempotency_key + post 'payments', { amount: 100_00 }.to_json + expect(last_response.body).to eq({ amount_to: 1 }.to_json) + + header "idempotency-key", idempotency_key + post 'payments', { amount: 100_00 }.to_json + expect(last_response.body).to eq({ amount_to: 2 }.to_json) + end + + context 'and the gem is configured for NOT managing Redis exceptions' do + before do + Grape::Idempotency.configure do |c| + c.storage = storage + c.manage_redis_exceptions = false + end + end + + after do + Grape::Idempotency.configure do |c| + c.storage = storage + end + end + + it 'raises the redis exception' do + app.post('/payments') do + idempotent do + status 201 + { amount_to: SecureRandom.random_number }.to_json + end + end + + expect { + header "idempotency-key", idempotency_key + post 'payments', { amount: 100_00 }.to_json + }.to raise_error(Redis::CannotConnectError) + end + end + end end context 'and a idempotency key is NOT provided in the request' do it 'returns the block result in the response body without checking idempotency' do allow(SecureRandom).to receive(:random_number).and_return(1, 2) - + app.post('/payments') do idempotent do status 201 From e3bef43736b6463e3f5c469ac058ae1b7bf4e83e Mon Sep 17 00:00:00 2001 From: Juan Carlos Garcia Date: Mon, 11 Dec 2023 21:32:46 +0100 Subject: [PATCH 2/3] feat(#19): Re-raise redis exceptions by default --- README.md | 58 +++++++++++++++++++--------------------- lib/grape/idempotency.rb | 2 +- spec/idempotent_spec.rb | 36 ++++++++++++------------- 3 files changed, 47 insertions(+), 49 deletions(-) diff --git a/README.md b/README.md index 5712e1c..07e66da 100644 --- a/README.md +++ b/README.md @@ -116,11 +116,32 @@ If you want to change the error message returned in this scenario, check [How to ### Redis Storage Connectivity Issue -By default, the `grape-idempotency` gem is configured to handle potential `Redis` exceptions. +By default, `Redis` exceptions are not handled by the `grape-idempotency` gem. -Therefore, if an exception arises while attempting to read, write or delete data from the `Redis` storage, the gem will safeguard against a crash by returning empty results. Consequently, the associated `block` will execute without considering potential prior calls made with the provided idempotency key. As a result, the expected idempotent behavior will not be enforced. +Therefore, if an exception arises while attempting to read, write or delete data from the `Redis` storage, the gem will re-raise the identical exception to your application. Thus, you will be responsible for handling it within your own code, such as: -If you want to avoid this functionality, you have the option to configure the gem to refrain from handling these `Redis` exceptions. Please refer to the [manage_redis_exceptions](#manage_redis_exceptions) configuration property. +```ruby +require 'grape' +require 'grape-idempotency' + +class API < Grape::API + post '/payments' do + begin + idempotent do + status 201 + Payment.create!({ + amount: params[:amount] + }) + end + rescue Redis::BaseError => e + error!("Redis error! Idempotency is very important here and we cannot continue.", 500) + end + end + end +end +``` + +If you want to avoid this functionality, and you want the gem handles the potential `Redis` exceptions, you have the option to configure the gem for handling these `Redis` exceptions. Please refer to the [manage_redis_exceptions](#manage_redis_exceptions) configuration property. ## Configuration 🪚 @@ -206,41 +227,18 @@ I, [2023-11-23T22:41:39.148537 #1] DEBUG -- : [my-own-prefix] Returning the res ### manage_redis_exceptions -By default, the `grape-idempotency` gem is configured to handle potential `Redis` exceptions. - -However, this approach carries a certain level of risk. In the case that `Redis` experiences an outage, the idempotent functionality will be lost, and this issue may go unnoticed. +By default, the `grape-idempotency` gem is configured to re-raise `Redis` exceptions. -If you prefer to take control of handling potential `Redis` exceptions, you have the option to configure the gem to abstain from managing Redis exceptions. +If you want to delegate the `Redis` exception management into the gem, you can configure it using the `manage_redis_exceptions` configuration property. ```ruby Grape::Idempotency.configure do |c| c.storage = @storage - c.manage_redis_exceptions = false + c.manage_redis_exceptions = true end ``` -Now, if a `Redis` exception arises while attempting to utilize the `Redis` storage, the gem will re-raise the identical exception to your application. Thus, you will be responsible for handling it within your own code, such as: - -```ruby -require 'grape' -require 'grape-idempotency' - -class API < Grape::API - post '/payments' do - begin - idempotent do - status 201 - Payment.create!({ - amount: params[:amount] - }) - end - rescue Redis::BaseError => e - error!("Redis error! Idempotency is very important here and we cannot continue.", 500) - end - end - end -end -``` +However, this approach carries a certain level of risk. In the case that `Redis` experiences an outage, the idempotent functionality will be lost, the endpoint will behave as no idempotent, and this issue may go unnoticed. ### conflict_error_response diff --git a/lib/grape/idempotency.rb b/lib/grape/idempotency.rb index 2210bfd..b0c328f 100644 --- a/lib/grape/idempotency.rb +++ b/lib/grape/idempotency.rb @@ -298,7 +298,7 @@ def initialize @expires_in = 216_000 @idempotency_key_header = "idempotency-key" @request_id_header = "x-request-id" - @manage_redis_exceptions = true + @manage_redis_exceptions = false @conflict_error_response = { "title" => "Idempotency-Key is already used", "detail" => "This operation is idempotent and it requires correct usage of Idempotency Key. Idempotency Key MUST not be reused across different payloads of this operation." diff --git a/spec/idempotent_spec.rb b/spec/idempotent_spec.rb index de006a5..602279b 100644 --- a/spec/idempotent_spec.rb +++ b/spec/idempotent_spec.rb @@ -329,9 +329,7 @@ allow(storage).to receive(:del).and_raise(Redis::CannotConnectError) end - it 'manages the exeception and the endpoint behaves as no idempotent' do - allow(SecureRandom).to receive(:random_number).and_return(1, 2) - + it 'raises the redis exception' do app.post('/payments') do idempotent do status 201 @@ -339,20 +337,17 @@ end end - header "idempotency-key", idempotency_key - post 'payments', { amount: 100_00 }.to_json - expect(last_response.body).to eq({ amount_to: 1 }.to_json) - - header "idempotency-key", idempotency_key - post 'payments', { amount: 100_00 }.to_json - expect(last_response.body).to eq({ amount_to: 2 }.to_json) + expect { + header "idempotency-key", idempotency_key + post 'payments', { amount: 100_00 }.to_json + }.to raise_error(Redis::CannotConnectError) end - context 'and the gem is configured for NOT managing Redis exceptions' do + context 'and the gem is configured for managing Redis exceptions' do before do Grape::Idempotency.configure do |c| c.storage = storage - c.manage_redis_exceptions = false + c.manage_redis_exceptions = true end end @@ -362,18 +357,23 @@ end end - it 'raises the redis exception' do + it 'manages the exeception and the endpoint behaves as no idempotent' do + allow(SecureRandom).to receive(:random_number).and_return(1, 2) + app.post('/payments') do idempotent do status 201 { amount_to: SecureRandom.random_number }.to_json end end - - expect { - header "idempotency-key", idempotency_key - post 'payments', { amount: 100_00 }.to_json - }.to raise_error(Redis::CannotConnectError) + + header "idempotency-key", idempotency_key + post 'payments', { amount: 100_00 }.to_json + expect(last_response.body).to eq({ amount_to: 1 }.to_json) + + header "idempotency-key", idempotency_key + post 'payments', { amount: 100_00 }.to_json + expect(last_response.body).to eq({ amount_to: 2 }.to_json) end end end From c15b2e08915c3d9a4ea3d49bcdf56b44a0a020e2 Mon Sep 17 00:00:00 2001 From: Juan Carlos Garcia Date: Mon, 11 Dec 2023 21:56:56 +0100 Subject: [PATCH 3/3] feat(#19): Prevent to re-raise if exceptions appear after performing the wrapped code --- README.md | 2 ++ lib/grape/idempotency.rb | 12 ++++-------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 07e66da..7c2aacb 100644 --- a/README.md +++ b/README.md @@ -143,6 +143,8 @@ end If you want to avoid this functionality, and you want the gem handles the potential `Redis` exceptions, you have the option to configure the gem for handling these `Redis` exceptions. Please refer to the [manage_redis_exceptions](#manage_redis_exceptions) configuration property. +🚨 WARNING: If a `Redis` exception appears AFTER performing the wrapped code, nothing will be re-raised. The process will continue working and the response will be returned to the consumer of your API. However, a `409 Conflict` response can be returned to your consumer if it retried the same call with the same idempotency key. This is because the gem was not able to associate the response of the original request to the original idempotency key because those connectivity issues. + ## Configuration 🪚 In addition to the storage aspect, you have the option to supply additional configuration details to tailor the gem to the specific requirements of your project. diff --git a/lib/grape/idempotency.rb b/lib/grape/idempotency.rb index b0c328f..7af86e2 100644 --- a/lib/grape/idempotency.rb +++ b/lib/grape/idempotency.rb @@ -122,8 +122,7 @@ def update_error_with_rescue_from_result(error, status, response) storage.del(stored_error[:error_key]) rescue Redis::BaseError => e log(:error, "Storage error => #{e.message} - #{e}") - return if configuration.manage_redis_exceptions - raise + nil end private @@ -194,8 +193,7 @@ def store_request_response(idempotency_key, path, params, status, request_id, re idempotency_key rescue Redis::BaseError => e log(:error, "Storage error => #{e.message} - #{e}") - return if configuration.manage_redis_exceptions - raise + idempotency_key end def store_error_request(idempotency_key, path, params, status, request_id, error) @@ -215,8 +213,7 @@ def store_error_request(idempotency_key, path, params, status, request_id, error idempotency_key rescue Redis::BaseError => e log(:error, "Storage error => #{e.message} - #{e}") - return if configuration.manage_redis_exceptions - raise + idempotency_key end def get_error_request_for(error) @@ -238,8 +235,7 @@ def get_error_request_for(error) end.first rescue Redis::BaseError => e log(:error, "Storage error => #{e.message} - #{e}") - return if configuration.manage_redis_exceptions - raise + nil end def is_an_error?(response)