ECSError Display implementation prints double messages and unwraps on write failure #671

New issue

Open

opened 2026-07-05 03:22:40 +02:00 by Hermes · 0 comments

Hermes commented

2026-07-05 03:22:40 +02:00

Collaborator

Severity: 🟡 Medium

File: Crates/ecs/src/error.rs:11-22

Bug #1 — Double-printed error messages:
The Display implementation prints the formatted message AND then falls through to writeln!(f, "{self:?}") which prints the Debug representation. Every error message is printed twice with different formatting.

impl Display for ECSError {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        match self {
            ECSError::InvalidEntity(entity) => {
                writeln!(f, "Invalid Entity ...").unwrap()  // first message
            },
            ECSError::ComponentStoreNotExisting => {
                let _ = writeln!(f, "...");  // first message
            }
        }
        writeln!(f, "{self:?}")  // falls through to Debug — second message!
    }
}

Bug #2 — Inconsistent error handling:
The InvalidEntity branch uses .unwrap() on the writeln!() result, which will panic if the formatter encounters an error (e.g., broken pipe). The ComponentStoreNotExisting branch correctly uses let _ =.

Fixes:

Remove the trailing writeln!(f, "{self:?}") or put it in an else.
Use let _ = consistently instead of .unwrap().

@SakulFlee

**Severity:** 🟡 Medium **File:** `Crates/ecs/src/error.rs:11-22` **Bug #1 — Double-printed error messages:** The `Display` implementation prints the formatted message AND then falls through to `writeln!(f, "{self:?}")` which prints the Debug representation. Every error message is printed twice with different formatting. ```rust impl Display for ECSError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { ECSError::InvalidEntity(entity) => { writeln!(f, "Invalid Entity ...").unwrap() // first message }, ECSError::ComponentStoreNotExisting => { let _ = writeln!(f, "..."); // first message } } writeln!(f, "{self:?}") // falls through to Debug — second message! } } ``` **Bug #2 — Inconsistent error handling:** The `InvalidEntity` branch uses `.unwrap()` on the `writeln!()` result, which will **panic** if the formatter encounters an error (e.g., broken pipe). The `ComponentStoreNotExisting` branch correctly uses `let _ =`. **Fixes:** 1. Remove the trailing `writeln!(f, "{self:?}")` or put it in an `else`. 2. Use `let _ =` consistently instead of `.unwrap()`. @SakulFlee

Hermes added the

Context: Bug

Difficulty: Easy

labels

2026-07-05 03:22:40 +02:00