Standardize error handling patterns: Choose between Report<TrustedServerError> and bare TrustedServerError

[aram356]

Summary

Standardize error handling across the codebase. Currently there's a mix of Report<TrustedServerError> (wrapped errors) and bare TrustedServerError returns, making error handling inconsistent and harder to maintain.

Current State

File	Pattern	Example
crates/common/src/backend.rs:20-22	Report::new(TrustedServerError::...)	Wrapped errors
crates/common/src/backend.rs:79	.change_context()	Uses change_context for wrapping
crates/common/src/fastly_storage.rs:20+	Returns bare TrustedServerError	Direct error, not wrapped
crates/common/src/proxy.rs	Mixed Report<> and bare errors	Inconsistent

Target State

All functions should consistently use Result<T, Report<TrustedServerError>>:

// Consistent pattern
fn my_function() -> Result<String, Report<TrustedServerError>> {
    some_operation()
        .change_context(TrustedServerError::MyError { message: "...".into() })?;
    
    Ok(result)
}

Rationale

• Consistency - One pattern to learn and use
• Context preservation - Report<> allows attaching context via .change_context() and .attach_printable()
• Better debugging - Error chains show full context
• error-stack benefits - The crate is already a dependency, should use it consistently

Changes Required

1. Audit all Result return types

Find all functions returning:

• Result<T, TrustedServerError> → Change to Result<T, Report<TrustedServerError>>
• Functions using ? on bare errors → Wrap with .change_context()

2. Update crates/common/src/fastly_storage.rs

- pub fn get(key: &str) -> Result<String, TrustedServerError> {
+ pub fn get(key: &str) -> Result<String, Report<TrustedServerError>> {

3. Update callers to handle Report<> consistently

Ensure all error handling uses:

• .change_context() to wrap errors with new context
• .attach_printable() to add debug information
• Report::new() to create new errors

4. Document the pattern

Add to CONTRIBUTING.md or create error handling guide:

// Creating a new error
Err(Report::new(TrustedServerError::MyError { message: "...".into() }))

// Wrapping an existing error with context
some_result.change_context(TrustedServerError::MyError { message: "...".into() })?

// Adding debug info without changing error type
some_result.attach_printable("additional context")?

Files to Audit

• crates/common/src/fastly_storage.rs
• crates/common/src/backend.rs
• crates/common/src/proxy.rs
• crates/common/src/synthetic.rs
• crates/common/src/http_util.rs
• crates/common/src/integrations/*.rs

Testing

1. cargo build - Ensure compilation succeeds
2. cargo test - Ensure all tests pass
3. Verify error messages still contain useful context

Complexity

Medium - Touches multiple files, requires understanding error handling patterns

Labels

• tech-debt
• refactoring
• code-quality