oxidecomputer / oxidecomputer/omicron

sled-agent: Clean up error types and HttpError conversions

Open
#11,158 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

cleanup
Dominant language
Rust
Stars
572
Forks
97
Avg merge
2d 12h
Merged PRs (30d)
96

Description

I have wondered for a while here if we might want to instead do something where each of the individual error variants has its own Into<HttpError> implementation, and this higher-level thing looks like a match that just calls instance::Error::into(), instance_manager::Error::into(), BundleError::into(), and so forth, so that the conversions (and the corresponding big lists of every variant of every enum) are written in the files where those errors are defined, instead of all here, but I'm not sure OTTOMH whether that would actually end up making sense or not --- there might be some reason that didn't work that I've forgotten about?

This sounds reasonable to me, but honestly I'd prefer we break up these giant error types more significantly - it seems a little hard to believe that we really need to represent both "the sled doesn't have a boot disk" and "failed to resolve a name in internal DNS" in a single error type, to pick an a couple variants of this error. That would presumably force us to move the conversions down at least one layer too.

Originally posted by @jgallagher in https://github.com/oxidecomputer/omicron/issues/11142#issuecomment-5401209495

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

The issue names no files, tests, or entry points. Start by locating sled-agent's aggregate error types and HttpError conversions, then evaluate whether the errors should be split and their conversions moved to the defining modules. Done means the error structure and conversion ownership have been decided and consistently updated.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
api, backend
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Needs clarification
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.