Skip to content

feat(core): add surcharge support for MIT#12715

Open
swangi-kumari wants to merge 4 commits into
mainfrom
mit-surcharge
Open

feat(core): add surcharge support for MIT#12715
swangi-kumari wants to merge 4 commits into
mainfrom
mit-surcharge

Conversation

@swangi-kumari

Copy link
Copy Markdown
Contributor

Type of Change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates
  • Documentation
  • CI/CD

Description

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?

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

@swangi-kumari swangi-kumari requested review from a team as code owners June 11, 2026 12:20
@semanticdiff-com

semanticdiff-com Bot commented Jun 11, 2026

Copy link
Copy Markdown

Review changes with  SemanticDiff

Changed Files
File Status
  crates/router/src/core/payments.rs  22% smaller
  Dockerfile Unsupported file format
  crates/hyperswitch_domain_models/src/payment_method_data.rs  0% smaller

@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.

Automated Review Summary

PR: feat(core): add surcharge support for MIT
Reviewer: PR Reviewer Agent


Findings

⚠️ 1. Best-effort surcharge calculation silently swallows errors

File: crates/router/src/core/payments.rs

calculate_mit_external_surcharge returns Option and logs warnings for all error paths. While the "best-effort" semantics are documented, consider if any failure modes should surface to the caller rather than being silently ignored. For example:

  • MCA fetch failure (configuration error that should alert operators)
  • UCS timeout (may indicate service degradation)

Current behavior is acceptable for recurring payment UX, but ensure monitoring is in place for surcharge calculation failure rates.


🔍 2. Missing instrumentation on new async functions

Files: crates/router/src/core/payments.rs

The new async functions run_external_surcharge_ucs and calculate_mit_external_surcharge should have #[instrument(skip_all)] attributes for observability, consistent with other payment core functions.


🔍 3. Card IIN extraction fallback chain

File: crates/router/src/core/payments.rs

The fallback from payment_method_data.get_card_iin() to payment_attempt.payment_method_data JSON extraction is reasonable, but document why PSP-tokenized MIT flows don't have a Card variant available. This is subtle behavior that future maintainers may question.


Verdict:Approve with minor suggestions — Clean refactor with proper best-effort semantics for MIT surcharge. Add instrumentation attributes before merge.

@swangi-kumari swangi-kumari self-assigned this Jun 11, 2026
@swangi-kumari swangi-kumari requested a review from a team as a code owner June 11, 2026 14:00
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