fix(installation)!: Return Result instead of panicking in `Octocrab::…

…installation` (#687) * fix(installation)!: Return Result instead of panicking Refs: #641 BREAKING CHANGE: `Octocrab::installation` now returns a Result, changing the public API. * refactor(error)!: add `installation` variant Adds a new variant `Installation` on the Error enum and also makes the enum non-exhaustive to prevent further breaking changes when adding new variants in the future. * style: remove unused import * style: remove unnecessary error let binding * refactor: remove installation error message
XAMPPRocky · Sep 30, 2024 · 4164ea9 · 4164ea9
1 parent 6ca4140
commit 4164ea9
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 7 deletions.
diff --git a/src/error.rs b/src/error.rs
@@ -22,6 +22,7 @@ impl std::error::Error for UriParseError {}
 /// An error that could have occurred while using [`crate::Octocrab`].
 #[derive(Snafu, Debug)]
 #[snafu(visibility(pub))]
+#[non_exhaustive]
 pub enum Error {
     GitHub {
         source: GitHubError,
@@ -35,7 +36,8 @@ pub enum Error {
         source: InvalidUri,
         backtrace: Backtrace,
     },
-
+    #[snafu(display("Installation Error: Github App authorization is required to target an installation.\n\nFound at {}", backtrace))]
+    Installation { backtrace: Backtrace },
     InvalidHeaderValue {
         source: http::header::InvalidHeaderValue,
         backtrace: Backtrace,

diff --git a/src/lib.rs b/src/lib.rs
@@ -1010,20 +1010,22 @@ impl Octocrab {
     /// then obtain an installation ID, and then pass that here to
     /// obtain a new `Octocrab` with which you can make API calls
     /// with the permissions of that installation.
-    pub fn installation(&self, id: InstallationId) -> Octocrab {
+    pub fn installation(&self, id: InstallationId) -> Result<Octocrab> {
         let app_auth = if let AuthState::App(ref app_auth) = self.auth_state {
             app_auth.clone()
         } else {
-            panic!("Github App authorization is required to target an installation");
+            return Err(Error::Installation {
+                backtrace: Backtrace::generate(),
+            });
         };
-        Octocrab {
+        Ok(Octocrab {
             client: self.client.clone(),
             auth_state: AuthState::Installation {
                 app: app_auth,
                 installation: id,
                 token: CachedToken::default(),
             },
-        }
+        })
     }
 
     /// Similar to `installation`, but also eagerly caches the installation
@@ -1036,7 +1038,7 @@ impl Octocrab {
         &self,
         id: InstallationId,
     ) -> Result<(Octocrab, SecretString)> {
-        let crab = self.installation(id);
+        let crab = self.installation(id)?;
         let token = crab.request_installation_auth_token().await?;
         Ok((crab, token))
     }
@@ -1499,7 +1501,9 @@ impl Octocrab {
         {
             (app, installation, token)
         } else {
-            panic!("Installation not configured");
+            return Err(Error::Installation {
+                backtrace: Backtrace::generate(),
+            });
         };
         let mut request = Builder::new();
         let mut sensitive_value =