Skip to content

Replace anyhow with structured error types in support crates#1755

Merged
Ericson2314 merged 2 commits into
NixOS:masterfrom
obsidiansystems:error-types
May 20, 2026
Merged

Replace anyhow with structured error types in support crates#1755
Ericson2314 merged 2 commits into
NixOS:masterfrom
obsidiansystems:error-types

Conversation

@Ericson2314
Copy link
Copy Markdown
Member

@Ericson2314 Ericson2314 commented May 19, 2026

Introduce typed error enums so callers can distinguish error kinds
(e.g. DB infrastructure failure vs bad data) without string matching.
This lays the groundwork for the future error handling cleanups, by
allowing us to see more clearly what changes are made.

db crate:

  • New db::Error enum: Sql (infrastructure) vs Data(DataError)
  • DataError has StorePath, StorePathName, IntConversion, and
    BuildProductMissingPath { build_id, productnr } variants
  • BuildProductRow now carries build and productnr from the query
    so error messages identify the offending row
  • Removed anyhow dependency

nix-utils crate:

  • Replaced Error::Anyhow catch-all with DerivationParse and
    StorePathName variants
  • Fixed CA fixed-output test to use correct output path
  • Removed anyhow dependency

store-transfer crate:

  • New Error enum: Store, Grpc, Io, Protocol, Join
  • ProtocolError sub-enum: EmptyStream, MissingHeader,
    MissingGrpcField, InvalidPathInfo, TruncatedNar
  • Removed anyhow dependency

nix-support crate:

  • anyhow::Resultstd::io::Result (all errors are IO)
  • Removed anyhow dependency

hydra-tracing crate:

  • anyhow::ResultResult<_, Box<dyn Error>> (init-only)
  • Removed anyhow dependency

binary-cache crate:

  • InvalidS3Scheme and InvalidCompression error structs with
    named got field replace String error variants
  • UrlParseError variants renamed for clarity

proto crate:

  • NarInfoConvertError now implements std::error::Error

Also adds sqlx-prepare.sh for regenerating the sqlx offline cache.

@Ericson2314 Ericson2314 force-pushed the error-types branch 2 times, most recently from aa464be to 8f51404 Compare May 19, 2026 22:55
@Ericson2314 Ericson2314 marked this pull request as ready for review May 19, 2026 22:56
@Ericson2314 Ericson2314 changed the title WIP structured errors Replace anyhow with structured error types in support crates May 19, 2026
@Mic92
Copy link
Copy Markdown
Member

Mic92 commented May 20, 2026

Sounds good. This composes better with the underlying libraries

@Mic92
Copy link
Copy Markdown
Member

Mic92 commented May 20, 2026

Should the postgres server not also be stopped in the script?

Update missed the trap

Copy link
Copy Markdown
Member Author

@Ericson2314 Ericson2314 May 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: end applications are done in the next PR

Copy link
Copy Markdown
Member

@Conni2461 Conni2461 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left two nits :D other than that, we can merge this

&store_dir,
&drv_path,
format!(
r#"Derive([("out","/nix/store/{HASH}-test-src","sha256","deadbeef00000000000000000000000000000000000000000000000000000000")],[],[],"{0}","{0}",[],[("name","test-src")])"#,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i still think we want {HASH}, a fake hash here

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a preexisting error (should be separate commit). This is actually correct, the ATerm parser will complain if the hash and the store path do not agree, which is good.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unrelated question but are we running cargo test currently in CI?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No we aren't, it wasn't working when I first tried, only impurely. That would be a good thing to fix.

Comment thread subprojects/crates/tracing/src/lib.rs Outdated
amaanq and others added 2 commits May 20, 2026 12:55
Harmonia's ATerm parser validates that CAFixed output paths match the
content hash. The placeholder `aaaa...` was accepted by the older
parser but is now rejected with `CAFixed output path mismatch`. Use
the correct base32 hash that sha256:deadbeef... produces for name
"test-src".
Introduce typed error enums so callers can distinguish error kinds
(e.g. DB infrastructure failure vs bad data) without string matching.
This lays the groundwork for the future error handling cleanups, by
allowing us to see more clearly what changes are made.

db crate:
- New `db::Error` enum: `Sql` (infrastructure) vs `Data(DataError)`
- `DataError` has `StorePath`, `StorePathName`, `IntConversion`, and
  `BuildProductMissingPath { build_id, productnr }` variants
- `BuildProductRow` now carries `build` and `productnr` from the query
  so error messages identify the offending row
- Removed `anyhow` dependency

nix-utils crate:
- Replaced `Error::Anyhow` catch-all with `DerivationParse` and
  `StorePathName` variants
- Fixed CA fixed-output test to use correct output path
- Removed `anyhow` dependency

store-transfer crate:
- New `Error` enum: `Store`, `Grpc`, `Io`, `Protocol`, `Join`
- `ProtocolError` sub-enum: `EmptyStream`, `MissingHeader`,
  `MissingGrpcField`, `InvalidPathInfo`, `TruncatedNar`
- Removed `anyhow` dependency

nix-support crate:
- `anyhow::Result` → `std::io::Result` (all errors are IO)
- Removed `anyhow` dependency

hydra-tracing crate:
- `anyhow::Result` → `Result<_, Box<dyn Error>>` (init-only)
- Removed `anyhow` dependency

binary-cache crate:
- `InvalidS3Scheme` and `InvalidCompression` error structs with
  named `got` field replace `String` error variants
- `UrlParseError` variants renamed for clarity

proto crate:
- `NarInfoConvertError` now implements `std::error::Error`

Also adds `sqlx-prepare.sh` for regenerating the sqlx offline cache.
@Ericson2314
Copy link
Copy Markdown
Member Author

OK Thanks! All the Box<dyn Error + ....> is gone!

@Ericson2314 Ericson2314 added this pull request to the merge queue May 20, 2026
Merged via the queue into NixOS:master with commit b25de0c May 20, 2026
2 checks passed
@Ericson2314 Ericson2314 deleted the error-types branch May 20, 2026 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants