fix: throw bad request while pushing duplicate data to redis (#3016)

This commit is contained in:
Kartikeya Hegde
2023-12-06 11:26:25 +05:30
committed by GitHub
parent c6e2ee29d9
commit a2405e56fb
9 changed files with 63 additions and 12 deletions

View File

@ -480,3 +480,25 @@ impl<T> ConnectorErrorExt<T> for error_stack::Result<T, errors::ConnectorError>
} }
} }
} }
pub trait RedisErrorExt {
#[track_caller]
fn to_redis_failed_response(self, key: &str) -> error_stack::Report<errors::StorageError>;
}
impl RedisErrorExt for error_stack::Report<errors::RedisError> {
fn to_redis_failed_response(self, key: &str) -> error_stack::Report<errors::StorageError> {
match self.current_context() {
errors::RedisError::NotFound => self.change_context(
errors::StorageError::ValueNotFound(format!("Data does not exist for key {key}",)),
),
errors::RedisError::SetNxFailed => {
self.change_context(errors::StorageError::DuplicateValue {
entity: "redis",
key: Some(key.to_string()),
})
}
_ => self.change_context(errors::StorageError::KVError),
}
}
}

View File

@ -275,7 +275,7 @@ mod storage {
use super::RefundInterface; use super::RefundInterface;
use crate::{ use crate::{
connection, connection,
core::errors::{self, CustomResult}, core::errors::{self, utils::RedisErrorExt, CustomResult},
db::reverse_lookup::ReverseLookupInterface, db::reverse_lookup::ReverseLookupInterface,
services::Store, services::Store,
types::storage::{self as storage_types, enums, kv}, types::storage::{self as storage_types, enums, kv},
@ -437,7 +437,7 @@ mod storage {
&key, &key,
) )
.await .await
.change_context(errors::StorageError::KVError)? .map_err(|err| err.to_redis_failed_response(&key))?
.try_into_hsetnx() .try_into_hsetnx()
{ {
Ok(HsetnxReply::KeyNotSet) => Err(errors::StorageError::DuplicateValue { Ok(HsetnxReply::KeyNotSet) => Err(errors::StorageError::DuplicateValue {
@ -544,7 +544,7 @@ mod storage {
&key, &key,
) )
.await .await
.change_context(errors::StorageError::KVError)? .map_err(|err| err.to_redis_failed_response(&key))?
.try_into_hset() .try_into_hset()
.change_context(errors::StorageError::KVError)?; .change_context(errors::StorageError::KVError)?;

View File

@ -69,6 +69,7 @@ mod storage {
use super::{ReverseLookupInterface, Store}; use super::{ReverseLookupInterface, Store};
use crate::{ use crate::{
connection, connection,
core::errors::utils::RedisErrorExt,
errors::{self, CustomResult}, errors::{self, CustomResult},
types::storage::{ types::storage::{
enums, kv, enums, kv,
@ -109,7 +110,7 @@ mod storage {
format!("reverse_lookup_{}", &created_rev_lookup.lookup_id), format!("reverse_lookup_{}", &created_rev_lookup.lookup_id),
) )
.await .await
.change_context(errors::StorageError::KVError)? .map_err(|err| err.to_redis_failed_response(&created_rev_lookup.lookup_id))?
.try_into_setnx() .try_into_setnx()
{ {
Ok(SetnxReply::KeySet) => Ok(created_rev_lookup), Ok(SetnxReply::KeySet) => Ok(created_rev_lookup),

View File

@ -1,4 +1,7 @@
use crate::{core::errors, routes::metrics}; use crate::{
core::errors::{self, utils::RedisErrorExt},
routes::metrics,
};
/// Generates hscan field pattern. Suppose the field is pa_1234_ref_1211 it will generate /// Generates hscan field pattern. Suppose the field is pa_1234_ref_1211 it will generate
/// pa_1234_ref_* /// pa_1234_ref_*
@ -28,7 +31,8 @@ where
metrics::KV_MISS.add(&metrics::CONTEXT, 1, &[]); metrics::KV_MISS.add(&metrics::CONTEXT, 1, &[]);
database_call_closure().await database_call_closure().await
} }
_ => Err(redis_error.change_context(errors::StorageError::KVError)), // Keeping the key empty here since the error would never go here.
_ => Err(redis_error.to_redis_failed_response("")),
}, },
} }
} }

View File

@ -150,6 +150,26 @@ impl StorageError {
} }
} }
pub trait RedisErrorExt {
#[track_caller]
fn to_redis_failed_response(self, key: &str) -> error_stack::Report<DataStorageError>;
}
impl RedisErrorExt for error_stack::Report<RedisError> {
fn to_redis_failed_response(self, key: &str) -> error_stack::Report<DataStorageError> {
match self.current_context() {
RedisError::NotFound => self.change_context(DataStorageError::ValueNotFound(format!(
"Data does not exist for key {key}",
))),
RedisError::SetNxFailed => self.change_context(DataStorageError::DuplicateValue {
entity: "redis",
key: Some(key.to_string()),
}),
_ => self.change_context(DataStorageError::KVError),
}
}
}
impl_error_type!(EncryptionError, "Encryption error"); impl_error_type!(EncryptionError, "Encryption error");
#[derive(Debug, thiserror::Error)] #[derive(Debug, thiserror::Error)]

View File

@ -11,6 +11,7 @@ use redis_interface::SetnxReply;
use crate::{ use crate::{
diesel_error_to_data_error, diesel_error_to_data_error,
errors::RedisErrorExt,
redis::kv_store::{kv_wrapper, KvOperation}, redis::kv_store::{kv_wrapper, KvOperation},
utils::{self, try_redis_get_else_try_database_get}, utils::{self, try_redis_get_else_try_database_get},
DatabaseStore, KVRouterStore, RouterStore, DatabaseStore, KVRouterStore, RouterStore,
@ -97,7 +98,7 @@ impl<T: DatabaseStore> ReverseLookupInterface for KVRouterStore<T> {
format!("reverse_lookup_{}", &created_rev_lookup.lookup_id), format!("reverse_lookup_{}", &created_rev_lookup.lookup_id),
) )
.await .await
.change_context(errors::StorageError::KVError)? .map_err(|err| err.to_redis_failed_response(&created_rev_lookup.lookup_id))?
.try_into_setnx() .try_into_setnx()
{ {
Ok(SetnxReply::KeySet) => Ok(created_rev_lookup), Ok(SetnxReply::KeySet) => Ok(created_rev_lookup),

View File

@ -29,6 +29,7 @@ use router_env::{instrument, tracing};
use crate::{ use crate::{
diesel_error_to_data_error, diesel_error_to_data_error,
errors::RedisErrorExt,
lookup::ReverseLookupInterface, lookup::ReverseLookupInterface,
redis::kv_store::{kv_wrapper, KvOperation}, redis::kv_store::{kv_wrapper, KvOperation},
utils::{pg_connection_read, pg_connection_write, try_redis_get_else_try_database_get}, utils::{pg_connection_read, pg_connection_write, try_redis_get_else_try_database_get},
@ -423,7 +424,7 @@ impl<T: DatabaseStore> PaymentAttemptInterface for KVRouterStore<T> {
&key, &key,
) )
.await .await
.change_context(errors::StorageError::KVError)? .map_err(|err| err.to_redis_failed_response(&key))?
.try_into_hsetnx() .try_into_hsetnx()
{ {
Ok(HsetnxReply::KeyNotSet) => Err(errors::StorageError::DuplicateValue { Ok(HsetnxReply::KeyNotSet) => Err(errors::StorageError::DuplicateValue {

View File

@ -38,6 +38,7 @@ use router_env::{instrument, tracing};
use crate::connection; use crate::connection;
use crate::{ use crate::{
diesel_error_to_data_error, diesel_error_to_data_error,
errors::RedisErrorExt,
redis::kv_store::{kv_wrapper, KvOperation}, redis::kv_store::{kv_wrapper, KvOperation},
utils::{self, pg_connection_read, pg_connection_write}, utils::{self, pg_connection_read, pg_connection_write},
DataModelExt, DatabaseStore, KVRouterStore, DataModelExt, DatabaseStore, KVRouterStore,
@ -117,7 +118,7 @@ impl<T: DatabaseStore> PaymentIntentInterface for KVRouterStore<T> {
&key, &key,
) )
.await .await
.change_context(StorageError::KVError)? .map_err(|err| err.to_redis_failed_response(&key))?
.try_into_hsetnx() .try_into_hsetnx()
{ {
Ok(HsetnxReply::KeyNotSet) => Err(StorageError::DuplicateValue { Ok(HsetnxReply::KeyNotSet) => Err(StorageError::DuplicateValue {
@ -178,7 +179,7 @@ impl<T: DatabaseStore> PaymentIntentInterface for KVRouterStore<T> {
&key, &key,
) )
.await .await
.change_context(StorageError::KVError)? .map_err(|err| err.to_redis_failed_response(&key))?
.try_into_hset() .try_into_hset()
.change_context(StorageError::KVError)?; .change_context(StorageError::KVError)?;

View File

@ -3,7 +3,7 @@ use data_models::errors::StorageError;
use diesel::PgConnection; use diesel::PgConnection;
use error_stack::{IntoReport, ResultExt}; use error_stack::{IntoReport, ResultExt};
use crate::{metrics, DatabaseStore}; use crate::{errors::RedisErrorExt, metrics, DatabaseStore};
pub async fn pg_connection_read<T: DatabaseStore>( pub async fn pg_connection_read<T: DatabaseStore>(
store: &T, store: &T,
@ -64,7 +64,8 @@ where
metrics::KV_MISS.add(&metrics::CONTEXT, 1, &[]); metrics::KV_MISS.add(&metrics::CONTEXT, 1, &[]);
database_call_closure().await database_call_closure().await
} }
_ => Err(redis_error.change_context(StorageError::KVError)), // Keeping the key empty here since the error would never go here.
_ => Err(redis_error.to_redis_failed_response("")),
}, },
} }
} }