refactor(disputes): resolve incorrect 5xx error mappings for disputes (#1360)

Co-authored-by: Kritik Modi <61862301+kritikmodi@users.noreply.github.com>
Co-authored-by: kritikmodi <kritik.modi@juspay.in>
Co-authored-by: Sai Harsha Vardhan <56996463+sai-harsha-vardhan@users.noreply.github.com>
This commit is contained in:
Abhishek Marrivagu
2023-06-14 18:31:31 +05:30
committed by GitHub
parent 5535159d5c
commit c9b400e186
4 changed files with 53 additions and 6 deletions

View File

@ -40,6 +40,9 @@ pub enum StripeErrorCode {
#[error(error_type = StripeErrorType::ApiError, code = "payment_intent_payment_attempt_failed", message = "Capture attempt failed while processing with connector.")] #[error(error_type = StripeErrorType::ApiError, code = "payment_intent_payment_attempt_failed", message = "Capture attempt failed while processing with connector.")]
PaymentIntentPaymentAttemptFailed { data: Option<serde_json::Value> }, PaymentIntentPaymentAttemptFailed { data: Option<serde_json::Value> },
#[error(error_type = StripeErrorType::ApiError, code = "dispute_failure", message = "Dispute failed while processing with connector. Retry operation.")]
DisputeFailed { data: Option<serde_json::Value> },
#[error(error_type = StripeErrorType::CardError, code = "expired_card", message = "Card Expired. Please use another card")] #[error(error_type = StripeErrorType::CardError, code = "expired_card", message = "Card Expired. Please use another card")]
ExpiredCard, ExpiredCard,
@ -405,6 +408,7 @@ impl From<errors::ApiErrorResponse> for StripeErrorCode {
errors::ApiErrorResponse::PaymentCaptureFailed { data } => { errors::ApiErrorResponse::PaymentCaptureFailed { data } => {
Self::PaymentIntentPaymentAttemptFailed { data } Self::PaymentIntentPaymentAttemptFailed { data }
} }
errors::ApiErrorResponse::DisputeFailed { data } => Self::DisputeFailed { data },
errors::ApiErrorResponse::InvalidCardData { data } => Self::InvalidCardType, // Maybe it is better to de generalize this router error errors::ApiErrorResponse::InvalidCardData { data } => Self::InvalidCardType, // Maybe it is better to de generalize this router error
errors::ApiErrorResponse::CardExpired { data } => Self::ExpiredCard, errors::ApiErrorResponse::CardExpired { data } => Self::ExpiredCard,
errors::ApiErrorResponse::RefundNotPossible { connector } => Self::RefundFailed, errors::ApiErrorResponse::RefundNotPossible { connector } => Self::RefundFailed,
@ -548,6 +552,7 @@ impl actix_web::ResponseError for StripeErrorCode {
| Self::DuplicatePaymentMethod | Self::DuplicatePaymentMethod
| Self::PaymentFailed | Self::PaymentFailed
| Self::VerificationFailed { .. } | Self::VerificationFailed { .. }
| Self::DisputeFailed { .. }
| Self::MaximumRefundCount | Self::MaximumRefundCount
| Self::PaymentIntentInvalidParameter { .. } | Self::PaymentIntentInvalidParameter { .. }
| Self::SerdeQsError { .. } | Self::SerdeQsError { .. }

View File

@ -5,7 +5,7 @@ use router_env::{instrument, tracing};
pub mod transformers; pub mod transformers;
use super::{ use super::{
errors::{self, RouterResponse, StorageErrorExt}, errors::{self, ConnectorErrorExt, RouterResponse, StorageErrorExt},
metrics, metrics,
}; };
use crate::{ use crate::{
@ -129,7 +129,7 @@ pub async fn accept_dispute(
payments::CallConnectorAction::Trigger, payments::CallConnectorAction::Trigger,
) )
.await .await
.change_context(errors::ApiErrorResponse::InternalServerError) .map_err(|error| error.to_dispute_failed_response())
.attach_printable("Failed while calling accept dispute connector api")?; .attach_printable("Failed while calling accept dispute connector api")?;
let accept_dispute_response = let accept_dispute_response =
response response
@ -236,7 +236,7 @@ pub async fn submit_evidence(
payments::CallConnectorAction::Trigger, payments::CallConnectorAction::Trigger,
) )
.await .await
.change_context(errors::ApiErrorResponse::InternalServerError) .map_err(|error| error.to_payment_failed_response())
.attach_printable("Failed while calling submit evidence connector api")?; .attach_printable("Failed while calling submit evidence connector api")?;
let submit_evidence_response = let submit_evidence_response =
response response
@ -272,7 +272,7 @@ pub async fn submit_evidence(
payments::CallConnectorAction::Trigger, payments::CallConnectorAction::Trigger,
) )
.await .await
.change_context(errors::ApiErrorResponse::InternalServerError) .map_err(|error| error.to_payment_failed_response())
.attach_printable("Failed while calling defend dispute connector api")?; .attach_printable("Failed while calling defend dispute connector api")?;
let defend_dispute_response = defend_response.response.map_err(|err| { let defend_dispute_response = defend_response.response.map_err(|err| {
errors::ApiErrorResponse::ExternalConnectorError { errors::ApiErrorResponse::ExternalConnectorError {
@ -300,7 +300,9 @@ pub async fn submit_evidence(
let updated_dispute = db let updated_dispute = db
.update_dispute(dispute.clone(), update_dispute) .update_dispute(dispute.clone(), update_dispute)
.await .await
.change_context(errors::ApiErrorResponse::InternalServerError) .to_not_found_response(errors::ApiErrorResponse::DisputeNotFound {
dispute_id: dispute_id.to_owned(),
})
.attach_printable_lazy(|| { .attach_printable_lazy(|| {
format!("Unable to update dispute with dispute_id: {dispute_id}") format!("Unable to update dispute with dispute_id: {dispute_id}")
})?; })?;
@ -373,7 +375,9 @@ pub async fn attach_evidence(
}; };
db.update_dispute(dispute, update_dispute) db.update_dispute(dispute, update_dispute)
.await .await
.change_context(errors::ApiErrorResponse::InternalServerError) .to_not_found_response(errors::ApiErrorResponse::DisputeNotFound {
dispute_id: dispute_id.to_owned(),
})
.attach_printable_lazy(|| { .attach_printable_lazy(|| {
format!("Unable to update dispute with dispute_id: {dispute_id}") format!("Unable to update dispute with dispute_id: {dispute_id}")
})?; })?;

View File

@ -111,6 +111,8 @@ pub enum ApiErrorResponse {
RefundFailed { data: Option<serde_json::Value> }, RefundFailed { data: Option<serde_json::Value> },
#[error(error_type = ErrorType::ProcessingError, code = "CE_07", message = "Verification failed while processing with connector. Retry operation")] #[error(error_type = ErrorType::ProcessingError, code = "CE_07", message = "Verification failed while processing with connector. Retry operation")]
VerificationFailed { data: Option<serde_json::Value> }, VerificationFailed { data: Option<serde_json::Value> },
#[error(error_type = ErrorType::ProcessingError, code = "CE_08", message = "Dispute operation failed while processing with connector. Retry operation")]
DisputeFailed { data: Option<serde_json::Value> },
#[error(error_type = ErrorType::ServerNotAvailable, code = "HE_00", message = "Something went wrong")] #[error(error_type = ErrorType::ServerNotAvailable, code = "HE_00", message = "Something went wrong")]
InternalServerError, InternalServerError,
@ -262,6 +264,7 @@ impl actix_web::ResponseError for ApiErrorResponse {
| Self::VerificationFailed { .. } | Self::VerificationFailed { .. }
| Self::PaymentUnexpectedState { .. } | Self::PaymentUnexpectedState { .. }
| Self::MandateValidationFailed { .. } | Self::MandateValidationFailed { .. }
| Self::DisputeFailed { .. }
| Self::RefundAmountExceedsPaymentAmount | Self::RefundAmountExceedsPaymentAmount
| Self::MaximumRefundCount | Self::MaximumRefundCount
| Self::IncorrectPaymentMethodConfiguration | Self::IncorrectPaymentMethodConfiguration
@ -428,6 +431,9 @@ impl common_utils::errors::ErrorSwitch<api_models::errors::types::ApiErrorRespon
Self::PaymentCaptureFailed { data } => { Self::PaymentCaptureFailed { data } => {
AER::BadRequest(ApiError::new("CE", 3, "Capture attempt failed while processing with connector", Some(Extra { data: data.clone(), ..Default::default()}))) AER::BadRequest(ApiError::new("CE", 3, "Capture attempt failed while processing with connector", Some(Extra { data: data.clone(), ..Default::default()})))
} }
Self::DisputeFailed { data } => {
AER::BadRequest(ApiError::new("CE", 1, "Dispute operation failed while processing with connector. Retry operation", Some(Extra { data: data.clone(), ..Default::default()})))
}
Self::InvalidCardData { data } => AER::BadRequest(ApiError::new("CE", 4, "The card data is invalid", Some(Extra { data: data.clone(), ..Default::default()}))), Self::InvalidCardData { data } => AER::BadRequest(ApiError::new("CE", 4, "The card data is invalid", Some(Extra { data: data.clone(), ..Default::default()}))),
Self::CardExpired { data } => AER::BadRequest(ApiError::new("CE", 5, "The card has expired", Some(Extra { data: data.clone(), ..Default::default()}))), Self::CardExpired { data } => AER::BadRequest(ApiError::new("CE", 5, "The card has expired", Some(Extra { data: data.clone(), ..Default::default()}))),
Self::RefundFailed { data } => AER::BadRequest(ApiError::new("CE", 6, "Refund failed while processing with connector. Retry refund", Some(Extra { data: data.clone(), ..Default::default()}))), Self::RefundFailed { data } => AER::BadRequest(ApiError::new("CE", 6, "Refund failed while processing with connector. Retry refund", Some(Extra { data: data.clone(), ..Default::default()}))),

View File

@ -50,6 +50,8 @@ pub trait ConnectorErrorExt {
fn to_payment_failed_response(self) -> error_stack::Report<errors::ApiErrorResponse>; fn to_payment_failed_response(self) -> error_stack::Report<errors::ApiErrorResponse>;
#[track_caller] #[track_caller]
fn to_verify_failed_response(self) -> error_stack::Report<errors::ApiErrorResponse>; fn to_verify_failed_response(self) -> error_stack::Report<errors::ApiErrorResponse>;
#[track_caller]
fn to_dispute_failed_response(self) -> error_stack::Report<errors::ApiErrorResponse>;
} }
impl ConnectorErrorExt for error_stack::Report<errors::ConnectorError> { impl ConnectorErrorExt for error_stack::Report<errors::ConnectorError> {
@ -149,6 +151,36 @@ impl ConnectorErrorExt for error_stack::Report<errors::ConnectorError> {
}; };
self.change_context(data) self.change_context(data)
} }
fn to_dispute_failed_response(self) -> error_stack::Report<errors::ApiErrorResponse> {
let error = match self.current_context() {
errors::ConnectorError::ProcessingStepFailed(Some(bytes)) => {
let response_str = std::str::from_utf8(bytes);
let data = match response_str {
Ok(s) => serde_json::from_str(s)
.map_err(
|error| logger::error!(%error,"Failed to convert response to JSON"),
)
.ok(),
Err(error) => {
logger::error!(%error,"Failed to convert response to UTF8 string");
None
}
};
errors::ApiErrorResponse::DisputeFailed { data }
}
errors::ConnectorError::MissingRequiredField { field_name } => {
errors::ApiErrorResponse::MissingRequiredField { field_name }
}
errors::ConnectorError::MissingRequiredFields { field_names } => {
errors::ApiErrorResponse::MissingRequiredFields {
field_names: field_names.to_vec(),
}
}
_ => errors::ApiErrorResponse::InternalServerError,
};
self.change_context(error)
}
} }
pub trait RedisErrorExt { pub trait RedisErrorExt {