From 4a00c1fe7793c0a1ede33882540cd45be3804ba4 Mon Sep 17 00:00:00 2001
From: Sergio Benitez <sb@sergio.bz>
Date: Mon, 3 Jun 2024 20:11:20 -0700
Subject: [PATCH] Improve 'Error' type: make 'ErrorKind' accessible.

This commit improves the 'Error' type such that:
  - It is now fully documented.
  - The `ErrorKind` enum variant fields are all publicly reachable.
  - The `Sentry` type is exposed.

This is a breaking change:
  - `ErrorKind::Collisions` is now struct-like with two fields.
---
 core/codegen/tests/route-ranking.rs |  2 +-
 core/lib/src/error.rs               | 97 +++++++++++++++++++++++------
 core/lib/src/lib.rs                 |  2 +-
 core/lib/src/rocket.rs              |  2 +-
 core/lib/src/router/router.rs       | 10 +--
 core/lib/src/sentinel.rs            | 50 ++++++++++++++-
 core/lib/src/trace/traceable.rs     | 11 +---
 7 files changed, 135 insertions(+), 39 deletions(-)

diff --git a/core/codegen/tests/route-ranking.rs b/core/codegen/tests/route-ranking.rs
index 103e5052e8..075e8268ee 100644
--- a/core/codegen/tests/route-ranking.rs
+++ b/core/codegen/tests/route-ranking.rs
@@ -46,7 +46,7 @@ fn test_rank_collision() {
     let rocket = rocket::build().mount("/", routes![get0, get0b]);
     let client_result = Client::debug(rocket);
     match client_result.as_ref().map_err(|e| e.kind()) {
-        Err(ErrorKind::Collisions(..)) => { /* o.k. */ },
+        Err(ErrorKind::Collisions { .. }) => { /* o.k. */ },
         Ok(_) => panic!("client succeeded unexpectedly"),
         Err(e) => panic!("expected collision, got {}", e)
     }
diff --git a/core/lib/src/error.rs b/core/lib/src/error.rs
index 05e955a625..808b79d213 100644
--- a/core/lib/src/error.rs
+++ b/core/lib/src/error.rs
@@ -7,30 +7,45 @@ use std::sync::Arc;
 use figment::Profile;
 
 use crate::listener::Endpoint;
-use crate::{Ignite, Orbit, Phase, Rocket};
+use crate::{Catcher, Ignite, Orbit, Phase, Rocket, Route};
 use crate::trace::Trace;
 
-/// An error that occurs during launch.
+/// An error that occurred during launch or ignition.
 ///
-/// An `Error` is returned by [`launch()`](Rocket::launch()) when launching an
-/// application fails or, more rarely, when the runtime fails after launching.
+/// An `Error` is returned by [`Rocket::launch()`] or [`Rocket::ignite()`] on
+/// failure to launch or ignite, respectively. An `Error` may occur when the
+/// configuration is invalid, when a route or catcher collision is detected, or
+/// when a fairing fails to launch. An `Error` may also occur when the Rocket
+/// instance fails to liftoff or when the Rocket instance fails to shutdown.
+/// Finally, an `Error` may occur when a sentinel requests an abort.
 ///
-/// # Usage
+/// To determine the kind of error that occurred, use [`Error::kind()`].
 ///
-/// An `Error` value should usually be allowed to `drop` without inspection.
-/// There are at least two exceptions:
+/// # Example
 ///
-///   1. If you are writing a library or high-level application on-top of
-///      Rocket, you likely want to inspect the value before it drops to avoid a
-///      Rocket-specific `panic!`. This typically means simply printing the
-///      value.
+/// ```rust
+/// # use rocket::*;
+/// use rocket::trace::Trace;
+/// use rocket::error::ErrorKind;
 ///
-///   2. You want to display your own error messages.
+/// # async fn run() -> Result<(), rocket::error::Error> {
+/// if let Err(e) = rocket::build().ignite().await {
+///     match e.kind() {
+///         ErrorKind::Bind(_, e) => info!("binding failed: {}", e),
+///         ErrorKind::Io(e) => info!("I/O error: {}", e),
+///         _ => e.trace_error(),
+///     }
+///
+///     return Err(e);
+/// }
+/// # Ok(())
+/// # }
+/// ```
 pub struct Error {
     pub(crate) kind: ErrorKind
 }
 
-/// The kind error that occurred.
+/// The error kind that occurred. Returned by [`Error::kind()`].
 ///
 /// In almost every instance, a launch error occurs because of an I/O error;
 /// this is represented by the `Io` variant. A launch error may also occur
@@ -39,17 +54,22 @@ pub struct Error {
 /// `FailedFairing` variants, respectively.
 #[derive(Debug)]
 #[non_exhaustive]
-// FIXME: Don't expose this. Expose access methods from `Error` instead.
 pub enum ErrorKind {
-    /// Binding to the network interface at `.0` failed with error `.1`.
+    /// Binding to the network interface at `.0` (if known) failed with `.1`.
     Bind(Option<Endpoint>, Box<dyn StdError + Send>),
     /// An I/O error occurred during launch.
     Io(io::Error),
     /// A valid [`Config`](crate::Config) could not be extracted from the
     /// configured figment.
     Config(figment::Error),
-    /// Route collisions were detected.
-    Collisions(crate::router::Collisions),
+    /// Route or catcher collisions were detected. At least one of `routes` or
+    /// `catchers` is guaranteed to be non-empty.
+    Collisions {
+        /// Pairs of colliding routes, if any.
+        routes: Vec<(Route, Route)>,
+        /// Pairs of colliding catchers, if any.
+        catchers: Vec<(Catcher, Catcher)>,
+    },
     /// Launch fairing(s) failed.
     FailedFairings(Vec<crate::fairing::Info>),
     /// Sentinels requested abort.
@@ -75,11 +95,48 @@ impl Error {
         Error { kind }
     }
 
-    // FIXME: Don't expose this. Expose finer access methods instead.
+    /// Returns the kind of error that occurred.
+    ///
+    /// # Example
+    ///
+    /// ```rust
+    /// # use rocket::*;
+    /// use rocket::trace::Trace;
+    /// use rocket::error::ErrorKind;
+    ///
+    /// # async fn run() -> Result<(), rocket::error::Error> {
+    /// if let Err(e) = rocket::build().ignite().await {
+    ///     match e.kind() {
+    ///         ErrorKind::Bind(_, e) => info!("binding failed: {}", e),
+    ///         ErrorKind::Io(e) => info!("I/O error: {}", e),
+    ///         _ => e.trace_error(),
+    ///    }
+    /// }
+    /// # Ok(())
+    /// # }
+    /// ```
     pub fn kind(&self) -> &ErrorKind {
         &self.kind
     }
 
+    /// Given the return value of [`Rocket::launch()`] or [`Rocket::ignite()`],
+    /// which return a `Result<Rocket<P>, Error>`, logs the error, if any, and
+    /// returns the appropriate exit code.
+    ///
+    /// For `Ok(_)`, returns `ExitCode::SUCCESS`. For `Err(e)`, logs the error
+    /// and returns `ExitCode::FAILURE`.
+    ///
+    /// # Example
+    ///
+    /// ```rust
+    /// # use rocket::*;
+    /// use std::process::ExitCode;
+    /// use rocket::error::Error;
+    ///
+    /// async fn run() -> ExitCode {
+    ///     Error::report(rocket::build().launch().await)
+    /// }
+    /// ```
     pub fn report<P: Phase>(result: Result<Rocket<P>, Error>) -> process::ExitCode {
         match result {
             Ok(_) => process::ExitCode::SUCCESS,
@@ -114,7 +171,7 @@ impl StdError for Error {
         match &self.kind {
             ErrorKind::Bind(_, e) => Some(&**e),
             ErrorKind::Io(e) => Some(e),
-            ErrorKind::Collisions(_) => None,
+            ErrorKind::Collisions { .. } => None,
             ErrorKind::FailedFairings(_) => None,
             ErrorKind::InsecureSecretKey(_) => None,
             ErrorKind::Config(e) => Some(e),
@@ -131,7 +188,7 @@ impl fmt::Display for ErrorKind {
         match self {
             ErrorKind::Bind(_, e) => write!(f, "binding failed: {e}"),
             ErrorKind::Io(e) => write!(f, "I/O error: {e}"),
-            ErrorKind::Collisions(_) => "collisions detected".fmt(f),
+            ErrorKind::Collisions { .. } => "collisions detected".fmt(f),
             ErrorKind::FailedFairings(_) => "launch fairing(s) failed".fmt(f),
             ErrorKind::InsecureSecretKey(_) => "insecure secret key config".fmt(f),
             ErrorKind::Config(_) => "failed to extract configuration".fmt(f),
diff --git a/core/lib/src/lib.rs b/core/lib/src/lib.rs
index 097b3b8e44..8e629ed685 100644
--- a/core/lib/src/lib.rs
+++ b/core/lib/src/lib.rs
@@ -174,7 +174,7 @@ mod erased;
 #[doc(inline)] pub use crate::route::Route;
 #[doc(inline)] pub use crate::phase::{Phase, Build, Ignite, Orbit};
 #[doc(inline)] pub use crate::error::Error;
-#[doc(inline)] pub use crate::sentinel::Sentinel;
+#[doc(inline)] pub use crate::sentinel::{Sentinel, Sentry};
 #[doc(inline)] pub use crate::request::Request;
 #[doc(inline)] pub use crate::rkt::Rocket;
 #[doc(inline)] pub use crate::shutdown::Shutdown;
diff --git a/core/lib/src/rocket.rs b/core/lib/src/rocket.rs
index cafd674a10..0a9dd07afc 100644
--- a/core/lib/src/rocket.rs
+++ b/core/lib/src/rocket.rs
@@ -556,7 +556,7 @@ impl Rocket<Build> {
         let mut router = Router::new();
         self.routes.clone().into_iter().for_each(|r| router.add_route(r));
         self.catchers.clone().into_iter().for_each(|c| router.add_catcher(c));
-        router.finalize().map_err(ErrorKind::Collisions)?;
+        router.finalize().map_err(|(r, c)| ErrorKind::Collisions { routes: r, catchers: c, })?;
 
         // Finally, freeze managed state for faster access later.
         self.state.freeze();
diff --git a/core/lib/src/router/router.rs b/core/lib/src/router/router.rs
index a50c24ba9c..84da1b9843 100644
--- a/core/lib/src/router/router.rs
+++ b/core/lib/src/router/router.rs
@@ -12,11 +12,7 @@ pub(crate) struct Router {
     catchers: HashMap<Option<u16>, Vec<Catcher>>,
 }
 
-#[derive(Debug)]
-pub struct Collisions {
-    pub routes: Vec<(Route, Route)>,
-    pub catchers: Vec<(Catcher, Catcher)>,
-}
+pub type Collisions<T> = Vec<(T, T)>;
 
 impl Router {
     pub fn new() -> Self {
@@ -84,12 +80,12 @@ impl Router {
             })
     }
 
-    pub fn finalize(&self) -> Result<(), Collisions> {
+    pub fn finalize(&self) -> Result<(), (Collisions<Route>, Collisions<Catcher>)> {
         let routes: Vec<_> = self.collisions(self.routes()).collect();
         let catchers: Vec<_> = self.collisions(self.catchers()).collect();
 
         if !routes.is_empty() || !catchers.is_empty() {
-            return Err(Collisions { routes, catchers })
+            return Err((routes, catchers))
         }
 
         Ok(())
diff --git a/core/lib/src/sentinel.rs b/core/lib/src/sentinel.rs
index 37a149c48a..c45517b0da 100644
--- a/core/lib/src/sentinel.rs
+++ b/core/lib/src/sentinel.rs
@@ -317,25 +317,73 @@ impl<T> Sentinel for crate::response::Debug<T> {
     }
 }
 
-/// The information resolved from a `T: ?Sentinel` by the `resolve!()` macro.
+/// Information resolved at compile-time from eligible [`Sentinel`] types.
+///
+/// Returned as a result of the [`ignition`](Rocket::ignite()) method, this
+/// struct contains information about a resolved sentinel including the type ID
+/// and type name. It is made available via the [`ErrorKind::SentinelAborts`]
+/// variant of the [`ErrorKind`] enum.
+///
+/// [`ErrorKind`]: crate::error::ErrorKind
+/// [`ErrorKind::SentinelAborts`]: crate::error::ErrorKind::SentinelAborts
+///
+// The information resolved from a `T: ?Sentinel` by the `resolve!()` macro.
 #[derive(Clone, Copy)]
 pub struct Sentry {
     /// The type ID of `T`.
+    #[doc(hidden)]
     pub type_id: TypeId,
     /// The type name `T` as a string.
+    #[doc(hidden)]
     pub type_name: &'static str,
     /// The type ID of type in which `T` is nested if not a top-level type.
+    #[doc(hidden)]
     pub parent: Option<TypeId>,
     /// The source (file, column, line) location of the resolved `T`.
+    #[doc(hidden)]
     pub location: (&'static str, u32, u32),
     /// The value of `<T as Sentinel>::SPECIALIZED` or the fallback.
     ///
     /// This is `true` when `T: Sentinel` and `false` when `T: !Sentinel`.
+    #[doc(hidden)]
     pub specialized: bool,
     /// The value of `<T as Sentinel>::abort` or the fallback.
+    #[doc(hidden)]
     pub abort: fn(&Rocket<Ignite>) -> bool,
 }
 
+impl Sentry {
+    /// Returns the type ID of the resolved sentinal type.
+    ///
+    /// # Example
+    ///
+    /// ```rust
+    /// use rocket::Sentry;
+    ///
+    /// fn handle_error(sentry: &Sentry) {
+    ///     let type_id = sentry.type_id();
+    /// }
+    /// ```
+    pub fn type_id(&self) -> TypeId {
+        self.type_id
+    }
+
+    /// Returns the type name of the resolved sentinal type.
+    ///
+    /// # Example
+    ///
+    /// ```rust
+    /// use rocket::Sentry;
+    ///
+    /// fn handle_error(sentry: &Sentry) {
+    ///     let type_name = sentry.type_name();
+    ///     println!("Type name: {}", type_name);
+    /// }
+    pub fn type_name(&self) -> &'static str {
+        self.type_name
+    }
+}
+
 /// Query `sentinels`, once for each unique `type_id`, returning an `Err` of all
 /// of the sentinels that triggered an abort or `Ok(())` if none did.
 pub(crate) fn query<'s>(
diff --git a/core/lib/src/trace/traceable.rs b/core/lib/src/trace/traceable.rs
index bbf11fa01b..19b8c8b580 100644
--- a/core/lib/src/trace/traceable.rs
+++ b/core/lib/src/trace/traceable.rs
@@ -314,16 +314,12 @@ impl Trace for ErrorKind {
             }
             Io(reason) => event!(level, "error::io", %reason, "i/o error"),
             Config(error) => error.trace(level),
-            Collisions(collisions) => {
-                let routes = collisions.routes.len();
-                let catchers = collisions.catchers.len();
-
+            Collisions { routes, catchers }=> {
                 span!(level, "collision",
-                    route.pairs = routes,
-                    catcher.pairs = catchers,
+                    route.pairs = routes.len(),
+                    catcher.pairs = catchers.len(),
                     "colliding items detected"
                 ).in_scope(|| {
-                    let routes = &collisions.routes;
                     for (a, b) in routes {
                         span!(level, "colliding route pair").in_scope(|| {
                             a.trace(level);
@@ -331,7 +327,6 @@ impl Trace for ErrorKind {
                         })
                     }
 
-                    let catchers = &collisions.catchers;
                     for (a, b) in catchers {
                         span!(level, "colliding catcher pair").in_scope(|| {
                             a.trace(level);