hyperswitch icon indicating copy to clipboard operation
hyperswitch copied to clipboard

[BUG] Improve Error Traceability by Replacing `map_err()` with `change_context()` in Payment List

Open apoorvdixit88 opened this issue 3 months ago • 3 comments

In crates/storage_impl/src/payments/payment_intent.rs, the get_filtered_active_attempt_ids_for_total_count method uses map_err() for error handling:

.map_err(|er| {
    StorageError::DatabaseError(
        error_stack::report!(diesel_models::errors::DatabaseError::from(er))
            .attach_printable("Error filtering payment records"),
    )
})?
.await

This approach creates a new error report instead of preserving the original error context. As a result:

  1. The original error's stack trace and context are lost
  2. Debugging becomes more difficult as we can't see the root cause
  3. Error logs are less informative, making production issues harder to diagnose
  4. The error handling is inconsistent with the rest of the codebase, which uses change_context()

This is particularly problematic for detailed error information is crucial for troubleshooting.

Solution

Replace the map_err() call with change_context() to preserve the error context, similar to how it's done in other parts of the codebase.

Benefits of this change:

  1. Improved Error Traceability: Preserves the complete error chain, making it possible to trace from the high-level error back to the root cause
  2. Better Debugging Experience: Developers can see the full context of what went wrong
  3. Consistent Error Handling: Aligns with the project's established error handling patterns
  4. More Informative Logs: Error logs will contain more detailed information about what went wrong
  5. Faster Issue Resolution: Support and development teams can more quickly identify and fix issues

Files to modify

  • crates/storage_impl/src/payments/payment_intent.rs

Testing Requirements

  1. Compilation Check: Ensure the code compiles successfully with cargo check --package storage_impl
  2. List Endpoint Testing: Verify that the payment list endpoint works correctly after the change
    • The affected code is in the get_filtered_active_attempt_ids_for_total_count method which is used by the list endpoint
    • Test by making a request to list payments and verify that the response is correct
    • Ensure error scenarios are properly handled and error messages contain the expected detailed information

Verification Steps

  1. Make the code changes as described
  2. Run cargo check to verify compilation
  3. Run the application and test the payment list endpoint

Additional context

The error-stack crate is designed to provide rich, contextual error handling in Rust. Using change_context() instead of map_err() is a key part of leveraging this library correctly.

This pattern is used consistently in other parts of the codebase. For example, in the same file we can see:

.change_context(StorageError::DecryptionError)

apoorvdixit88 avatar Sep 01 '25 13:09 apoorvdixit88

@apoorvdixit88 I would like to work on this issue , just for clarification what i believe the changes i have to do:

    .await
    .change_context(StorageError::DatabaseError)
    .attach_printable("Error filtering payment records")?;

If yes, then assign me!

Raj-G07 avatar Sep 01 '25 19:09 Raj-G07

@Raj-G07 Still figuring out a few things. Once done, I’ll validate the change and, after triage, assign it over.

apoorvdixit88 avatar Sep 01 '25 21:09 apoorvdixit88

@apoorvdixit88 Thanks , looking forward to the update after triage.

Raj-G07 avatar Sep 02 '25 05:09 Raj-G07

hi @apoorvdixit88 , could you assign this issue to me.

3ndeav0ur avatar Oct 01 '25 16:10 3ndeav0ur

Hii @apoorvdixit88 !!

I'd would like to work on this issue.. /assign

sAchin-680 avatar Oct 06 '25 23:10 sAchin-680

This issue is still awaiting triage. In meantime you guys can try picking other good first issues.

apoorvdixit88 avatar Oct 07 '25 07:10 apoorvdixit88

I would like to work on this issue, if no one is picking this up!

suraj-mandal avatar Oct 23 '25 15:10 suraj-mandal