Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG]: MCA metadata deserialization failures should be 4xx #2899

Closed
2 tasks done
swangi-kumari opened this issue Nov 17, 2023 · 5 comments · Fixed by #3168
Closed
2 tasks done

[BUG]: MCA metadata deserialization failures should be 4xx #2899

swangi-kumari opened this issue Nov 17, 2023 · 5 comments · Fixed by #3168
Assignees
Labels
A-connector-integration Area: Connector integration C-bug Category: Bug good first issue Good for newcomers help wanted Extra attention is needed

Comments

@swangi-kumari
Copy link
Contributor

swangi-kumari commented Nov 17, 2023

📝 Feature Description

  • We don't have validation for Merchant Connector Account metadata, So while creating MCA we are allowing any json value. Because of which RequestEncodingFailure happening during payments for the MCA which doesn't have the expected fields.
  • Recreating scenario : Create a merchant connector account for coinbase with empty metadata .It doesn't throw error as we don't have metadata validation for coinbase . If you try to make payment now this will throw deserilization error for CoinbaseConnectorMeta which is merchant connector metadata for coinbase .

🔨 Possible Implementation

  • In the event of a deserialization failure of MCA metadata during payments and refunds, convert the ParsingFailed error to ConnectorError::InvalidConnectorConfig.
  • We already have authType validation in core/admin.rs , we need similar validation for connector metadata for coinbase .
  • Make sure that the config field in ConnectorError::InvalidConnectorConfig specifies the invalid configuration name.
  • Note that the ConnectorError::InvalidConnectorConfig error is already associated with HTTP 400 in the core.
  • You can check this PR for further reference fix(connector): [fiserv] fix metadata deserialization in merchant_connector_account #2746

🔖 Note: All the changes needed should be contained within /hyperswitch_oss/crates/router/src/connector/utils.rs

📦 Have you spent some time checking if this feature request has been raised before?

  • I checked and didn't find a similar issue

📦 Have you read the Contributing Guidelines?

✨ Are you willing to submit a PR?

@swangi-kumari swangi-kumari added A-connector-integration Area: Connector integration C-bug Category: Bug good first issue Good for newcomers labels Nov 17, 2023
@srujanchikke srujanchikke changed the title [BUG]: [Coinbase] SessionObject deserialization error [BUG]: [Coinbase] CoinbaseConnectorMeta deserialization error Nov 17, 2023
@swangi-kumari swangi-kumari changed the title [BUG]: [Coinbase] CoinbaseConnectorMeta deserialization error [BUG]: MCA metadata deserialization failures should be 4xx Nov 17, 2023
@VedantKhairnar VedantKhairnar added the help wanted Extra attention is needed label Nov 23, 2023
@bkontur
Copy link
Contributor

bkontur commented Dec 19, 2023

Incidentally, I came across this issue through the This Week in Rust newsletter, so please take a look: #3168

@SanchithHegde
Copy link
Member

Sure @bkontur, I'll assign this issue to you so that nobody else picks this up.

@bkontur
Copy link
Contributor

bkontur commented Dec 20, 2023

@SanchithHegde can you, please, ping/approve CI on that #3168 , that we know all pipelines are passing?

@bkontur
Copy link
Contributor

bkontur commented Dec 22, 2023

@SanchithHegde just out of curiosity, do you have any estimate for reviewing? How long does PR review take here? :)

@SanchithHegde
Copy link
Member

Hey @bkontur, we don't have many folks available due to the holidays, I expect limited activity over the next week. Apologies for the delay!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-connector-integration Area: Connector integration C-bug Category: Bug good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
4 participants