# ADR-0015: Dual Error Handling Strategy — `thiserror` for Domain Errors, `anyhow` for Propagation

## Status

✅ Accepted

## Context

omni-dev is a library-backed CLI binary. Its error surface falls into two distinct
categories with different requirements:

**Domain errors** arise from specific, identifiable failure conditions in a subsystem
— for example, a missing API key, an HTTP rate-limit response, or a prompt that
exceeds a model's token budget. Callers that need to recover from or branch on these
conditions must be able to match the variant. The error message format should be
stable and descriptive, because it reaches the end user verbatim.

**Propagation errors** arise at every layer of glue code — parsing YAML, opening git
repositories, reading config files, spawning subprocesses. These errors are not
recovered from; they terminate the current operation and surface a diagnostic to the
user. What matters is not the error type but the context chain: _why_ did the
operation fail, expressed as a sequence of human-readable annotations added at each
call site.

Four approaches were evaluated:

- **`anyhow` everywhere.** All functions return `anyhow::Result<T>`. Error
  propagation is ergonomic and context chains are easy to build with `.context()`.
  However, callers that need to branch on specific failure conditions must parse
  error messages — a fragile string-matching approach. Domain subsystems lose
  their ability to expose a stable, typed contract.

- **`thiserror` everywhere.** Every module defines its own error enum with explicit
  variants for every failure mode. This is maximally type-safe but imposes high
  boilerplate: propagating an error from one domain into another requires a `From`
  impl or an explicit `.map_err()` at every boundary. Application-level glue
  functions, which compose many domains, accumulate large error enums whose variants
  are never matched.

- **Custom manual error types without macros.** Each module implements `fmt::Display`,
  `std::error::Error`, and any needed `From` impls by hand. This achieves the same
  goals as `thiserror` without the macro dependency but at the cost of significant
  boilerplate. `UtilError` in `src/utils/general.rs` was a pre-existing example of
  this pattern; it predated `thiserror` adoption. It was subsequently found to have
  no callers outside its own file and was removed entirely as dead code (issue #228).

- **`thiserror` for domain errors, `anyhow` for propagation.** Domain subsystems
  expose typed error enums derived with `thiserror`. Application code and glue layers
  return `anyhow::Result<T>`, using `.context()` to annotate errors and `?` to
  propagate them. Because `anyhow` provides a blanket `From<E: std::error::Error>`
  implementation, any `thiserror`-derived type converts to `anyhow::Error`
  automatically — no explicit `From` impls are needed at the boundary.

## Decision

We use `thiserror` for domain-specific error types and `anyhow` for application-level
error propagation.

### Domain error types (`thiserror`)

Domain subsystems that have identifiable, matchable failure modes define a typed error
enum using `#[derive(thiserror::Error)]`. Each variant carries a stable `#[error("...")]`
message. Currently one domain error type exists:

**`ClaudeError`** (`src/claude/error.rs`) — models failures in the Claude API subsystem:

| Variant                  | Condition                                              |
|--------------------------|--------------------------------------------------------|
| `ApiKeyNotFound`         | No API key in the environment                          |
| `ApiRequestFailed(String)` | Non-success HTTP response from the API               |
| `InvalidResponseFormat(String)` | Unexpected structure in an API response         |
| `AmendmentParsingFailed(String)` | Could not extract amendments from the response |
| `PromptTooLarge { … }`  | Assembled prompt exceeds the model's input token budget |
| `RateLimitExceeded`      | HTTP 429 from the API                                  |
| `NetworkError(String)`   | Transport-level failure (DNS, TLS, connection refused) |

Lower-level library errors (`reqwest`, `serde_json`, `url`) are converted to the
appropriate `ClaudeError` variant via `.map_err()` at the point where the
AI-specific meaning is known. The resulting `ClaudeError` is then propagated as
`anyhow::Error` at call sites that do not need to match variants.

Callers that _do_ need to match — for example, to distinguish `PromptTooLarge` from
`RateLimitExceeded` in the retry logic — receive the typed variant directly before
the `.into()` conversion.

### Application-level propagation (`anyhow`)

All other functions return `anyhow::Result<T>`. Context is added with `.context()`
or `.with_context()` at each call site that can add meaningful information. The `?`
operator propagates errors upward; `anyhow::bail!()` and `anyhow::anyhow!()` are
used where an error must be constructed inline.

The conversion from any `thiserror`-derived type to `anyhow::Error` is automatic:
`anyhow` provides `impl<E: std::error::Error + Send + Sync + 'static> From<E> for anyhow::Error`.
No explicit bridge impls are needed.

### Former inconsistency: `UtilError` (resolved)

`UtilError` in `src/utils/general.rs` predated `thiserror` adoption. It implemented
`fmt::Display`, `std::error::Error`, and `From<std::io::Error>` manually without
using the `thiserror` macro. During evaluation for migration it was found to have no
callers outside the file where it was defined — `validate_input` and `format_bytes`
were similarly unreachable. The entire `src/utils/general.rs` module was deleted as
dead code (issue #228, PR #229). The inconsistency no longer exists.

## Consequences

**Positive:**

- **Callers can branch on domain failures.** `ClaudeError` variants are directly
  matchable. The retry logic, token-budget validation, and API key detection can
  all act on specific conditions without parsing strings. The compiler enforces
  exhaustiveness when new variants are added.

- **Propagation is ergonomic.** `anyhow::Result<T>` at application boundaries
  removes the need to define, maintain, or convert through large intermediate
  error enums. A single `?` propagates any error type; `.context()` attaches
  the call-site annotation without creating a new type.

- **Error messages are well-formed end-to-end.** `thiserror`'s `#[error("...")]`
  attribute keeps domain-level messages co-located with their variants. `anyhow`'s
  context chain assembles the final user-visible message from the inside out,
  preserving all intermediate annotations.

- **No explicit bridge code.** `anyhow`'s blanket `From` implementation converts
  any `std::error::Error` automatically. A `ClaudeError` propagates through
  `anyhow` boundaries with a plain `?` — no `.map_err(|e| anyhow::anyhow!(e))`
  wrappers needed.

**Negative:**

- **Two mental models to hold.** Contributors must understand when to use
  `thiserror` (new domain subsystem with matchable conditions) versus `anyhow`
  (application glue, propagation-only paths). The rule is straightforward — domain
  vs. propagation — but requires judgment at the boundary.

- **Typed errors are lost after the first `?` into `anyhow`.** Once a `ClaudeError`
  is converted to `anyhow::Error`, callers can only recover the original type via
  `anyhow::Error::downcast_ref::<ClaudeError>()`. Code that needs to match on the
  error must do so before the conversion, or use downcasting. Currently this
  constraint is handled by keeping retry and validation logic within the Claude
  module, before errors propagate out as `anyhow::Error`.

- ~~**`UtilError` is inconsistent with the `thiserror` convention.**~~ This
  inconsistency was resolved when `src/utils/general.rs` was deleted as dead
  code (issue #228). The codebase now has a single style for domain error
  implementation.

**Neutral:**

- **`thiserror` is a thin macro dependency.** It generates code equivalent to the
  manual impls in `UtilError`. At compile time it adds negligible overhead; at
  runtime the generated code is identical to what `thiserror` replaces. The
  dependency is compile-time only and imposes no runtime cost.

- **`anyhow` errors are opaque at the type level.** In a library context this would
  be a significant constraint — callers cannot pattern-match on `anyhow::Error`
  without downcasting. omni-dev is a binary; `anyhow::Result` appears only in
  internal application code and CLI handlers, not in a public API surface. This
  makes the trade-off acceptable.
