juspay / hyperswitch

An open source payments switch written in Rust to make payments fast, reliable and affordable
https://hyperswitch.io/
Apache License 2.0
12.62k stars 1.36k forks source link

[FIX] Change Not Found Response to Internal Server Error for Empty Vector #5916

Open apoorvdixit88 opened 2 months ago

apoorvdixit88 commented 2 months ago

Feature Description

In the get_aggregates_for_payments function, when querying the database using the get_intent_status_withcount method, the function currently returns a Not Found response when an empty vector is encountered. **The not found case will never arise since we will be getting empty vector in such cases._**

Since this scenario represents an internal logic issue rather than a missing payment, we should update the function to return an Internal Server Error instead.

let intent_status_with_count = db
    .get_intent_status_with_count(merchant.get_id(), profile_id_list, &time_range)
    .await
    .to_not_found_response(errors::ApiErrorResponse::PaymentNotFound)?;

Possible Implementation

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

Submission Process:

Refer here for Terms and conditions for the contest.

shreyshukla29 commented 1 month ago

Hi,

I’d like to take up this issue and implement the necessary changes to handle empty vectors in the get_aggregates_for_payments function. The goal would be to replace the "Not Found" response with an Internal Server Error (500), since an empty vector indicates an internal logic issue rather than a missing payment.

Could you please assign this issue to me? I’m ready to submit a PR once implemented.

Thanks!

Best regards, Shrey Shukla

apoorvdixit88 commented 1 month ago

Hi @shreyshukla29 , Thanks for showing interest! Just to clarify:

An empty vector from get_intent_status_with_count is fine and doesn't mean there's an issue. We only want to throw an error when an actual problem occurs while fetching data (e.g., an internal error).

The current logic uses the to_not_found_response method to handle different error types. Right now, it maps certain cases (like ValueNotFound) to return a "Not Found" response, such as PaymentNotFound. However, the issue here is that an empty vector result is fine and doesn't indicate an error. We should only throw an Internal Server Error (500) if there's an actual problem with fetching the data, not just because the result is empty.

So, we need to update the logic to keep the empty vector as valid, but replace the to_not_found_response with an error handler that returns a 500 only when such issue occurs.

Feel free to ask if you need more details!

shreyshukla29 commented 1 month ago

Thank you for the clarification!

Got it — the empty vector is a valid result and doesn’t necessarily indicate an error. I'll ensure the logic remains intact for such cases, and instead, I'll focus on returning an Internal Server Error (500) only when an actual issue occurs while fetching the data (e.g., internal errors), without changing how the empty vector is handled.

I'll update the error handler accordingly and remove the use of to_not_found_response in cases where it isn't appropriate.

Please assign this issue to me, and I’ll get started. Feel free to share any further details if necessary!

apoorvdixit88 commented 1 month ago

sure @shreyshukla29, you can pick this up