[BUG] Improve Error Traceability by Replacing `map_err()` with `change_context()` in Payment List
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:
- The original error's stack trace and context are lost
- Debugging becomes more difficult as we can't see the root cause
- Error logs are less informative, making production issues harder to diagnose
- 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:
- Improved Error Traceability: Preserves the complete error chain, making it possible to trace from the high-level error back to the root cause
- Better Debugging Experience: Developers can see the full context of what went wrong
- Consistent Error Handling: Aligns with the project's established error handling patterns
- More Informative Logs: Error logs will contain more detailed information about what went wrong
- 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
- Compilation Check: Ensure the code compiles successfully with
cargo check --package storage_impl - 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_countmethod 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
- The affected code is in the
Verification Steps
- Make the code changes as described
- Run
cargo checkto verify compilation - 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 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 Still figuring out a few things. Once done, I’ll validate the change and, after triage, assign it over.
@apoorvdixit88 Thanks , looking forward to the update after triage.
hi @apoorvdixit88 , could you assign this issue to me.
Hii @apoorvdixit88 !!
I'd would like to work on this issue.. /assign
This issue is still awaiting triage. In meantime you guys can try picking other good first issues.
I would like to work on this issue, if no one is picking this up!