From e044c2fd9a4464e59ffc372b9333af6acbc9809a Mon Sep 17 00:00:00 2001 From: Prasunna Soppa <70575890+prasunna09@users.noreply.github.com> Date: Tue, 16 May 2023 13:29:05 +0530 Subject: [PATCH] refactor(connector): update error handling for Paypal, Checkout, Mollie to include detailed messages (#1150) --- crates/router/src/connector/checkout.rs | 13 ++-- crates/router/src/connector/mollie.rs | 7 ++- crates/router/src/connector/paypal.rs | 63 ++++++++----------- crates/router/tests/connectors/checkout.rs | 33 ++-------- crates/router/tests/connectors/paypal.rs | 58 +++++++++-------- .../router/tests/connectors/sample_auth.toml | 4 +- 6 files changed, 75 insertions(+), 103 deletions(-) diff --git a/crates/router/src/connector/checkout.rs b/crates/router/src/connector/checkout.rs index 707e4360a7..8fab1be96e 100644 --- a/crates/router/src/connector/checkout.rs +++ b/crates/router/src/connector/checkout.rs @@ -92,17 +92,18 @@ impl ConnectorCommon for Checkout { .parse_struct("ErrorResponse") .change_context(errors::ConnectorError::ResponseDeserializationFailed)? }; - Ok(types::ErrorResponse { status_code: res.status_code, code: response - .error_codes - .unwrap_or_else(|| vec![consts::NO_ERROR_CODE.to_string()]) - .join(" & "), - message: response .error_type + .clone() + .unwrap_or_else(|| consts::NO_ERROR_CODE.to_string()), + message: response + .error_codes + .as_ref() + .and_then(|error_codes| error_codes.first().cloned()) .unwrap_or_else(|| consts::NO_ERROR_MESSAGE.to_string()), - reason: None, + reason: response.error_codes.map(|errors| errors.join(" & ")), }) } } diff --git a/crates/router/src/connector/mollie.rs b/crates/router/src/connector/mollie.rs index 2006be4742..1518eefcd9 100644 --- a/crates/router/src/connector/mollie.rs +++ b/crates/router/src/connector/mollie.rs @@ -7,6 +7,7 @@ use transformers as mollie; use crate::{ configs::settings, + consts, core::{ errors::{self, CustomResult}, payments, @@ -82,10 +83,12 @@ impl ConnectorCommon for Mollie { .parse_struct("MollieErrorResponse") .change_context(errors::ConnectorError::ResponseDeserializationFailed)?; Ok(ErrorResponse { + status_code: response.status, + code: response + .title + .unwrap_or_else(|| consts::NO_ERROR_CODE.to_string()), message: response.detail, reason: response.field, - status_code: response.status, - ..Default::default() }) } } diff --git a/crates/router/src/connector/paypal.rs b/crates/router/src/connector/paypal.rs index b6bb325400..95ffb69bb8 100644 --- a/crates/router/src/connector/paypal.rs +++ b/crates/router/src/connector/paypal.rs @@ -71,37 +71,32 @@ impl Paypal { .parse_struct("Paypal ErrorResponse") .change_context(errors::ConnectorError::ResponseDeserializationFailed)?; - let message = match response.details { - Some(mes) => { - let mut des = "".to_owned(); - for item in mes.iter() { - let mut description = format!("description - {}", item.to_owned().description); - - if let Some(data) = &item.value { - description.push_str(format!(", value - {}", data.to_owned()).as_str()); + let error_reason = match response.details { + Some(order_errors) => order_errors + .iter() + .map(|error| { + let mut reason = format!("description - {}", error.description); + if let Some(value) = &error.value { + reason.push_str(&format!(", value - {value}")); } - - if let Some(data) = &item.field { - let field = data - .clone() - .split('/') - .last() - .unwrap_or_default() - .to_owned(); - - description.push_str(format!(", field - {};", field).as_str()); + if let Some(field) = error + .field + .as_ref() + .and_then(|field| field.split('/').last()) + { + reason.push_str(&format!(", field - {field}")); } - des.push_str(description.as_str()) - } - des - } + reason.push(';'); + reason + }) + .collect::(), None => consts::NO_ERROR_MESSAGE.to_string(), }; Ok(ErrorResponse { status_code: res.status_code, code: response.name, - message, - reason: None, + message: response.message, + reason: Some(error_reason), }) } } @@ -168,24 +163,18 @@ impl ConnectorCommon for Paypal { .parse_struct("Paypal ErrorResponse") .change_context(errors::ConnectorError::ResponseDeserializationFailed)?; - let message = match response.details { - Some(mes) => { - let mut des = "".to_owned(); - for item in mes.iter() { - let x = item.clone().description; - let st = format!("description - {} ; ", x); - des.push_str(&st); - } - des - } + let error_reason = match response.details { + Some(error_details) => error_details + .iter() + .map(|error| format!("description - {} ; ", error.description)) + .collect::(), None => consts::NO_ERROR_MESSAGE.to_string(), }; - Ok(ErrorResponse { status_code: res.status_code, code: response.name, - message, - reason: None, + message: response.message, + reason: Some(error_reason), }) } } diff --git a/crates/router/tests/connectors/checkout.rs b/crates/router/tests/connectors/checkout.rs index 56ef70ab9a..bd11a43236 100644 --- a/crates/router/tests/connectors/checkout.rs +++ b/crates/router/tests/connectors/checkout.rs @@ -1,6 +1,3 @@ -use std::str::FromStr; - -use cards::CardNumber; use masking::Secret; use router::types::{self, api, storage::enums}; @@ -313,28 +310,6 @@ async fn should_sync_refund() { } // Cards Negative scenerios -// Creates a payment with incorrect card number. -#[serial_test::serial] -#[actix_web::test] -async fn should_fail_payment_for_incorrect_card_number() { - let response = CONNECTOR - .make_payment( - Some(types::PaymentsAuthorizeData { - payment_method_data: types::api::PaymentMethodData::Card(api::Card { - card_number: CardNumber::from_str("1234567891011").unwrap(), - ..utils::CCardType::default().0 - }), - ..utils::PaymentAuthorizeType::default().0 - }), - get_default_payment_info(), - ) - .await - .unwrap(); - assert_eq!( - response.response.unwrap_err().code, - "card_number_invalid".to_string(), - ); -} // Creates a payment with incorrect CVC. #[serial_test::serial] @@ -354,7 +329,7 @@ async fn should_fail_payment_for_incorrect_cvc() { .await .unwrap(); assert_eq!( - response.response.unwrap_err().code, + response.response.unwrap_err().message, "cvv_invalid".to_string(), ); } @@ -377,7 +352,7 @@ async fn should_fail_payment_for_invalid_exp_month() { .await .unwrap(); assert_eq!( - response.response.unwrap_err().code, + response.response.unwrap_err().message, "card_expiry_month_invalid".to_string(), ); } @@ -400,7 +375,7 @@ async fn should_fail_payment_for_incorrect_expiry_year() { .await .unwrap(); assert_eq!( - response.response.unwrap_err().code, + response.response.unwrap_err().message, "card_expired".to_string(), ); } @@ -450,7 +425,7 @@ async fn should_fail_for_refund_amount_higher_than_payment_amount() { .await .unwrap(); assert_eq!( - response.response.unwrap_err().code, + response.response.unwrap_err().message, "refund_amount_exceeds_balance", ); } diff --git a/crates/router/tests/connectors/paypal.rs b/crates/router/tests/connectors/paypal.rs index d804dd21c7..1fbd031314 100644 --- a/crates/router/tests/connectors/paypal.rs +++ b/crates/router/tests/connectors/paypal.rs @@ -428,27 +428,6 @@ async fn should_sync_refund() { } // Cards Negative scenerios -// Creates a payment with incorrect card number. -#[actix_web::test] -async fn should_fail_payment_for_incorrect_card_number() { - let response = CONNECTOR - .make_payment( - Some(types::PaymentsAuthorizeData { - payment_method_data: types::api::PaymentMethodData::Card(api::Card { - card_number: cards::CardNumber::from_str("1234567891011").unwrap(), - ..utils::CCardType::default().0 - }), - ..utils::PaymentAuthorizeType::default().0 - }), - get_default_payment_info(), - ) - .await - .unwrap(); - assert_eq!( - response.response.unwrap_err().message, - "description - UNPROCESSABLE_ENTITY", - ); -} // Creates a payment with incorrect CVC. #[actix_web::test] @@ -467,7 +446,11 @@ async fn should_fail_payment_for_incorrect_cvc() { .await .unwrap(); assert_eq!( - response.response.unwrap_err().message, + response.response.clone().unwrap_err().message, + "Request is not well-formed, syntactically incorrect, or violates schema.", + ); + assert_eq!( + response.response.unwrap_err().reason.unwrap(), "description - The value of a field does not conform to the expected format., value - 12345, field - security_code;", ); } @@ -489,7 +472,11 @@ async fn should_fail_payment_for_invalid_exp_month() { .await .unwrap(); assert_eq!( - response.response.unwrap_err().message, + response.response.clone().unwrap_err().message, + "Request is not well-formed, syntactically incorrect, or violates schema.", + ); + assert_eq!( + response.response.unwrap_err().reason.unwrap(), "description - The value of a field does not conform to the expected format., value - 2025-20, field - expiry;", ); } @@ -511,7 +498,11 @@ async fn should_fail_payment_for_incorrect_expiry_year() { .await .unwrap(); assert_eq!( - response.response.unwrap_err().message, + response.response.clone().unwrap_err().message, + "The requested action could not be performed, semantically incorrect, or failed business validation.", + ); + assert_eq!( + response.response.unwrap_err().reason.unwrap(), "description - The card is expired., field - expiry;", ); } @@ -551,7 +542,11 @@ async fn should_fail_void_payment_for_auto_capture() { .await .expect("Void payment response"); assert_eq!( - void_response.response.unwrap_err().message, + void_response.response.clone().unwrap_err().message, + "The requested action could not be performed, semantically incorrect, or failed business validation." + ); + assert_eq!( + void_response.response.unwrap_err().reason.unwrap(), "description - Authorization has been previously captured and hence cannot be voided. ; " ); } @@ -576,7 +571,11 @@ async fn should_fail_capture_for_invalid_payment() { .await .unwrap(); assert_eq!( - capture_response.response.unwrap_err().message, + capture_response.response.clone().unwrap_err().message, + "The specified resource does not exist.", + ); + assert_eq!( + capture_response.response.unwrap_err().reason.unwrap(), "description - Specified resource ID does not exist. Please check the resource ID and try again. ; ", ); } @@ -600,7 +599,12 @@ async fn should_fail_for_refund_amount_higher_than_payment_amount() { ) .await .unwrap(); - assert_eq!(&response.response.unwrap_err().message, "description - The refund amount must be less than or equal to the capture amount that has not yet been refunded. ; "); + assert_eq!(&response.response.clone().unwrap_err().message, "The requested action could not be performed, semantically incorrect, or failed business validation."); + + assert_eq!( + response.response.unwrap_err().reason.unwrap(), + "description - The refund amount must be less than or equal to the capture amount that has not yet been refunded. ; ", + ); } // Connector dependent test cases goes here diff --git a/crates/router/tests/connectors/sample_auth.toml b/crates/router/tests/connectors/sample_auth.toml index 146cfeed62..1e4ca9ef18 100644 --- a/crates/router/tests/connectors/sample_auth.toml +++ b/crates/router/tests/connectors/sample_auth.toml @@ -14,8 +14,8 @@ api_key = "MyMerchantName" key1 = "MyTransactionKey" [checkout] -api_key = "Bearer PublicKey" -api_secret = "Bearer SecretKey" +api_key = "PublicKey" +api_secret = "SecretKey" key1 = "MyProcessingChannelId" [cybersource]