Replace unwrap() with expect() or ? operator in production code

[aram356]

Summary

Replace unwrap() calls in production code paths with proper error handling using expect() (with context message) or the ? operator. unwrap() causes panics without context, making debugging difficult.

Current State

Multiple unwrap() calls exist in production code:

File	Line(s)	Example
crates/common/src/proxy.rs	~18 instances	Various .unwrap() calls
crates/common/src/http_util.rs	64	.expect("encryption failure") (good) but others use .unwrap()
crates/common/src/http_util.rs	125	.unwrap_or_default() - silent error recovery
crates/common/src/synthetic.rs	36, 51, 55-60	.unwrap_or() chains

Target State

• Production code: Use ? operator with proper error types, or expect("clear reason") for truly impossible cases
• Test code: unwrap() is acceptable, but expect("test context") is preferred
• Optional values: Use unwrap_or_default() or unwrap_or() only when the default is semantically correct

// Before - no context on panic
let value = some_option.unwrap();

// After - context on panic (for truly impossible cases)
let value = some_option.expect("value should exist because X was validated");

// Better - proper error handling
let value = some_option.ok_or_else(|| 
    Report::new(TrustedServerError::InvalidState { 
        message: "value missing".into() 
    })
)?;

Changes Required

1. Audit crates/common/src/proxy.rs

This file has the most unwrap() usage. For each instance:

• Determine if the unwrap can actually fail
• If yes: convert to ? with proper error
• If no (impossible case): convert to expect("reason why this can't fail")

2. Audit crates/common/src/http_util.rs

Review encryption/decryption code for proper error handling.

3. Audit crates/common/src/synthetic.rs

Review unwrap_or() chains - ensure defaults are semantically correct.

4. Run clippy with unwrap_used lint

cargo clippy -- -W clippy::unwrap_used

This will identify all remaining unwrap() calls.

Decision Guide

Can this unwrap actually fail in production?
├── Yes → Use ? operator with proper error type
└── No (impossible by construction)
    └── Use expect("reason") documenting why it can't fail

Examples of Good Patterns

// Impossible case - document why
let port = url.port().expect("URL was validated to have port");

// Fallible case - propagate error
let parsed = serde_json::from_str(&body)
    .change_context(TrustedServerError::Parse { 
        message: "invalid JSON body".into() 
    })?;

// Optional with semantic default
let timeout = config.timeout.unwrap_or(Duration::from_secs(30));

Files to Audit

Priority order:

1. crates/common/src/proxy.rs (~18 instances)
2. crates/common/src/http_util.rs
3. crates/common/src/synthetic.rs
4. crates/common/src/backend.rs
5. crates/common/src/integrations/*.rs

Testing

1. cargo clippy -- -W clippy::unwrap_used - Find all instances
2. cargo build - Ensure compilation succeeds
3. cargo test - Ensure all tests pass
4. Test error scenarios return proper errors instead of panicking

Complexity

Medium - Many files to audit, requires understanding each code path

Related Issues

• Add clippy lints configuration to enforce codebase consistency #189 (Add clippy lints) - Will help enforce this going forward

Labels

• tech-debt
• reliability
• code-quality