S36: panic! used for normal errors

S36Severity: High

An expected failure stops the program instead of returning an error.

Clippy: panic, manual_assert

What it looks like

pub struct Account {
    pub balance_cents: u64,
}

impl Account {
    pub fn withdraw(&mut self, cents: u64) {
        if cents > self.balance_cents {
            panic!(
                "insufficient funds: balance {}, requested {cents}",
                self.balance_cents
            );
        }
        self.balance_cents -= cents;
    }
}

Why AI agents produce it

panic! looks like throw. In Java, throw new IllegalStateException("insufficient funds") is normal, and a caller can catch it. The agent translates throw to panic!.
It keeps the signature simple. fn withdraw(&mut self, cents: u64) has no Result, so no caller needs to change. The agent does not see that every caller now has a hidden failure mode.
Clippy suggests a different panic. Clippy’s manual_assert lint suggests assert!(cents <= self.balance_cents, ...) instead of if ... { panic!(...) }. That is shorter, but it is still a panic. An agent that “fixes the clippy warning” keeps the problem.

Why it is a problem

A panic is not an exception. It cannot be caught in normal code. It unwinds and stops the thread. In a web server, the request usually fails with no response. In a CLI tool, the program exits. If the code holds a Mutex, the lock is “poisoned” for every other thread.
The caller cannot see it. The signature does not say that withdraw can fail. The only way to know is to read the body.
The caller cannot handle it. “Insufficient funds” is a normal case: show a message, offer a smaller amount. With a panic, there is no place to do that.
Some builds abort instead of unwinding. With panic = "abort" in Cargo.toml, a panic stops the whole process at once.

The better version

WorseAn expected failure is a panic. The signature does not show it.

pub struct Account {
    pub balance_cents: u64,
}

impl Account {
    pub fn withdraw(&mut self, cents: u64) {
        if cents > self.balance_cents {
            panic!(
                "insufficient funds: balance {}, requested {cents}",
                self.balance_cents
            );
        }
        self.balance_cents -= cents;
    }
}

BetterAn error enum with the details, checked_sub for the arithmetic, and the balance is unchanged on failure.

#[derive(Debug, PartialEq, Eq)]
pub enum WithdrawError {
    InsufficientFunds {
        balance_cents: u64,
        requested_cents: u64,
    },
}

pub struct Account {
    pub balance_cents: u64,
}

impl Account {
    pub fn withdraw(&mut self, cents: u64) -> Result<(), WithdrawError> {
        let Some(rest) = self.balance_cents.checked_sub(cents) else {
            return Err(WithdrawError::InsufficientFunds {
                balance_cents: self.balance_cents,
                requested_cents: cents,
            });
        };
        self.balance_cents = rest;
        Ok(())
    }
}

checked_sub returns None when the result would be negative. It does the check and the subtraction in one step, so the check cannot drift away from the arithmetic in a later change. See Result, not exceptions.

When it is acceptable

A panic is correct for bugs: states that the program’s own code should never produce.

Invariants that a caller in your code must keep, with a # Panics section in the documentation, and a non-panicking alternative for input from outside:

pub struct Percent(u8);

impl Percent {
    /// For values from code, such as constants. A value over 100 is a bug
    /// in the caller, so the panic is documented and intended.
    ///
    /// # Panics
    ///
    /// If `value` is greater than 100.
    pub fn new(value: u8) -> Self {
        assert!(value <= 100, "percent must be 0 to 100, got {value}");
        Self(value)
    }

    /// For values from users or files: returns `None` instead.
    pub fn parse(value: u8) -> Option<Self> {
        (value <= 100).then_some(Self(value))
    }

    pub fn get(&self) -> u8 {
        self.0
    }
}

unreachable!() after a match that the code has already narrowed, with a comment that says why.
todo!() in work in progress, never in a merged pull request.
Tests: assert!, assert_eq! and panic! are how tests fail.
Startup, when the program cannot do anything useful: a required configuration value is missing. Even there, returning an error from main gives a cleaner message.

How to detect it

Search for panic!(, assert!(, unreachable!(, todo!( and unimplemented!( outside tests. For each one, ask: can this happen with valid use of the program, or with bad input?
Clippy panic (restriction group, off by default) reports every panic!. Clippy manual_assert (pedantic group) reports if ... { panic!(...) }, but its suggestion is still a panic.
Look at functions with no Result in their signature that do I/O, parse input, or apply business rules. They often hide a panic.

What to tell your agent

Account::withdraw must not panic. Return Result<(), WithdrawError>, with an
InsufficientFunds variant that has the balance and the requested amount. Use
checked_sub for the subtraction. Leave the balance unchanged on failure.
Update the callers to handle the error, and add a test for a withdrawal that
is larger than the balance.