Replace anyhow with structured error types in application crates#1761
Replace anyhow with structured error types in application crates#1761Ericson2314 wants to merge 1 commit into
anyhow with structured error types in application crates#1761Conversation
Ericson2314
force-pushed
the
error-types-2
branch
from
4365bd2 to
a0097ea
Compare
| .tls_config(tls)? | ||
| .connect() | ||
| .await | ||
| .context("Failed to establish connection with Channel")? |
There was a problem hiding this comment.
Do we loose error context in cases like this where you prune the error message? I found it quiet useful to have errors at every step.
There was a problem hiding this comment.
Yeah I think the WithContext thing needs to be used in more places.
There was a problem hiding this comment.
The solution is to divide errors by cause, not type. The information that would be put in context should be put in the message with #[error("message")], and the variant should include the relevant source with #[source].
| current_bytes: fs_err::read_to_string(cgroups_path.join("memory.current"))? | ||
| .trim() | ||
| .parse() | ||
| .context("memory current parsing failed")?, |
There was a problem hiding this comment.
Same question here. I haven't read the whole error hierarchy to understand what impact this has on error messages.
Could you write pull request messages yourself, if you expect me to review them? |
Ericson2314
force-pushed
the
error-types-2
branch
from
a0097ea to
5491b18
Compare
Every function now returns the narrowest error type that covers its
actual failure modes, rather than a catch-all `anyhow::Error`. This
makes it possible to distinguish infrastructure failures (DB down, store
unreachable) from logic errors (step not found in queue, drv lookup
miss) at the type level.
Among other benefits, this will assist the "fail-fast" PR in deciding
what is fatal vs recoverable, and making sure those decisions are
consistent and enforced.
Error types are co-located with the functions that produce them, using
split `impl State` blocks where needed. The fine-grained sub-enums (e.g.
`ResolutionError`, `StepLookupError`, `MachineLookupError`,
`DrvLookupError`) might seem like overkill in a single 2500-line
`state/mod.rs`, but they are designed so that a future file split (e.g.
`state/resolution.rs`, `state/step_completion.rs`) would give each
sub-module its own natural error type, with `StateLogicError` in
`mod.rs` just re-exporting them. The error hierarchy becomes the module
hierarchy.
Similarly, the builder's per-phase error types (`ImportError`,
`UploadError`, `BuildOutputParseError`, `ResultInfoError`) are each
defined next to the free functions they belong to, and `JobFailure` ties
them together by build phase. A future split into `import.rs`,
`upload.rs`, etc. would be straightforward.
New `error-context` crate provides `WithContext<E>`, a recursive enum
(`Root(E)` | `Context { context, source }`) that adds context layers
without changing the type — a typed replacement for `anyhow::context()`.
`StateError` is a newtype around `WithContext<StateErrorKind>`.
`anyhow` is removed from workspace dependencies entirely. Examples use
it as a non-workspace `[dev-dependencies]` only.
Ericson2314
force-pushed
the
error-types-2
branch
from
5491b18 to
60f07d0
Compare
3 participants
Every function now returns the narrowest error type that covers its actual failure modes, rather than a catch-all
anyhow::Error. This makes it possible to distinguish infrastructure failures (DB down, store unreachable) from logic errors (step not found in queue, drv lookup miss) at the type level.Among other benefits, this will assist the "fail-fast" PR in deciding what is fatal vs recoverable, and making sure those decisions are consistent and enforced.
Error types are co-located with the functions that produce them, using split
impl Stateblocks where needed. The fine-grained sub-enums (e.g.ResolutionError,StepLookupError,MachineLookupError,DrvLookupError) might seem like overkill in a single 2500-linestate/mod.rs, but they are designed so that a future file split (e.g.state/resolution.rs,state/step_completion.rs) would give each sub-module its own natural error type, withStateLogicErrorinmod.rsjust re-exporting them. The error hierarchy becomes the module hierarchy.Similarly, the builder's per-phase error types (
ImportError,UploadError,BuildOutputParseError,ResultInfoError) are each defined next to the free functions they belong to, andJobFailureties them together by build phase. A future split intoimport.rs,upload.rs, etc. would be straightforward.New
error-contextcrate providesWithContext<E>, a recursive enum (Root(E)|Context { context, source }) that adds context layers without changing the type — a typed replacement foranyhow::context().StateErroris a newtype aroundWithContext<StateErrorKind>.anyhowis removed from workspace dependencies entirely. Examples use it as a non-workspace[dev-dependencies]only.