`WireframeError<E>` should derive `Display` and `std::error::Error`

[leynos]

WireframeError<E> is a public library error type but derives only Debug. Without Display and std::error::Error, it cannot participate in the standard error ecosystem: it cannot be wrapped by anyhow, returned through Box<dyn Error>, used with thiserror's #[from] or #[source], or inspected via std::error::Error::source.

The blocking constraint is the Protocol(E) variant. std::error::Error requires Display, and Display requires E: Display. The default E = () satisfies neither, so a single blanket impl cannot cover the full type.

---

Recommended approach: two complementary impls

1. Blanket impl covering all real protocol-error types:

use std::fmt;

impl<E: fmt::Display + fmt::Debug> fmt::Display for WireframeError<E> {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        match self {
            Self::DuplicateRoute(id) => write!(f, "duplicate route: {id}"),
            Self::Io(e)             => write!(f, "I/O error: {e}"),
            Self::Protocol(e)       => write!(f, "protocol error: {e}"),
            Self::Codec(e)          => write!(f, "codec error: {e}"),
        }
    }
}

impl<E: std::error::Error + 'static> std::error::Error for WireframeError<E> {
    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
        match self {
            Self::Io(e)    => Some(e),
            Self::Codec(e) => Some(e),
            Self::Protocol(e) => Some(e),
            Self::DuplicateRoute(_) => None,
        }
    }
}

2. Specific impl for the E = () default (unlocks #[from] in TestError):

impl fmt::Display for WireframeError<()> {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        match self {
            Self::DuplicateRoute(id) => write!(f, "duplicate route: {id}"),
            Self::Io(e)             => write!(f, "I/O error: {e}"),
            Self::Protocol(())      => write!(f, "protocol error"),
            Self::Codec(e)          => write!(f, "codec error: {e}"),
        }
    }
}

impl std::error::Error for WireframeError<()> {
    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
        match self {
            Self::Io(e)    => Some(e),
            Self::Codec(e) => Some(e),
            _              => None,
        }
    }
}

These two impls are not overlapping because () does not implement std::error::Error.

---

Downstream effect on TestError

Once WireframeError<()> implements std::error::Error, the manual From impl in src/testkit/result.rs can be replaced with #[from]:

#[error(transparent)]
Wireframe(#[from] WireframeError),

and the impl From<WireframeError> for TestError block deleted entirely.

---

Suggested path

This change is small, additive, and non-breaking (adding new trait impls cannot break existing callers).

[leynos]

@CodeRabbit plan

[coderabbitai]

Caution

CodeRabbit encountered an error during planning. Please try again later.

[leynos]

@CodeRabbit plan

[coderabbitai]

Coding Plan

Summary

• Modify the existing Display impl to use E: Display + Debug bounds with {} formatting for the Protocol variant, enabling proper user-facing error messages.
• Add a specific impl std::error::Error for WireframeError<()> that coexists with the blanket impl because () does not implement Error.
• Replace the manual From<WireframeError> impl in TestError with thiserror's #[from] attribute, simplifying the testkit error handling.
• Update tests to reflect the new Display format and verify the new Error impl for the unit-type specialisation.

Design Choices

Design Choice 1: Display format for the Protocol variant

Options Considered:

1. Keep E: Debug bound and {:?} format — minimal change, existing tests pass unmodified.
2. Change to E: Display + Debug bound and {} format — aligns with ticket recommendation and produces cleaner user-facing messages.

Chosen Option: 2

Rationale: The ticket explicitly specifies E: Display + Debug bounds and using {e} formatting. This produces more natural error messages and aligns with the convention that Display is for user-facing output.

💡 User Tips

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

Implementation Steps

Phase 1: Core Trait Implementations in src/error.rs

Modify the existing Display implementation and add a specific Error implementation for the unit-type default. The two Error impls (blanket and specific) do not overlap because () does not implement std::error::Error.

Task 1: Update the Display Implementation

Change the blanket Display impl to use E: Display + Debug bounds and format the Protocol variant with {} instead of {:?}.

• In src/error.rs, modify the existing impl<E: std::fmt::Debug> std::fmt::Display for WireframeError<E> block
• Change the trait bound from E: std::fmt::Debug to E: std::fmt::Display + std::fmt::Debug
• Change the Protocol arm from write!(f, "protocol error: {error:?}") to write!(f, "protocol error: {error}")
• Retain the existing format strings for all other variants (DuplicateRoute, Io, Codec)

Task 2: Add Error Implementation for WireframeError<()>

Add a specific std::error::Error impl for the unit-type specialisation that does not attempt to return a source for the Protocol variant.

• In src/error.rs, add a new impl block: impl std::error::Error for WireframeError<()>
• Implement source() to return Some(e) for Io and Codec variants, and None for DuplicateRoute and Protocol(())
• Place this impl after the existing blanket Error impl for logical grouping

🤖 Prompt for AI agents

Modify `src/error.rs` to update the `Display` impl and add a unit-type-specific
`Error` impl for `WireframeError<()>`.

**Task 1: Update the Display Implementation**
- Locate the existing `impl<E: std::fmt::Debug> std::fmt::Display for
WireframeError<E>` block
- Change the trait bound from `E: std::fmt::Debug` to `E: std::fmt::Display +
std::fmt::Debug`
- In the `Protocol` match arm, change `write!(f, "protocol error: {error:?}")`
to `write!(f, "protocol error: {error}")`
- Leave all other variant format strings (`DuplicateRoute`, `Io`, `Codec`)
unchanged

**Task 2: Add `impl std::error::Error for WireframeError<()>`**
- After the existing blanket `std::error::Error` impl, add a new impl block
specifically for `WireframeError<()>`
- Implement `source()` returning `Some(e)` for the `Io` and `Codec` variants,
and `None` for `DuplicateRoute` and `Protocol(())`
- Do not add a `#[from]` or derive — write the impl block explicitly

Phase 2: Downstream Integration

Update the testkit error type to leverage the new Error implementation, and update tests to reflect the changed Display format.

Task 1: Replace Manual From Impl in TestError

Now that WireframeError<()> implements std::error::Error, thiserror's #[from] attribute works correctly.

• In src/testkit/result.rs, locate the Wireframe(WireframeError) variant (around line 25)
• Change the annotation from #[error("wireframe error: {0}")] to #[error(transparent)]
• Add #[from] attribute to the variant: Wireframe(#[from] WireframeError)
• Delete the manual impl From<WireframeError> for TestError block (lines 98–100)

Task 2: Update Display Format Tests

Adjust test expectations to account for the Display-based (rather than Debug-based) formatting of the Protocol variant.

• In tests/error_display.rs, locate the wireframe_error_messages test
• Update the expected string for the Protocol variant: if ProtoErr's Display outputs "boom", change the expectation from "protocol error: ProtoErr" to "protocol error: boom"
• Verify that the ProtoErr fixture struct implements Display (it should already, per the exploration findings)

Task 3: Add Test for WireframeError<()> Error Implementation

Add a test verifying that WireframeError<()> now implements std::error::Error and that source() behaves correctly.

• In tests/error_display.rs, add a new test function (e.g., wireframe_error_unit_implements_error)
• Construct WireframeError::<()>::Io(...) and WireframeError::<()>::Codec(...) and verify source() returns Some
• Construct WireframeError::<()>::Protocol(()) and WireframeError::<()>::DuplicateRoute(...) and verify source() returns None
• Use the existing pattern of .source().expect(...) followed by downcast_ref for type verification

🤖 Prompt for AI agents

Update the testkit and tests to integrate with the new `Error` impl and
corrected Display format.

**Task 1: Replace the manual `From` impl in `src/testkit/result.rs`**
- Locate the `Wireframe(WireframeError)` variant (around line 25)
- Replace the `#[error("wireframe error: {0}")]` annotation with
`#[error(transparent)]`
- Add the `#[from]` attribute to the variant field: `Wireframe(#[from]
WireframeError)`
- Delete the manual `impl From<WireframeError> for TestError` block (lines
98–100)

**Task 2: Update Display format test expectations in `tests/error_display.rs`**
- Locate the `wireframe_error_messages` test
- Update the expected string for the Protocol variant from `"protocol error:
ProtoErr"` to `"protocol error: boom"` (or whatever `ProtoErr`'s Display impl
outputs)
- Confirm the `ProtoErr` fixture struct already implements `Display`

**Task 3: Add a new test function `wireframe_error_unit_implements_error` in
`tests/error_display.rs`**
- Construct `WireframeError::<()>::Io(...)` and
`WireframeError::<()>::Codec(...)` — assert that `source()` returns `Some` for
each, and use `downcast_ref` to confirm the inner type
- Construct `WireframeError::<()>::Protocol(())` and
`WireframeError::<()>::DuplicateRoute(...)` — assert that `source()` returns
`None` for each
- Follow the existing test patterns in the file for consistency

Research

Codebase Summary

WireframeError<E> is defined in src/error.rs with four variants: DuplicateRoute(u32), Io(std::io::Error), Protocol(E), and Codec(CodecError). The current Display impl uses bound E: Debug and formats the Protocol variant with {:?}. The current std::error::Error impl uses bound E: Debug + Error + 'static, which excludes WireframeError<()> since () does not implement Error. The testkit module (src/testkit/result.rs) contains TestError with a manual From<WireframeError> impl that exists solely because thiserror's #[from] requires the source type to implement std::error::Error. Tests in tests/error_display.rs verify Display formatting and Error::source() behaviour using a minimal ProtoErr fixture struct.

---

🚀 Next Steps

🤖 All AI agent prompts combined

Task: 1

Modify `src/error.rs` to update the `Display` impl and add a unit-type-specific
`Error` impl for `WireframeError<()>`.

**Task 1: Update the Display Implementation**
- Locate the existing `impl<E: std::fmt::Debug> std::fmt::Display for
WireframeError<E>` block
- Change the trait bound from `E: std::fmt::Debug` to `E: std::fmt::Display +
std::fmt::Debug`
- In the `Protocol` match arm, change `write!(f, "protocol error: {error:?}")`
to `write!(f, "protocol error: {error}")`
- Leave all other variant format strings (`DuplicateRoute`, `Io`, `Codec`)
unchanged

**Task 2: Add `impl std::error::Error for WireframeError<()>`**
- After the existing blanket `std::error::Error` impl, add a new impl block
specifically for `WireframeError<()>`
- Implement `source()` returning `Some(e)` for the `Io` and `Codec` variants,
and `None` for `DuplicateRoute` and `Protocol(())`
- Do not add a `#[from]` or derive — write the impl block explicitly
===============================================================================

Task: 2

Update the testkit and tests to integrate with the new `Error` impl and
corrected Display format.

**Task 1: Replace the manual `From` impl in `src/testkit/result.rs`**
- Locate the `Wireframe(WireframeError)` variant (around line 25)
- Replace the `#[error("wireframe error: {0}")]` annotation with
`#[error(transparent)]`
- Add the `#[from]` attribute to the variant field: `Wireframe(#[from]
WireframeError)`
- Delete the manual `impl From<WireframeError> for TestError` block (lines
98–100)

**Task 2: Update Display format test expectations in `tests/error_display.rs`**
- Locate the `wireframe_error_messages` test
- Update the expected string for the Protocol variant from `"protocol error:
ProtoErr"` to `"protocol error: boom"` (or whatever `ProtoErr`'s Display impl
outputs)
- Confirm the `ProtoErr` fixture struct already implements `Display`

**Task 3: Add a new test function `wireframe_error_unit_implements_error` in
`tests/error_display.rs`**
- Construct `WireframeError::<()>::Io(...)` and
`WireframeError::<()>::Codec(...)` — assert that `source()` returns `Some` for
each, and use `downcast_ref` to confirm the inner type
- Construct `WireframeError::<()>::Protocol(())` and
`WireframeError::<()>::DuplicateRoute(...)` — assert that `source()` returns
`None` for each
- Follow the existing test patterns in the file for consistency

💡 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 design choice 1 go ahead with option 3 and replan.

---

💬 Have feedback or questions? Drop into our discord!