Skip to content

fix(connectors): graceful response deserialization for loonie#12731

Open
sai-harsha-vardhan wants to merge 6 commits into
mainfrom
fix-loonie-response-deserialization
Open

fix(connectors): graceful response deserialization for loonie#12731
sai-harsha-vardhan wants to merge 6 commits into
mainfrom
fix-loonie-response-deserialization

Conversation

@sai-harsha-vardhan

Copy link
Copy Markdown
Contributor

Summary

Applied "graceful response deserialization" fixes to the loonie connector to prevent production SEV-1/SEV-2 incidents when the connector adds new fields, enum variants, or changes response structures.

Changes Made

  • Fix 2 (Response Enum Catch-alls): Added #[serde(other)] Unknown variants to all response enums used in business logic:
    • LoonioTransactionStatus
    • RefundStatus
    • LoonioWebhookEventCode
    • LoonioWebhookTransactionType
    • LoonioPayoutStatus
  • Fix 2 (State Preservation): Updated all TryFrom response converters to log a warning and return Ok(item.data) (preserving existing state) when an Unknown variant is received, instead of overwriting Unknown/None to the database.
  • Fix 1 (No deny_unknown_fields): Verified no #[serde(deny_unknown_fields)] attributes exist on response structs.
  • Fix 3 (Unused Fields): Confirmed no unused fields require opaque optional conversion.

Testing

  • Compilation verified via cargo check.
  • Connector-specific clippy checks passed with no loonie-specific warnings.

@sai-harsha-vardhan sai-harsha-vardhan requested a review from a team as a code owner June 11, 2026 16:13
@semanticdiff-com

semanticdiff-com Bot commented Jun 11, 2026

Copy link
Copy Markdown

@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: Graceful Response Deserialization for Loonie + Multiple Connectors

✅ Approved

This PR applies the standard "graceful response deserialization" fixes to prevent SEV incidents. The pattern is consistent across all connectors:

  1. Loonie: Added #[serde(other)] Unknown variants to all response enums
  2. Authorize.Net: Added Unknown variant to webhook events and result codes
  3. Mollie: Converted unused response fields to Option<Secret<T>>
  4. PayPal: Added Unknown variants to incremental auth, extended auth, cancel status, and dispute outcome enums
  5. Shift4: Added Unknown variant to payment/refund status; wrapped unused fields in Secret
  6. Zift: Already has Unknown variant support (PR #12730 covers this separately)

💡 Findings

1. Consistent status preservation pattern
All connectors follow the same pattern: on Unknown status, log a warning and return Ok(item.data) to preserve the existing payment state. This is the correct approach.

2. Authorize.Net webhook mapping
The Unknown webhook event maps to SyncStatus::GeneralError, which eventually becomes WebhookEventTypeNotFound. This is appropriate — unknown webhook events should not be silently accepted.

3. Shift4 field conversions
The conversion of unused fields like currency, amount, first6, last4, brand to Option<Secret<T>> is correct — these fields aren't consumed in business logic and making them opaque prevents hard failures when Shift4 changes response shapes.

🔍 Minor Notes

  • The PR description only mentions Loonie but the diff includes changes to 6+ connectors. The implementation is correct, but consider updating the PR title/description to reflect the full scope.
  • All warning logs follow the same pattern, making it easy to search for "unknown" responses in logs.

Verdict: Clean implementation following established patterns. Ready to merge.

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