Remove all remaining unwrap calls in src/ and tests/

[coderabbitai]

Summary

Found 46 unwrap occurrences across 20 files that need to be addressed for better error handling.

Files and Occurrence Count

• src/push/queues.rs (14 calls)
• src/extractor.rs (5 calls)
• tests/push.rs (4 calls)
• tests/connection_actor.rs (3 calls)
• src/server/config/binding.rs (3 calls)
• tests/world.rs (2 calls)
• tests/wireframe_protocol.rs (2 calls)
• tests/stream_end.rs (2 calls)
• tests/session_registry.rs (1 call)
• tests/server.rs (1 call)
• tests/push_policies.rs (1 call)
• tests/correlation_id.rs (1 call)
• tests/async_stream.rs (1 call)
• tests/advanced/interaction_fuzz.rs (1 call)
• tests/advanced/concurrency_loom.rs (1 call)
• src/server/connection.rs (1 call)
• src/frame/tests.rs (1 call)
• src/app.rs (1 call)

Requirements

Production Code (src/)
Replace unwrap uses with proper error propagation:

• Use ? operator where appropriate
• Use map_err for error transformation
• Use expect with descriptive messages where unwrap is unavoidable

Test Code (tests/)
Replace raw unwrap() calls with expect("descriptive failure message") to provide better debugging information when tests fail.

Task

Review each file and refactor accordingly to improve error handling and debugging experience.

References

• Related PR: Add builder for push queues #335
• Original request: Add builder for push queues #335 (comment)

[leynos]

@coderabbitai plan

[coderabbitai]

Coding Plan

Approach

Remove all remaining unwrap() calls from production code in src/ (replacing with proper error propagation) and from test code in tests/ (replacing with expect() with descriptive messages).

Observations

The Wireframe codebase uses thiserror for structured error handling with hierarchical error types using #[from] and #[source] attributes. The codebase follows excellent error handling discipline with domain-specific Result type aliases and comprehensive use of the ? operator. Test files use custom macros (push_expect!, recv_expect!) and consistently follow the "failed to [action] [object]" message pattern for expect() calls.

Assumptions

Assumption 1: The ticket reflects an outdated state of the codebase

Options Considered:

1. Proceed with the ticket as written, modifying all listed files
2. Verify actual codebase state and only address confirmed unwrap() occurrences
3. Close ticket as already completed with no implementation needed

Chosen Option: 2

Rationale: The exploration conclusively shows that production code has zero unwrap() calls (only 9 in rustdoc examples) and tests already use expect() throughout (149 occurrences, 0 raw unwrap). However, a verification phase ensures nothing is missed before confirming completion.

Assumption 2: Documentation examples with unwrap() are acceptable

Options Considered:

1. Replace doc example unwrap() calls with expect() or proper error handling
2. Leave doc example unwrap() calls as-is per Rust community conventions

Chosen Option: 2

Rationale: Rustdoc examples conventionally use unwrap() for brevity and clarity. These are not executed in production and help readers focus on the API being demonstrated. This is standard Rust practice.

💡 User Tips

Regenerate the plan with different choices with @coderabbitai <feedback>.

Implementation Steps

Phase 1: Verification and Audit

Confirm the current codebase state matches exploration findings before declaring the ticket complete. This phase ensures no unwrap() calls were missed and documents the actual state.

Task 1: Audit Production Code (src/)

Perform a comprehensive verification that no unwrap() calls exist in production code paths.

• Run grep -rn "\.unwrap()" src/ --include="*.rs" to get complete list of all unwrap occurrences
• For each occurrence found, verify it is within a /// rustdoc comment block (documentation example)
• Confirm the following files have zero production-code unwrap() calls:
  • src/push/queues/mod.rs (not src/push/queues.rs which doesn't exist)
  • src/server/connection.rs
  • src/app/mod.rs (not src/app.rs which doesn't exist)
  • src/frame/tests.rs (test-only module, imported with #[cfg(test)])
• Document findings: expected result is 9 unwrap() calls, all in doc examples in src/extractor.rs (6) and src/server/config/binding.rs (3)

Task 2: Audit Test Code (tests/)

Verify that test files use expect() with descriptive messages rather than raw unwrap().

• Run grep -rn "\.unwrap()" tests/ --include="*.rs" to search for any raw unwrap() calls
• Confirm the following files have zero unwrap() calls (they should already use expect()):
  • tests/push.rs
  • tests/world.rs
  • tests/wireframe_protocol.rs
  • tests/stream_end.rs
  • tests/session_registry.rs
  • tests/server.rs
  • tests/push_policies.rs
  • tests/correlation_id.rs
  • tests/async_stream.rs
  • tests/advanced/interaction_fuzz.rs
  • tests/advanced/concurrency_loom.rs
• Note: tests/connection_actor.rs does not exist; related files are tests/connection_actor_errors.rs, tests/connection_actor_fairness.rs, tests/connection_actor_shutdown.rs
• Expected result: 0 unwrap() calls found; 149+ expect() calls with descriptive messages already present

Task 3: Document Verification Results

Create documentation of the audit findings to close out the ticket.

• Summarize the discrepancy between ticket claims (46 unwrap across 20 files) and actual state (0 production unwrap, 0 test unwrap)
• List any file path corrections (e.g., src/push/queues.rs → src/push/queues/mod.rs)
• Confirm that rustdoc example unwrap() calls are intentionally left as-is per Rust conventions
• Reference this verification when closing the ticket or related PR

🤖 Prompt for AI agents

This phase verifies the current state of the codebase to confirm that unwrap()
calls have already been addressed. The goal is to audit both production and test
code, then document the findings.

**Audit Production Code:**
- Execute `grep -rn "\.unwrap()" src/ --include="*.rs"` to identify all unwrap()
occurrences in the src/ directory
- For each result, inspect the surrounding context to determine if it's within a
`///` rustdoc comment block
- Verify the following files specifically:
  - `src/push/queues/mod.rs`
  - `src/server/connection.rs`
  - `src/app/mod.rs`
  - `src/frame/tests.rs`
- Expected result: 9 unwrap() calls total, all in documentation examples
(`src/extractor.rs` has 6, `src/server/config/binding.rs` has 3)

**Audit Test Code:**
- Execute `grep -rn "\.unwrap()" tests/ --include="*.rs"` to search for unwrap()
calls in test files
- Check all test files including:
- `tests/push.rs`, `tests/world.rs`, `tests/wireframe_protocol.rs`,
`tests/stream_end.rs`, `tests/session_registry.rs`, `tests/server.rs`,
`tests/push_policies.rs`, `tests/correlation_id.rs`, `tests/async_stream.rs`
  - `tests/advanced/interaction_fuzz.rs`, `tests/advanced/concurrency_loom.rs`
- `tests/connection_actor_errors.rs`, `tests/connection_actor_fairness.rs`,
`tests/connection_actor_shutdown.rs`
- Expected result: 0 unwrap() calls; tests should already use expect() with
descriptive messages (149+ occurrences)

**Document Findings:**
- Create a summary document comparing the ticket's original claims (46 unwrap
calls across 20 files) with the actual audit results (0 production unwrap, 0
test unwrap, 9 doc example unwrap)
- Note any file path discrepancies found during verification
- Include a statement confirming that rustdoc example unwrap() calls are
acceptable per Rust conventions
- Prepare this documentation for ticket closure or PR reference

Phase 2: Address Any Discovered Issues (Conditional)

If the verification phase discovers any unwrap() calls that were missed by the exploration, apply the appropriate fixes following established codebase patterns.

Task 1: Fix Production Code unwrap() (If Found)

Apply proper error handling to any production-code unwrap() calls discovered during verification.

• For functions already returning Result: Convert unwrap() to ? operator with appropriate error transformation via map_err() or ok_or_else()
• For functions not returning Result: Evaluate if the function signature should change to return Result, or use expect() with a descriptive message if panic is acceptable
• Follow existing error type patterns:
  • Use domain-specific error types from src/*/error.rs modules
  • Use #[from] for automatic error conversion
  • Use ok_or_else() for Option → Result conversion with context
• Reference patterns in src/codec/error.rs and src/app/error.rs for error type structure

Task 2: Fix Test Code unwrap() (If Found)

Convert any discovered test unwrap() calls to expect() with descriptive messages.

• Replace .unwrap() with .expect("descriptive message") following the established pattern
• Use message format: "failed to [verb] [object]" (e.g., "failed to create app", "failed to bind port")
• For push/recv operations, consider using existing macros push_expect! and recv_expect! from wireframe_testing/src/helpers.rs which add file:line context automatically
• Reference existing expect() messages in tests/frame_codec.rs and tests/advanced/concurrency_loom.rs as templates

🤖 Prompt for AI agents

This conditional phase addresses any unwrap() calls discovered during the
verification phase. The goal is to apply proper error handling patterns
consistent with the existing codebase.

**Fix Production Code unwrap() (If Found):**
- For each unwrap() call found in production code, analyze the surrounding
context:
  - If the function already returns a `Result` type:
    - Replace `unwrap()` with the `?` operator
- Use `map_err()` to transform errors into the appropriate domain-specific error
type
- Use `ok_or_else()` for `Option` to `Result` conversions with contextual error
information
  - If the function does not return a `Result`:
- Evaluate whether the function signature should be changed to return a `Result`
type
- If panic behavior is acceptable, replace `unwrap()` with `expect()` using a
descriptive message
- Apply error handling patterns consistent with the codebase:
  - Use domain-specific error types defined in `src/*/error.rs` modules
  - Leverage `#[from]` attribute for automatic error conversion with `thiserror`
  - Follow the hierarchical error structure with `#[source]` attributes
- Reference `src/codec/error.rs` and `src/app/error.rs` for error type structure
examples

**Fix Test Code unwrap() (If Found):**
- For each unwrap() call in test files:
  - Replace `.unwrap()` with `.expect("descriptive message")`
- Follow the message pattern: `"failed to [verb] [object]"` (e.g., "failed to
create app", "failed to bind port")
- For push/recv operations specifically:
- Consider using the custom macros `push_expect!` and `recv_expect!` from
`wireframe_testing/src/helpers.rs`
  - These macros automatically add file:line context to error messages
- Reference existing expect() usage in `tests/frame_codec.rs` and
`tests/advanced/concurrency_loom.rs` for message format examples

---

🚀 Next Steps

🤖 All AI agent prompts combined

Task: 1

This phase verifies the current state of the codebase to confirm that unwrap()
calls have already been addressed. The goal is to audit both production and test
code, then document the findings.

**Audit Production Code:**
- Execute `grep -rn "\.unwrap()" src/ --include="*.rs"` to identify all unwrap()
occurrences in the src/ directory
- For each result, inspect the surrounding context to determine if it's within a
`///` rustdoc comment block
- Verify the following files specifically:
  - `src/push/queues/mod.rs`
  - `src/server/connection.rs`
  - `src/app/mod.rs`
  - `src/frame/tests.rs`
- Expected result: 9 unwrap() calls total, all in documentation examples
(`src/extractor.rs` has 6, `src/server/config/binding.rs` has 3)

**Audit Test Code:**
- Execute `grep -rn "\.unwrap()" tests/ --include="*.rs"` to search for unwrap()
calls in test files
- Check all test files including:
- `tests/push.rs`, `tests/world.rs`, `tests/wireframe_protocol.rs`,
`tests/stream_end.rs`, `tests/session_registry.rs`, `tests/server.rs`,
`tests/push_policies.rs`, `tests/correlation_id.rs`, `tests/async_stream.rs`
  - `tests/advanced/interaction_fuzz.rs`, `tests/advanced/concurrency_loom.rs`
- `tests/connection_actor_errors.rs`, `tests/connection_actor_fairness.rs`,
`tests/connection_actor_shutdown.rs`
- Expected result: 0 unwrap() calls; tests should already use expect() with
descriptive messages (149+ occurrences)

**Document Findings:**
- Create a summary document comparing the ticket's original claims (46 unwrap
calls across 20 files) with the actual audit results (0 production unwrap, 0
test unwrap, 9 doc example unwrap)
- Note any file path discrepancies found during verification
- Include a statement confirming that rustdoc example unwrap() calls are
acceptable per Rust conventions
- Prepare this documentation for ticket closure or PR reference
===============================================================================

Task: 2

This conditional phase addresses any unwrap() calls discovered during the
verification phase. The goal is to apply proper error handling patterns
consistent with the existing codebase.

**Fix Production Code unwrap() (If Found):**
- For each unwrap() call found in production code, analyze the surrounding
context:
  - If the function already returns a `Result` type:
    - Replace `unwrap()` with the `?` operator
- Use `map_err()` to transform errors into the appropriate domain-specific error
type
- Use `ok_or_else()` for `Option` to `Result` conversions with contextual error
information
  - If the function does not return a `Result`:
- Evaluate whether the function signature should be changed to return a `Result`
type
- If panic behavior is acceptable, replace `unwrap()` with `expect()` using a
descriptive message
- Apply error handling patterns consistent with the codebase:
  - Use domain-specific error types defined in `src/*/error.rs` modules
  - Leverage `#[from]` attribute for automatic error conversion with `thiserror`
  - Follow the hierarchical error structure with `#[source]` attributes
- Reference `src/codec/error.rs` and `src/app/error.rs` for error type structure
examples

**Fix Test Code unwrap() (If Found):**
- For each unwrap() call in test files:
  - Replace `.unwrap()` with `.expect("descriptive message")`
- Follow the message pattern: `"failed to [verb] [object]"` (e.g., "failed to
create app", "failed to bind port")
- For push/recv operations specifically:
- Consider using the custom macros `push_expect!` and `recv_expect!` from
`wireframe_testing/src/helpers.rs`
  - These macros automatically add file:line context to error messages
- Reference existing expect() usage in `tests/frame_codec.rs` and
`tests/advanced/concurrency_loom.rs` for message format examples

💡 Iterate on the plan with: @coderabbitai <feedback>

Example Feedback
- `@coderabbitai` You can skip phase 3. Add a simple unit test case for phase 2.
- `@coderabbitai` For assumption 1 go ahead with option 3 and replan.

---

💬 Have feedback or questions? Drop into our discord!