search

habitat-sh / habitat

Modern applications with built-in automation
https://www.habitat.sh
Apache License 2.0
2.6k stars 314 forks source link

Modularize our Supervisor errors to make them richer and more meaningful #6557

Open christophermaier opened 5 years ago

christophermaier commented 5 years ago

Our error.rs module is a bit of a dumping ground. Every error that could possibly be thrown from anywhere in the entire crate is mapped directly back to here. This makes it difficult to discern any kind of order or structure to the errors.

Additionally, there are a large number of From implementations that map errors from our dependency crates into trivial wrapper errors in our crate. The fact that From is used for all ? invocations, coupled with the fact that the wrappers are trivial, means that we lose a lot of potentially valuable context. For example, what does Error::TryRecvError mean in the context of the Supervisor? Currently, it just means that something, somewhere, threw a std::sync::mpsc::TryRecvError. Absent some in-depth code reading, you can't easily discover where in the code that error might have come from (let alone where it actually did come from), and there's no indication of what that error actually means, in concrete domain terms, at the point it was thrown.

Rather than having a single "dumping ground", I think that we can structure things a bit more hierarchically. That is, we could have a distinct error.rs module for each meaningful subsystem of the Supervisor, full of meaningful domain-specific error types (where exactly we draw the lines between those subsystems is an exercise for the reader 😉 ). These types could in turn be collected and consolidated into a single top-level error type for interoperability. Rather than indiscriminately using ? and From implementations to map from errors in dependency crates, we can explicitly map to our domain errors. That is, instead of using ?, we could use .map_err(MySpecificError). This stands to make the error types much richer and more meaningful.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We value your input and contribution. Please leave a comment if this issue still affects you.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We value your input and contribution. Please leave a comment if this issue still affects you.

stale[bot] commented 1 year ago

This issue has been automatically closed after being stale for 400 days. We still value your input and contribution. Please re-open the issue if desired and leave a comment with details.