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

refactor(router): remove WebhookApiErrorSwitch and implement error mapping using ErrorSwitch #1660

Merged
merged 4 commits into from
Jul 17, 2023

Conversation

PanGan21
Copy link
Contributor

@PanGan21 PanGan21 commented Jul 8, 2023

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

Refactor error mapping with WebhookApiErrorSwitch to make use of the existing ErrorSwitch trait which satisfies the same usecase.
Closes #1326

How did you test it?

Checklist

  • I formatted the code cargo +nightly fmt --all
  • I addressed lints thrown by cargo clippy
  • I reviewed submitted code
  • I added unit tests for my changes where possible
  • I added a CHANGELOG entry if applicable

@github-actions github-actions bot added the S-conventions-not-followed Status: This PR does not follow contributing guidelines label Jul 8, 2023
@PanGan21 PanGan21 changed the title refactopr: remove WebhookApiErrorSwitch and implement error mapping using ErrorSwitch refactor: remove WebhookApiErrorSwitch and implement error mapping using ErrorSwitch Jul 8, 2023
@github-actions github-actions bot removed the S-conventions-not-followed Status: This PR does not follow contributing guidelines label Jul 8, 2023
@PanGan21
Copy link
Contributor Author

PanGan21 commented Jul 8, 2023

This line for example would give an error type annotations needed which is not fixed if I add the type

screenshot:
Screen Shot 2023-07-09 at 1 41 30 PM

@PanGan21 PanGan21 changed the title refactor: remove WebhookApiErrorSwitch and implement error mapping using ErrorSwitch refactor(router): remove WebhookApiErrorSwitch and implement error mapping using ErrorSwitch Jul 9, 2023
@vspecky
Copy link
Member

vspecky commented Jul 10, 2023

@PanGan21 Hey, you need to bring the common_utils::errors::ReportSwitchExt trait into scope to use switch() there. impl ErrorSwitch<U> for T provides the T => U conversion logic, but we need additional functionality to promote this logic to support Result<R, error_stack::Report<T>> => Result<R, error_stack::Report<U>> (which is what get_dispute_details() returns and what disputes_incoming_webhook_flow() is expecting to be returned). The ReportSwitchExt trait and a corresponding impl imparts this functionality to all such Result types.

impl<T, U, V> ReportSwitchExt<T, U> for Result<T, error_stack::Report<V>>

@PanGan21
Copy link
Contributor Author

@PanGan21 Hey, you need to bring the common_utils::errors::ReportSwitchExt trait into scope to use switch() there. impl ErrorSwitch<U> for T provides the T => U conversion logic, but we need additional functionality to promote this logic to support Result<R, error_stack::Report<T>> => Result<R, error_stack::Report<U>> (which is what get_dispute_details() returns and what disputes_incoming_webhook_flow() is expecting to be returned). The ReportSwitchExt trait and a corresponding impl imparts this functionality to all such Result types.

impl<T, U, V> ReportSwitchExt<T, U> for Result<T, error_stack::Report<V>>

It indeed did the trick! Thanks for the help and the insights. I will publish the pr then 😃

@PanGan21 PanGan21 marked this pull request as ready for review July 10, 2023 21:21
@PanGan21 PanGan21 requested a review from a team as a code owner July 10, 2023 21:21
@vspecky vspecky added A-core Area: Core flows S-waiting-on-review Status: This PR has been implemented and needs to be reviewed C-refactor Category: Refactor A-errors Area: error messages, structure & logging R-waiting-on-L1 Review: Waiting on L1 reviewer labels Jul 11, 2023
@vspecky vspecky added this to the July 2023 Release milestone Jul 11, 2023
vspecky
vspecky previously approved these changes Jul 11, 2023
SanchithHegde
SanchithHegde previously approved these changes Jul 11, 2023
@SanchithHegde
Copy link
Member

Thanks for the PR @PanGan21!

@SanchithHegde SanchithHegde dismissed stale reviews from vspecky and themself via 7523a59 July 17, 2023 17:23
@SanchithHegde SanchithHegde merged commit a7c66dd into juspay:main Jul 17, 2023
@SanchithHegde SanchithHegde removed S-waiting-on-review Status: This PR has been implemented and needs to be reviewed R-waiting-on-L1 Review: Waiting on L1 reviewer labels Jul 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-core Area: Core flows A-errors Area: error messages, structure & logging C-refactor Category: Refactor
Projects
No open projects
Status: Merged
Development

Successfully merging this pull request may close these issues.

[REFACTOR] Define the Mapping between ConnectorError and ApiErrorResponse using the ErrorSwitch trait
3 participants