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

fix(router): change Not Found response to Internal Server Error for empty vector #6150

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

shreyshukla29
Copy link

@shreyshukla29 shreyshukla29 commented Sep 27, 2024

…ctor (#5916)

Modified the get_aggregates_for_payments function to address issue #5916.

  • Replaced the to_not_found_response handler with map_err to handle internal errors during data fetching.
  • Ensured that an empty vector returned from the get_intent_status_with_count method is treated as valid and does not result in a "Not Found" error.
  • Now, an Internal Server Error (HTTP 500) is returned only when there is an actual problem with fetching data from the database.
  • Attached a descriptive error message using attach_printable to provide more context in case of internal errors.

This fix ensures that valid empty results no longer cause false error responses and internal issues are correctly flagged with a 500 error.

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

…ctor (juspay#5916)

Modified the get_aggregates_for_payments function to address issue juspay#5916. 

- Replaced the to_not_found_response handler with map_err to handle internal errors during data fetching.
- Ensured that an empty vector returned from the get_intent_status_with_count method is treated as valid and does not result in a "Not Found" error.
- Now, an Internal Server Error (HTTP 500) is returned only when there is an actual problem with fetching data from the database.
- Attached a descriptive error message using attach_printable to provide more context in case of internal errors.

This fix ensures that valid empty results no longer cause false error responses and internal issues are correctly flagged with a 500 error.
@shreyshukla29 shreyshukla29 requested a review from a team as a code owner September 27, 2024 13:49
Copy link

semanticdiff-com bot commented Sep 27, 2024

Review changes with SemanticDiff.

Analyzed 1 of 1 files.

Overall, the semantic diff is 55% smaller than the GitHub diff.

Filename Status
✔️ crates/router/src/core/payments.rs 54.89% smaller

@shreyshukla29 shreyshukla29 changed the title [FIX] Change Not Found Response to Internal Server Error for Empty Ve… fix(router): change Not Found response to Internal Server Error for empty vector Sep 27, 2024
@shreyshukla29 shreyshukla29 reopened this Sep 27, 2024
@gorakhnathy7 gorakhnathy7 added the hacktoberfest Issues that are up for grabs for Hacktoberfest participants label Oct 24, 2024
@gorakhnathy7
Copy link
Collaborator

Hey @shreyshukla29 Any update on changes?

@shreyshukla29
Copy link
Author

Hey @shreyshukla29 Any update on changes?

I’ve been reviewing the suggestions and working on implementing the changes to the get_aggregates_for_payments function. I plan to utilize the .change_context(errors::ApiErrorResponse::InternalServerError) method to enhance error handling.

… vector Updated the get_aggregates_for_payments function to use change_context to return an Internal Server Error (500) when there's a failure fetching payment intent status. Added a human-readable message for better debugging. Fixes juspay#5916
@shreyshukla29
Copy link
Author

Hi @gorakhnathy7 ,

Thanks for following up! I wanted to let you know that I’ve been reviewing the code, but it appears that the get_aggregates_for_payments function is not present in the recent branch, and the branch I’m working from seems to be outdated.

Could you please confirm if there’s a newer branch I should be looking at? I want to ensure that my changes are aligned with the latest codebase.

Thanks for your help!

Best,
Shrey

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest Issues that are up for grabs for Hacktoberfest participants
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FIX] Change Not Found Response to Internal Server Error for Empty Vector
3 participants