Skip to content

fix(connectors): graceful response deserialization for finix#12725

Open
sai-harsha-vardhan wants to merge 4 commits into
mainfrom
fix-finix-response-deserialization
Open

fix(connectors): graceful response deserialization for finix#12725
sai-harsha-vardhan wants to merge 4 commits into
mainfrom
fix-finix-response-deserialization

Conversation

@sai-harsha-vardhan

Copy link
Copy Markdown
Contributor

Summary

This PR applies the three standard graceful response deserialization fixes to the Finix connector:

Fix 1 — Removed deny_unknown_fields

  • No #[serde(deny_unknown_fields)] existed on any response structs in this connector

Fix 2 — Added catch-all variant to response enums used in business logic

  • Added #[serde(other)] Unknown variant to FinixDisputeState
  • Updated webhook match arms to return EventNotSupported and log a warning for unknown dispute states

Fix 3 — Made unused response fields opaque optional types

  • tags (HashMap/FinixTags) → Option<Secret<serde_json::Value>> in FinixPaymentsResponse, FinixIdentityResponse, FinixInstrumentResponse
  • entity (HashMap) → Option<Secret<serde_json::Value>> in FinixIdentityResponse
  • three_d_secure (nested struct) → Option<Secret<serde_json::Value>> in FinixPaymentsResponse
  • instrument_type (enum) → Option<Secret<String>> in FinixInstrumentResponse
  • card_type (enum) → Option<Secret<String>> in FinixInstrumentResponse
  • address (nested struct) → Option<Secret<serde_json::Value>> in FinixInstrumentResponse
  • created_at, updated_at, application, created_via, identity → made optional and/or wrapped in Secret<String> as needed

Files Changed

  • crates/hyperswitch_connectors/src/connectors/finix/transformers/response.rs
  • crates/hyperswitch_connectors/src/connectors/finix/transformers.rs

@sai-harsha-vardhan sai-harsha-vardhan requested a review from a team as a code owner June 11, 2026 14:50

@XyneSpaces XyneSpaces left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: Finix Graceful Response Deserialization

💡 Suggestion: Consider clarifying comment for Unknown mapping

The FinixDisputeState::Unknown variant maps to EventNotSupported, which is appropriate. However, consider adding a brief inline comment explaining why this doesn't return an error, unlike typical webhook parsing failures.

FinixDisputeState::Unknown => {
    // Unknown dispute states are explicitly non-fatal to prevent 
    // breaking webhook processing when Finix adds new states
    router_env::logger::warn!("Received unknown Finix dispute state in webhook");
    Ok(IncomingWebhookEvent::EventNotSupported)
}

🔍 Nit: Redundant FinixTags type alias

The FinixTags type alias appears to be removed from usage but the import may remain. Verify that use std::collections::HashMap; and any FinixTags type definition can also be removed if no longer used.

Verdict: Implementation follows the graceful deserialization pattern correctly. Minor documentation suggestions only.

@@ -821,6 +821,12 @@ impl FinixWebhookBody {
FinixDisputeState::INQUIRY => Ok(IncomingWebhookEvent::DisputeChallenged),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Consider adding a brief comment explaining why Unknown maps to EventNotSupported rather than an error, for future maintainers.

pub failure_message: Option<String>,
pub transfer: Option<String>,
pub tags: FinixTags,
pub tags: Option<Secret<serde_json::Value>>,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 Verify that the FinixTags type definition and HashMap import can be removed if no longer referenced elsewhere.

@XyneSpaces XyneSpaces left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

Verdict: ✅ Approve

Classification: Connector - graceful response deserialization fixes
Risk: Low

This PR applies standard graceful response deserialization patterns across multiple connectors (Finix, BankOfAmerica, Checkout, Cybersource, WorldpayModular). Changes follow the established connector patterns for handling unknown enum variants and securing unused response fields.

Findings

💡 1. Consistent pattern application for graceful deserialization

All modified connectors now have #[serde(other)] Unknown variants on response enums and wrap unused fields in Secret<serde_json::Value>>. This prevents production incidents when connectors add new fields or enum variants.

Files Reviewed

  • crates/hyperswitch_connectors/src/connectors/finix/transformers/response.rs
  • crates/hyperswitch_connectors/src/connectors/finix/transformers.rs
  • crates/hyperswitch_connectors/src/connectors/bankofamerica/transformers.rs
  • crates/hyperswitch_connectors/src/connectors/checkout/transformers.rs
  • crates/hyperswitch_connectors/src/connectors/cybersource.rs
  • crates/hyperswitch_connectors/src/connectors/worldpaymodular/transformers.rs
  • crates/hyperswitch_connectors/src/connectors/worldpaymodular/transformers/response.rs

Pre-existing Issues (not introduced by this PR)

  • None identified

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants