Split up and refactor Status enum #2097
Description
I feel like the Status enum has grown beyond its original purpose and has become too big and complicated.
The main problem is that it mixes up two things which I think should be separated: (1) the actual outcome of the request, and (2) how that outcome should be interpreted. Another problem is that the enum has grown data fields in the form of the Redirected variant. This history information should really be separate from the actual status and stored in a struct.
The presence of functions like Status::is_success, is_error, etc shows that the shape of the enum does not match the shape of the information you want it to store - ideally, these would all be match statements on a flat enum type. Functions like is_success_ignoring_timeouts are also desiring more flexibility than the current types can provide.
I've sketched out a rough idea for splitting it up into two parts:
- Outcome, the objective outcome of the check, and
- Verdict or Conclusion, the interpretation of the Outcome after taking into account --accept, etc.
(This naming is taken from Github actions)
/// Outcome of the checker, without any presumption of success or failure.
pub enum Outcome {
Website {
result: Result<StatusCode, reqwest::Error>,
redirects: Redirects,
fragment_check: Option<bool>,
},
Mail {
result: mailify_lib::CheckResult,
},
File {
result: Result<Metadata, io::Error>,
applied_fallback_extension: Option<String>,
applied_index_file: Option<String>,
fragment_check: Option<bool>,
},
Excluded,
RequestError(RequestError),
Unsupported(ErrorKind),
Cached(CacheStatus),
}
/// Response to a lychee Request. Bundles an Outcome along with metadata
/// (like source location) and pre-check transformations like remaps.
pub struct Response {
outcome: Outcome,
request: Request, // <-- remap_history is inside Request
input_source: crate::InputSource,
response_body: crate::ResponseBody,
}
/// An interpretation of an [`Outcome`], taking into account user-provided
/// configuration. Corresponds to counters in the CLI output.
pub enum Conclusion {
Ok,
Error,
Timeout,
Redirected,
Cached,
Excluded,
}
impl Conclusion {
/// For example.
pub fn from_response(
response: Response,
accept: StatusCodeSelector,
accept_timeouts: bool,
other_things: MoreOptions,
) -> Conclusion {
Self::Ok
}
}Obviously this is all open to change. Let me know what you think.
Here's a comment where I mentioned this before: #2063 (comment)