From c9b400e186731b7de6073fece662fd0fcbbfc953 Mon Sep 17 00:00:00 2001 From: Abhishek Marrivagu <68317979+Abhicodes-crypto@users.noreply.github.com> Date: Wed, 14 Jun 2023 18:31:31 +0530 Subject: [PATCH] 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 Co-authored-by: Sai Harsha Vardhan <56996463+sai-harsha-vardhan@users.noreply.github.com> --- .../router/src/compatibility/stripe/errors.rs | 5 +++ crates/router/src/core/disputes.rs | 16 ++++++---- .../src/core/errors/api_error_response.rs | 6 ++++ crates/router/src/core/errors/utils.rs | 32 +++++++++++++++++++ 4 files changed, 53 insertions(+), 6 deletions(-) diff --git a/crates/router/src/compatibility/stripe/errors.rs b/crates/router/src/compatibility/stripe/errors.rs index c68f56d70f..bb0fe46b7e 100644 --- a/crates/router/src/compatibility/stripe/errors.rs +++ b/crates/router/src/compatibility/stripe/errors.rs @@ -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.")] PaymentIntentPaymentAttemptFailed { data: Option }, + #[error(error_type = StripeErrorType::ApiError, code = "dispute_failure", message = "Dispute failed while processing with connector. Retry operation.")] + DisputeFailed { data: Option }, + #[error(error_type = StripeErrorType::CardError, code = "expired_card", message = "Card Expired. Please use another card")] ExpiredCard, @@ -405,6 +408,7 @@ impl From for StripeErrorCode { errors::ApiErrorResponse::PaymentCaptureFailed { 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::CardExpired { data } => Self::ExpiredCard, errors::ApiErrorResponse::RefundNotPossible { connector } => Self::RefundFailed, @@ -548,6 +552,7 @@ impl actix_web::ResponseError for StripeErrorCode { | Self::DuplicatePaymentMethod | Self::PaymentFailed | Self::VerificationFailed { .. } + | Self::DisputeFailed { .. } | Self::MaximumRefundCount | Self::PaymentIntentInvalidParameter { .. } | Self::SerdeQsError { .. } diff --git a/crates/router/src/core/disputes.rs b/crates/router/src/core/disputes.rs index 78c8941f70..37387d6e7f 100644 --- a/crates/router/src/core/disputes.rs +++ b/crates/router/src/core/disputes.rs @@ -5,7 +5,7 @@ use router_env::{instrument, tracing}; pub mod transformers; use super::{ - errors::{self, RouterResponse, StorageErrorExt}, + errors::{self, ConnectorErrorExt, RouterResponse, StorageErrorExt}, metrics, }; use crate::{ @@ -129,7 +129,7 @@ pub async fn accept_dispute( payments::CallConnectorAction::Trigger, ) .await - .change_context(errors::ApiErrorResponse::InternalServerError) + .map_err(|error| error.to_dispute_failed_response()) .attach_printable("Failed while calling accept dispute connector api")?; let accept_dispute_response = response @@ -236,7 +236,7 @@ pub async fn submit_evidence( payments::CallConnectorAction::Trigger, ) .await - .change_context(errors::ApiErrorResponse::InternalServerError) + .map_err(|error| error.to_payment_failed_response()) .attach_printable("Failed while calling submit evidence connector api")?; let submit_evidence_response = response @@ -272,7 +272,7 @@ pub async fn submit_evidence( payments::CallConnectorAction::Trigger, ) .await - .change_context(errors::ApiErrorResponse::InternalServerError) + .map_err(|error| error.to_payment_failed_response()) .attach_printable("Failed while calling defend dispute connector api")?; let defend_dispute_response = defend_response.response.map_err(|err| { errors::ApiErrorResponse::ExternalConnectorError { @@ -300,7 +300,9 @@ pub async fn submit_evidence( let updated_dispute = db .update_dispute(dispute.clone(), update_dispute) .await - .change_context(errors::ApiErrorResponse::InternalServerError) + .to_not_found_response(errors::ApiErrorResponse::DisputeNotFound { + dispute_id: dispute_id.to_owned(), + }) .attach_printable_lazy(|| { 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) .await - .change_context(errors::ApiErrorResponse::InternalServerError) + .to_not_found_response(errors::ApiErrorResponse::DisputeNotFound { + dispute_id: dispute_id.to_owned(), + }) .attach_printable_lazy(|| { format!("Unable to update dispute with dispute_id: {dispute_id}") })?; diff --git a/crates/router/src/core/errors/api_error_response.rs b/crates/router/src/core/errors/api_error_response.rs index 4238a53092..cd637e96c4 100644 --- a/crates/router/src/core/errors/api_error_response.rs +++ b/crates/router/src/core/errors/api_error_response.rs @@ -111,6 +111,8 @@ pub enum ApiErrorResponse { RefundFailed { data: Option }, #[error(error_type = ErrorType::ProcessingError, code = "CE_07", message = "Verification failed while processing with connector. Retry operation")] VerificationFailed { data: Option }, + #[error(error_type = ErrorType::ProcessingError, code = "CE_08", message = "Dispute operation failed while processing with connector. Retry operation")] + DisputeFailed { data: Option }, #[error(error_type = ErrorType::ServerNotAvailable, code = "HE_00", message = "Something went wrong")] InternalServerError, @@ -262,6 +264,7 @@ impl actix_web::ResponseError for ApiErrorResponse { | Self::VerificationFailed { .. } | Self::PaymentUnexpectedState { .. } | Self::MandateValidationFailed { .. } + | Self::DisputeFailed { .. } | Self::RefundAmountExceedsPaymentAmount | Self::MaximumRefundCount | Self::IncorrectPaymentMethodConfiguration @@ -428,6 +431,9 @@ impl common_utils::errors::ErrorSwitch { 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::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()}))), diff --git a/crates/router/src/core/errors/utils.rs b/crates/router/src/core/errors/utils.rs index 25fa8a5664..ae4ab86943 100644 --- a/crates/router/src/core/errors/utils.rs +++ b/crates/router/src/core/errors/utils.rs @@ -50,6 +50,8 @@ pub trait ConnectorErrorExt { fn to_payment_failed_response(self) -> error_stack::Report; #[track_caller] fn to_verify_failed_response(self) -> error_stack::Report; + #[track_caller] + fn to_dispute_failed_response(self) -> error_stack::Report; } impl ConnectorErrorExt for error_stack::Report { @@ -149,6 +151,36 @@ impl ConnectorErrorExt for error_stack::Report { }; self.change_context(data) } + + fn to_dispute_failed_response(self) -> error_stack::Report { + 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 {