fix(router): fix retry_count and add validation for process_tracker #7614

AdityaKumaar21 · 2025-03-25T06:33:44Z

Type of Change

Description

This PR will fix the bug where the retry_count was stuck at 0 and will add validation for entry in process_tracker only when payment_retry_count exceeds the billing_connector_retry_threshold.

Additional Changes

This PR modifies the API contract
This PR modifies the database schema
This PR modifies application configuration/environment variables

Motivation and Context

Earlier the retry_count was stuck at 0 and was not increasing, since updated payment intent was not being passing, which i have fixed in this PR

How did you test it?

tested it from stripe dashboard

Checklist

I formatted the code cargo +nightly fmt --all
I addressed lints thrown by cargo clippy
I reviewed the submitted code
I added unit tests for my changes where possible

semanticdiff-com · 2025-03-25T06:33:47Z

Review changes with

Changed Files

File	Status
crates/router/src/core/revenue_recovery.rs	85% smaller
crates/common_enums/src/enums.rs	78% smaller
crates/hyperswitch_domain_models/src/payments/payment_intent.rs	73% smaller
crates/router/src/core/webhooks/recovery_incoming.rs	62% smaller
crates/hyperswitch_domain_models/src/router_data.rs	50% smaller
crates/openapi/src/openapi_v2.rs	13% smaller
api-reference-v2/openapi_spec.json	10% smaller
crates/api_models/src/payments.rs	0% smaller
crates/hyperswitch_domain_models/src/lib.rs	0% smaller
crates/hyperswitch_domain_models/src/merchant_connector_account.rs	0% smaller
crates/hyperswitch_domain_models/src/payments.rs	0% smaller
crates/hyperswitch_domain_models/src/revenue_recovery.rs	0% smaller
crates/router/src/core/errors.rs	0% smaller
crates/router/src/core/payments.rs	0% smaller
crates/router/src/core/payments/operations/payment_attempt_record.rs	0% smaller
crates/router/src/core/payments/transformers.rs	0% smaller

…ttempt ID handling

Aprabhat19 · 2025-03-26T08:02:16Z

crates/hyperswitch_domain_models/src/payments.rs

@@ -811,9 +811,9 @@ where
                total_retry_count: revenue_recovery
                    .as_ref()
                    .map_or(1, |data| (data.total_retry_count + 1)),
-                // Since this is an external system call, marking this payment_connector_transmission to ConnectorCallSucceeded.
+                // Since this is an external system call, marking this payment_connector_transmission to ConnectorCallUnsucceeded.
                payment_connector_transmission:


As discussed can you make this an Optional field and have ConnectorCallUnsucceeded as the default value

Aprabhat19 · 2025-03-26T09:35:15Z

crates/hyperswitch_domain_models/src/lib.rs

@@ -266,7 +266,7 @@ impl ApiModelToDieselModelConvertor<ApiRevenueRecoveryMetadata> for PaymentReven
    fn convert_from(from: ApiRevenueRecoveryMetadata) -> Self {
        Self {
            total_retry_count: from.total_retry_count,
-            payment_connector_transmission: from.payment_connector_transmission,
+            payment_connector_transmission: Some(from.payment_connector_transmission.unwrap_or_default()),


You can remove the unwrap_or_default here

Aprabhat19 · 2025-03-26T09:35:25Z

crates/hyperswitch_domain_models/src/lib.rs

@@ -281,7 +281,7 @@ impl ApiModelToDieselModelConvertor<ApiRevenueRecoveryMetadata> for PaymentReven
    fn convert_back(self) -> ApiRevenueRecoveryMetadata {
        ApiRevenueRecoveryMetadata {
            total_retry_count: self.total_retry_count,
-            payment_connector_transmission: self.payment_connector_transmission,
+            payment_connector_transmission: Some(self.payment_connector_transmission.unwrap_or_default()),


…el_model

srujanchikke · 2025-03-27T05:43:07Z

crates/api_models/src/payments.rs

@@ -1586,6 +1586,21 @@ pub struct PaymentAttemptResponse {
    pub feature_metadata: Option<PaymentAttemptFeatureMetadata>,
 }

+#[cfg(feature = "v2")]
+#[derive(Debug, serde::Serialize, Clone, ToSchema)]
+pub struct PaymentRecordResponse {


Suggested change

pub struct PaymentRecordResponse {

pub struct PaymentsAttemptRecordRequest {

srujanchikke · 2025-03-27T05:43:54Z

crates/common_enums/src/enums.rs

 #[serde(rename = "snake_case")]
 pub enum PaymentConnectorTransmission {
    /// Failed to call the payment connector
+    #[default]


why do we need this field as optional when we are already using this field as optional value ?

srujanchikke · 2025-03-27T05:47:06Z

crates/hyperswitch_domain_models/src/router_data.rs

+                // Since its an Error from the connector we should set it as connectors's set status or Failure by default
+                let attempt_status = attempt_status.unwrap_or(common_enums::AttemptStatus::Failure);


Why do we need to make it failure ? If connector doesn't update the status let it be the previous status. This might update it Psync process tracker right ?

srujanchikke · 2025-03-27T05:47:46Z

crates/router/src/core/payments/transformers.rs

+    fn foreign_from(feature_metadata: &diesel_models::types::FeatureMetadata) -> Self {
+        let revenue_recovery = feature_metadata
+            .payment_revenue_recovery_metadata
+            .clone()


can we pass this by reference ?

srujanchikke · 2025-03-27T05:48:10Z

crates/router/src/core/payments/transformers.rs

+        let revenue_recovery = feature_metadata
+            .payment_revenue_recovery_metadata
+            .clone()
+            .map(|r| api_models::payments::PaymentRevenueRecoveryMetadata {


Please avoid single letter variable names.

srujanchikke · 2025-03-27T05:50:20Z

crates/router/src/core/webhooks/recovery_incoming.rs

+                .as_ref()
+                .and_then(|metadata| metadata.get_retry_count())
+        })
+        .unwrap_or(0);


If we are supposed to get retry count at this point we better throw an error instead of setting the value to 0

srujanchikke · 2025-03-27T05:51:48Z

crates/router/src/core/webhooks/recovery_incoming.rs

+    if intent_retry_count <= mca_retry_threshold {
+        router_env::logger::error!(
+            "Payment retry count {} is less than threshold {}",
+            intent_retry_count,
+            mca_retry_threshold
+        );
+        return Ok(webhooks::WebhookResponseTracker::NoEffect);
+    }
+


Can we have this logic inside ScheduleFailedPayment and also use when instead of if

srujanchikke · 2025-03-27T05:56:00Z

crates/router/src/core/webhooks/recovery_incoming.rs

+        Option<(
+            revenue_recovery::RecoveryPaymentAttempt,
+            revenue_recovery::RecoveryPaymentIntent,
+        )>,


Instead of having the return type as Option<(Attempt,Intent)> can we make this to <(Option, intent)> ?

srujanchikke · 2025-03-27T05:56:38Z

crates/router/src/core/webhooks/recovery_incoming.rs

@@ -326,7 +363,9 @@ impl RevenueRecoveryAttempt {
                        attempt_status: attempt_res.status.to_owned(),
                        feature_metadata: attempt_res.feature_metadata.to_owned(),
                    });
-                Ok(payment_attempt)
+                // If we have an attempt, combine it with payment_intent in a tuple.
+                let combined = payment_attempt.map(|attempt| (attempt, (*payment_intent).clone()));


can we use better variable names here ?

introduce PaymentRecordResponse and enhance payment attempt handling

2c68a2f

AdityaKumaar21 requested review from a team as code owners March 25, 2025 06:33

hyperswitch-bot bot added 2 commits March 25, 2025 06:34

chore: run formatter

7944c5e

docs(openapi): re-generate OpenAPI specification

991e9c6

hyperswitch-bot bot added the M-api-contract-changes Metadata: This PR involves API contract changes label Mar 25, 2025

AdityaKumaar21 added the C-bug Category: Bug label Mar 25, 2025

AdityaKumaar21 self-assigned this Mar 25, 2025

AdityaKumaar21 requested review from srujanchikke and Aprabhat19 March 25, 2025 06:44

remove unnecessary comments in recovery_incoming_webhook_flow

729eb1d

AdityaKumaar21 added C-feature Category: Feature request or enhancement enhancement New feature or request and removed M-api-contract-changes Metadata: This PR involves API contract changes C-feature Category: Feature request or enhancement labels Mar 25, 2025

fix: update payment connector transmission status and refine active a…

00733b2

…ttempt ID handling

hyperswitch-bot bot added the M-api-contract-changes Metadata: This PR involves API contract changes label Mar 25, 2025

Merge branch 'main' into fix_retry_count

9c576c3

Aprabhat19 reviewed Mar 26, 2025

View reviewed changes

juspay deleted a comment from Aprabhat19 Mar 26, 2025

addressed comment

7042ac4

Aprabhat19 reviewed Mar 26, 2025

View reviewed changes

hyperswitch-bot bot and others added 6 commits March 26, 2025 09:44

docs(openapi): re-generate OpenAPI specification

460a1cd

chore: run formatter

b036e61

resolve comment

5972cc7

fix: update payment_connector_transmission to be non-optional in dies…

dcd70e1

…el_model

chore: run formatter

a0c5f99

spell check

7c238d6

chore: run formatter

646ec71

srujanchikke requested changes Mar 27, 2025

View reviewed changes

AdityaKumaar21 and others added 7 commits March 27, 2025 18:27

addressing comments

65c63e1

Merge branch 'main' into fix_retry_count

7d835be

chore: run formatter

00b17e9

docs(openapi): re-generate OpenAPI specification

5a9e208

resolve checks

2fba8fc

chore: run formatter

10d6106

Merge branch 'main' into fix_retry_count

0c6e9d7

srujanchikke mentioned this pull request Mar 28, 2025

feat(process_tracker): Invoke record back flow in PCR workflow [V2] #7660

Open

14 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(router): fix retry_count and add validation for process_tracker #7614

fix(router): fix retry_count and add validation for process_tracker #7614

AdityaKumaar21 commented Mar 25, 2025 •

edited

Loading

semanticdiff-com bot commented Mar 25, 2025 •

edited

Loading

Aprabhat19 Mar 26, 2025

Aprabhat19 Mar 26, 2025

Aprabhat19 Mar 26, 2025

srujanchikke Mar 27, 2025

srujanchikke Mar 27, 2025

srujanchikke Mar 27, 2025

srujanchikke Mar 27, 2025

srujanchikke Mar 27, 2025

srujanchikke Mar 27, 2025

srujanchikke Mar 27, 2025

srujanchikke Mar 27, 2025

srujanchikke Mar 27, 2025

	pub struct PaymentRecordResponse {
	pub struct PaymentsAttemptRecordRequest {

		// Since its an Error from the connector we should set it as connectors's set status or Failure by default
		let attempt_status = attempt_status.unwrap_or(common_enums::AttemptStatus::Failure);

fix(router): fix retry_count and add validation for process_tracker #7614

Are you sure you want to change the base?

fix(router): fix retry_count and add validation for process_tracker #7614

Conversation

AdityaKumaar21 commented Mar 25, 2025 • edited Loading

Type of Change

Description

Additional Changes

Motivation and Context

How did you test it?

Checklist

semanticdiff-com bot commented Mar 25, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AdityaKumaar21 commented Mar 25, 2025 •

edited

Loading

semanticdiff-com bot commented Mar 25, 2025 •

edited

Loading