aya-rs / aya

Aya is an eBPF library for the Rust programming language, built with a focus on developer experience and operability.
https://aya-rs.dev/book/
Apache License 2.0
3.25k stars 290 forks source link

WIP: Reduce Explosion of Aya Error Types #1061

Open dave-tucker opened 1 month ago

dave-tucker commented 1 month ago

I have been down a rabbit hole of cleaning up the aya error types πŸ˜…

Most of the important changes are in errors.rs.

TL;DR

Current exposed types are:

Honestly I'm still thinking about how we could collapse those types. Either into a single type, or at least fewer than we expose today.

Within each of those types, I've tried to remove any invariants that don't have any business being public (e.g if a syscall fails with -EINVAL, there is nothing at runtime you can do about it other than bailing).

πŸ‘† (and the spaghetti of errors depending on other errors) are replaced by an Other invariant that's a Box<dyn std::error::Error>.

There are still some pub(crate) XInternalError types, but these are used only to make nice error messages. This could plausibly be replaced with anyhow/context etc.. But I've left it as-is for now.


This change is Reviewable

netlify[bot] commented 1 month ago

Deploy Preview for aya-rs-docs failed.

Name Link
Latest commit e899c8e408f6d8f2cd25c7e777406f62b257197d
Latest deploy log https://app.netlify.com/sites/aya-rs-docs/deploys/67129caada3be000087d808b
mergify[bot] commented 1 month ago

Hey @alessandrod, this pull request changes the Aya Public API and requires your review.

tyrone-wu commented 1 month ago

I was wondering if the errors from aya/src/utils.rs should also be apart of this. They're technically not ebpf specific errors, but I think some parts would align with FileError.

dave-tucker commented 1 month ago

I was wondering if the errors from aya/src/utils.rs should also be apart of this. They're technically not ebpf specific errors, but I think some parts would align with FileError.

Good call! I missed those on the first pass through. Once I've got agreement on the general direction this PR is taking I'll go ahead and clean those up too

mergify[bot] commented 3 weeks ago

@dave-tucker, this pull request is now in conflict and requires a rebase.