refactor(client): make managed failure classification exhaustive #90

2026-08-14T18:04:01Z

Lochnair commented

2026-08-14 18:04:01 +00:00

(Migrated from github.com)

Summary

classify_client_error matched ClientError with a trailing _ => Other arm, so two variants (NoTokioRuntime, DatagramLengthMismatch) were silently classified as Other, and any future variant would have joined them without anyone noticing.
The match is now exhaustive over every ClientError variant. NoTokioRuntime classifies as InvalidState (an operation performed in the wrong runtime context, like the other API-misuse errors), and DatagramLengthMismatch as Socket (a UDP transport invariant failure).
ManagedTargetFailureKind::Other is kept, but only as a deliberate category: nothing maps to it by default, and that is asserted by a test.
ManagedTargetFailurePhase and ManagedTargetFailureKind gained controlled Display impls, and the CLI client diagnostics and TUI warning/status text now use them instead of {:?}.

Behavior

Adding a new ClientError variant now fails to compile at this seam until it is classified intentionally.
User-visible failure text changes spelling only: target edge failed (Receiving Protocol): … is now target edge failed (receiving protocol): …, and the TUI's cleanup failed (Closing Socket) is now cleanup failed (closing socket). No classification of an existing variant changed except the two that previously fell through to Other.
No public type, variant, or outcome semantics were removed or renamed.

Scope

Explicitly not changed:

ClientError itself — no variants added, removed, renamed, or re-shaped, and #[non_exhaustive] is retained.
ManagedTargetFailure / ManagedTargetOutcome structure and durable managed counters.
Managed event or outcome semantics.
CLI layout, rows, or which diagnostics are emitted.

Verification

Run from the task worktree, all successful:

cargo fmt --all -- --check
cargo check --workspace --all-targets --all-features --locked
cargo test --workspace --all-targets --all-features --locked
cargo clippy --workspace --all-targets --all-features --locked -- -D warnings
RUSTDOCFLAGS="-D warnings" cargo doc --workspace --all-features --no-deps --locked
cargo check -p irtt-cli --no-default-features --features client --all-targets --locked
cargo check -p irtt-cli --no-default-features --features tui --all-targets --locked
cargo check -p irtt-cli --no-default-features --features server --all-targets --locked
git diff --check

## Summary - `classify_client_error` matched `ClientError` with a trailing `_ => Other` arm, so two variants (`NoTokioRuntime`, `DatagramLengthMismatch`) were silently classified as `Other`, and any future variant would have joined them without anyone noticing. - The match is now exhaustive over every `ClientError` variant. `NoTokioRuntime` classifies as `InvalidState` (an operation performed in the wrong runtime context, like the other API-misuse errors), and `DatagramLengthMismatch` as `Socket` (a UDP transport invariant failure). - `ManagedTargetFailureKind::Other` is kept, but only as a deliberate category: nothing maps to it by default, and that is asserted by a test. - `ManagedTargetFailurePhase` and `ManagedTargetFailureKind` gained controlled `Display` impls, and the CLI client diagnostics and TUI warning/status text now use them instead of `{:?}`. ## Behavior - Adding a new `ClientError` variant now fails to compile at this seam until it is classified intentionally. - User-visible failure text changes spelling only: `target edge failed (Receiving Protocol): …` is now `target edge failed (receiving protocol): …`, and the TUI's `cleanup failed (Closing Socket)` is now `cleanup failed (closing socket)`. No classification of an existing variant changed except the two that previously fell through to `Other`. - No public type, variant, or outcome semantics were removed or renamed. ## Scope Explicitly not changed: - `ClientError` itself — no variants added, removed, renamed, or re-shaped, and `#[non_exhaustive]` is retained. - `ManagedTargetFailure` / `ManagedTargetOutcome` structure and durable managed counters. - Managed event or outcome semantics. - CLI layout, rows, or which diagnostics are emitted. ## Verification Run from the task worktree, all successful: - `cargo fmt --all -- --check` - `cargo check --workspace --all-targets --all-features --locked` - `cargo test --workspace --all-targets --all-features --locked` - `cargo clippy --workspace --all-targets --all-features --locked -- -D warnings` - `RUSTDOCFLAGS="-D warnings" cargo doc --workspace --all-features --no-deps --locked` - `cargo check -p irtt-cli --no-default-features --features client --all-targets --locked` - `cargo check -p irtt-cli --no-default-features --features tui --all-targets --locked` - `cargo check -p irtt-cli --no-default-features --features server --all-targets --locked` - `git diff --check`

👍 1

chatgpt-codex-connector[bot] (Migrated from github.com) reviewed

2026-08-14 18:08:44 +00:00

chatgpt-codex-connector[bot] (Migrated from github.com) left a comment

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e3003c348f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

### 💡 Codex Review Here are some automated review suggestions for this pull request. **Reviewed commit:** `e3003c348f` <details> <summary>ℹ️ About Codex in GitHub</summary> [Your team has set up Codex to review pull requests in this repo](https://chatgpt.com/codex/cloud/settings/general). Reviews are triggered when you - Open a pull request for review - Mark a draft as ready - Comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". </details>

crates/irtt-client/src/managed/model.rs

						
				@@ -375,13 +417,15 @@ pub(crate) fn classify_client_error(

				        | ClientError::CounterOverflow { .. }

chatgpt-codex-connector[bot] (Migrated from github.com) commented

2026-08-14 18:08:44 +00:00

Remove the coverage-only category enumeration

This test only verifies that every failure-kind bucket appears in the same hand-maintained case table already exercised by the preceding test; it cannot establish exhaustiveness or absence of a fallback, which the production match enforces at compile time. It instead pins the incidental fact that each category currently has an input, so a valid future regrouping would require unrelated test churn. Repository guidance explicitly discourages tests that exercise helpers solely for coverage or duplicate stronger coverage.

AGENTS.md reference: AGENTS.md:L93-L101

Useful? React with 👍 / 👎.

**![P2 Badge](https://img.shields.io/badge/P2-yellow?style=flat) Remove the coverage-only category enumeration** This test only verifies that every failure-kind bucket appears in the same hand-maintained case table already exercised by the preceding test; it cannot establish exhaustiveness or absence of a fallback, which the production match enforces at compile time. It instead pins the incidental fact that each category currently has an input, so a valid future regrouping would require unrelated test churn. Repository guidance explicitly discourages tests that exercise helpers solely for coverage or duplicate stronger coverage. AGENTS.md reference: [AGENTS.md:L93-L101](https://github.com/Lochnair/irtt-rs/blob/e3003c348fcba5b86955b9f514950db3d76ff759/AGENTS.md#L93-L101) Useful? React with 👍 / 👎.

Lochnair (Migrated from github.com) reviewed

2026-08-14 18:49:40 +00:00

crates/irtt-client/src/managed/model.rs

						
				@@ -375,13 +417,15 @@ pub(crate) fn classify_client_error(

				        | ClientError::CounterOverflow { .. }

Lochnair (Migrated from github.com) commented

2026-08-14 18:49:40 +00:00

Agreed — removed the test entirely; the mapping table plus compile-time exhaustiveness already carry that weight without pinning which categories happen to have inputs.

🤖 Addressed by Claude Code

Agreed — removed the test entirely; the mapping table plus compile-time exhaustiveness already carry that weight without pinning which categories happen to have inputs. _🤖 Addressed by [Claude Code](https://claude.com/claude-code)_

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Lochnair-Labs/irtt-rs#90

				`@@ -375,13 +417,15 @@ pub(crate) fn classify_client_error(`
				`\| ClientError::CounterOverflow { .. }`