refactor(core): add error handling wrapper to wehbook #6636

Sakilmostak · 2024-11-21T20:33:26Z

Type of Change

Description

In webhook, for specific connectors, we want to consume every webhook that arrives even if a error occurs while processing. This is done to keep the webhook channel open with the connector.
In this pr, a error wrapper is introduced to handle this logic where based on connector, we choose whether to send error or not.

Additional Changes

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

Motivation and Context

How did you test it?

Tested through Postman:

Create a MCA (Adyenplatform):
Setup webhook endpoint in dashboard (Adyenplatform):
Create a payout:

{
    "amount": 100,
    "currency": "EUR",
    "customer_id": "sakilcust",
    "email": "[email protected]",
    "phone": "999999999",
    "phone_country_code": "+65",
    "description": "Its my first payout request",
    "payout_type": "bank",
    "priority": "instant",
    "payout_method_data": {
        "bank": {
            "iban": "DE89370400440532013000"
        }
    },
    "billing": {
        "address": {
            "line1": "Raadhuisplein",
            "line2": "92",
            "city": "Hoogeveen",
            "state": "FL",
            "zip": "7901 BW",
            "country": "NL",
            "first_name": "John",
            "last_name": "Doe"
        },
        "phone": {
            "number": "0650242319",
            "country_code": "+31"
        }
    },
    "entity_type": "Individual",
    "recurring": true,
    "confirm": true,
    "auto_fulfill": true
}

You should receive webhook for the following payout to the endpoint mentioned during merchant creation

Case 2:

Setup ngrok to intercept webhook
Create a payout (same body as above)
delete the payout_attempt from the payout_attempt table against the generated payout_id
See the log in ngrok for response sent to the adyenplatform
The status should be 200 with x-http-code present in headers

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 · 2024-11-21T20:33:29Z

Review changes with

Changed Files

File	Status
crates/router/src/connector/adyenplatform.rs	23% smaller
crates/hyperswitch_interfaces/src/webhooks.rs	10% smaller
crates/hyperswitch_connectors/src/connectors/cashtocode.rs	6% smaller
crates/hyperswitch_connectors/src/connectors/worldline.rs	6% smaller
crates/router/src/core/webhooks/incoming.rs	6% smaller
crates/hyperswitch_connectors/src/connectors/zen.rs	1% smaller
crates/hyperswitch_connectors/src/connectors/zsl.rs	1% smaller
crates/api_models/src/connector_enums.rs	0% smaller
crates/router/src/connector/adyen.rs	0% smaller
crates/router/src/connector/braintree.rs	0% smaller
crates/router/src/core/webhooks/incoming_v2.rs	0% smaller
crates/router/src/services/connector_integration_interface.rs	Unsupported file format

sai-harsha-vardhan · 2024-11-26T13:49:51Z

crates/router/src/utils.rs

@@ -1344,3 +1344,7 @@ pub async fn trigger_refund_outgoing_webhook(
 ) -> RouterResult<()> {
    todo!()
 }
+
+pub fn is_webhook_not_found_error(err: &errors::ApiErrorResponse) -> bool {
+    matches!(err, errors::ApiErrorResponse::WebhookResourceNotFound)


Please add all the other NotFound errors which can occur like PaymentNotFound, RefundNotFound etc.

sai-harsha-vardhan · 2024-11-26T13:57:43Z

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

@@ -117,14 +117,104 @@ pub async fn incoming_webhooks_wrapper<W: types::OutgoingWebhookType>(
    Ok(application_response)
 }

+async fn incoming_webhooks_error_wrapper<W: types::OutgoingWebhookType>(


can we try to eliminate this wrapper?

sai-harsha-vardhan · 2024-11-26T18:01:44Z

crates/api_models/src/connector_enums.rs

@@ -282,6 +282,9 @@ impl Connector {
    pub fn is_pre_processing_required_before_authorize(&self) -> bool {
        matches!(self, Self::Airwallex)
    }
+    pub fn should_connector_bypass_error_if_webhook_not_found(&self) -> bool {


Suggested change

pub fn should_connector_bypass_error_if_webhook_not_found(&self) -> bool {

pub fn should_acknowledge_webhook_for_resource_not_found_errors(&self) -> bool {

sai-harsha-vardhan · 2024-11-26T18:03:08Z

crates/router/src/connector/adyenplatform.rs

+            )))
+        } else {
+            Ok(services::api::ApplicationResponse::TextPlain(
+                "[accepted]".to_string(),


There's no need to send "[accepted]" back in the response / any content body right?

sai-harsha-vardhan · 2024-11-26T18:05:30Z

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

            state.clone(),
            req_state,
            req,
            merchant_account.clone(),
            key_store,
-            connector_name_or_mca_id,
+            merchant_connector_account,
+            connector.clone(),


you can try passing references here

sai-harsha-vardhan · 2024-11-26T18:07:06Z

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

-    let (application_response, webhooks_response_tracker, serialized_req) =
-        Box::pin(incoming_webhooks_core::<W>(
+
+    // Fetch the merchant connector account to get the webhooks source secret


Can you also please add the comment why the mca and connector are being fetched in wrapper instead of core?

sai-harsha-vardhan · 2024-11-26T18:07:50Z

crates/router/src/utils.rs

+            | errors::ApiErrorResponse::PayoutNotFound
+            | errors::ApiErrorResponse::MandateNotFound
+            | errors::ApiErrorResponse::PaymentNotFound
+            | errors::ApiErrorResponse::RefundNotFound


Should add errors::ApiErrorResponse::AuthenticationNotFound error too

crates/router/src/utils.rs

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

SanchithHegde · 2024-11-27T11:53:18Z

crates/hyperswitch_interfaces/src/webhooks.rs

+            | ApiErrorResponse::PaymentNotFound
+            | ApiErrorResponse::RefundNotFound
+            | ApiErrorResponse::AuthenticationNotFound { .. } => Self::ResourceNotFound,
+            _ => Self::InternalError,


Should we map everything else to an internal error, or should we only map InternalServerError to InternalError and everything else to an Unknown error variant?

SanchithHegde · 2024-11-27T12:14:54Z

crates/router/src/connector/adyenplatform.rs

+                vec![(
+                    "x-http-code".to_string(),
+                    Maskable::Masked(Secret::new("404".to_string())),
+                )],


Do we still need this custom header? If yes, then where would be monitoring this (Grafana dashboard, etc.)? Or can we have metrics instead?

Sakilmostak added A-core Area: Core flows C-refactor Category: Refactor A-webhooks Area: Webhook flows labels Nov 21, 2024

Sakilmostak added this to the November 2024 Release milestone Nov 21, 2024

Sakilmostak self-assigned this Nov 21, 2024

Sakilmostak requested review from a team as code owners November 21, 2024 20:33

Sakilmostak requested a review from a team as a code owner November 26, 2024 08:04

sai-harsha-vardhan reviewed Nov 26, 2024

View reviewed changes

SanchithHegde reviewed Nov 26, 2024

View reviewed changes

crates/router/src/utils.rs Outdated Show resolved Hide resolved

crates/router/src/core/webhooks/incoming.rs Outdated Show resolved Hide resolved

refactor: comments

2f81d72

Sakilmostak force-pushed the webhook_error_wrapper branch from cc7f16c to 2f81d72 Compare November 27, 2024 11:25

refactor: resolve ci checks

a40a8c3

SanchithHegde reviewed Nov 27, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(core): add error handling wrapper to wehbook #6636

refactor(core): add error handling wrapper to wehbook #6636

Sakilmostak commented Nov 21, 2024 •

edited

Loading

semanticdiff-com bot commented Nov 21, 2024 •

edited

Loading

sai-harsha-vardhan Nov 26, 2024

sai-harsha-vardhan Nov 26, 2024

sai-harsha-vardhan Nov 26, 2024

sai-harsha-vardhan Nov 26, 2024

sai-harsha-vardhan Nov 26, 2024

sai-harsha-vardhan Nov 26, 2024

sai-harsha-vardhan Nov 26, 2024

SanchithHegde Nov 27, 2024

SanchithHegde Nov 27, 2024

	pub fn should_connector_bypass_error_if_webhook_not_found(&self) -> bool {
	pub fn should_acknowledge_webhook_for_resource_not_found_errors(&self) -> bool {

refactor(core): add error handling wrapper to wehbook #6636

Are you sure you want to change the base?

refactor(core): add error handling wrapper to wehbook #6636

Conversation

Sakilmostak commented Nov 21, 2024 • edited Loading

Type of Change

Description

Additional Changes

Motivation and Context

How did you test it?

Checklist

semanticdiff-com bot commented Nov 21, 2024 • 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

Sakilmostak commented Nov 21, 2024 •

edited

Loading

semanticdiff-com bot commented Nov 21, 2024 •

edited

Loading