Standardize function return types: Consistent Result and Option usage

[aram356]

Summary

Standardize function return types across the codebase. Currently similar operations return different types (Result<T, Report<E>> vs Result<T, E> vs Option<T>), making the API inconsistent.

Current State

Function	Return Type	File
ensure_origin_backend()	Result<String, Report<TrustedServerError>>	backend.rs
fastly_storage::get()	Result<String, TrustedServerError>	fastly_storage.rs
get_synthetic_id()	Result<Option<String>, Report<TrustedServerError>>	synthetic.rs
extract_credentials()	Option<(String, String)>	auth.rs
to_abs()	Option<String>	(various)

Target State

Establish clear conventions:

Convention 1: Fallible operations use Result<T, Report<TrustedServerError>>

// Operations that can fail should return Report-wrapped errors
fn fetch_data() -> Result<Data, Report<TrustedServerError>>

Convention 2: Optional values use Option<T>

// Values that may or may not exist (not an error condition)
fn get_header(name: &str) -> Option<&str>

Convention 3: Fallible + Optional use Result<Option<T>, Report<E>>

// Operation can fail, AND value might not exist
fn get_cached_value(key: &str) -> Result<Option<String>, Report<TrustedServerError>>

Convention 4: Never use bare TrustedServerError

// Bad - loses error context chain
fn bad() -> Result<T, TrustedServerError>

// Good - preserves error context
fn good() -> Result<T, Report<TrustedServerError>>

Changes Required

1. Update functions returning bare TrustedServerError

Find all:

fn foo() -> Result<T, TrustedServerError>

Change to:

fn foo() -> Result<T, Report<TrustedServerError>>

2. Review Option returns for error cases

Some functions return Option<T> but the None case represents an error that should be surfaced:

// Before - error is hidden
fn extract_credentials(req: &Request) -> Option<(String, String)>

// After - if missing credentials is an error condition
fn extract_credentials(req: &Request) -> Result<(String, String), Report<TrustedServerError>>

// OR - if missing credentials is expected/normal
fn extract_credentials(req: &Request) -> Option<(String, String)>  // Keep as-is, document why

3. Document conventions

Add to CONTRIBUTING.md:

## Return Type Conventions

- `Result<T, Report<TrustedServerError>>` - Operation can fail
- `Option<T>` - Value may not exist (not an error)
- `Result<Option<T>, Report<E>>` - Operation can fail AND value may not exist
- Never use `Result<T, TrustedServerError>` (always wrap in `Report<>`)

Files to Audit

• crates/common/src/fastly_storage.rs - Returns bare error
• crates/common/src/auth.rs - Returns Option for potential errors
• crates/common/src/http_util.rs - Mixed patterns
• crates/common/src/synthetic.rs - Already good pattern
• All integration files

Testing

1. cargo build - Ensure compilation succeeds
2. cargo test - Ensure all tests pass
3. Review that error information is preserved through call chains

Complexity

Medium - Requires consistent changes across multiple files and updating callers

Related Issues

• Added publisher config #29 (Standardize error handling patterns)
• Rebuild when environment variable change #30 (Replace unwrap() calls)

Labels

• tech-debt
• refactoring
• code-quality