Handle network errors when fetching GitHub user

Author: mtsgrd

Cargo.lock

@@ -23,6 +23,7 @@ export type ForgeConfig = {
 	baseBranch?: string;
 	githubAuthenticated?: boolean;
 	githubIsLoading?: boolean;
+	githubError?: { code: string; message: string };
 	gitlabAuthenticated?: boolean;
 	detectedForgeProvider: ForgeProvider | undefined;
 	forgeOverride?: ForgeName;
@@ -35,7 +36,12 @@ export class DefaultForgeFactory implements Reactive<Forge> {
 	private _forge = $state<Forge>(this.default);
 	private _config: any = undefined;
 	private _determinedForgeType = $state<ForgeName>('default');
+	private _githubError = $state<{ code: string; message: string } | undefined>(undefined);
 	private _canSetupIntegration = $derived.by(() => {
+		// Don't show the setup prompt if there's a network error
+		if (this._githubError?.code === 'errors.network') {
+			return undefined;
+		}
 		return isAvalilableForge(this._determinedForgeType) &&
 			!this._forge.authenticated &&
 			!this._forge.isLoading
@@ -78,10 +84,12 @@ export class DefaultForgeFactory implements Reactive<Forge> {
 			baseBranch,
 			githubAuthenticated,
 			githubIsLoading,
+			githubError,
 			gitlabAuthenticated,
 			detectedForgeProvider,
 			forgeOverride
 		} = config;
+		this._githubError = githubError;
 		if (repo && baseBranch) {
 			this._determinedForgeType = this.determineForgeType(repo, detectedForgeProvider);
 			this._forge = this.build({

apps/desktop/src/lib/forge/forgeFactory.svelte.ts

@@ -57,6 +57,8 @@ type GitHubAccess = {
 	host: Reactive<string | undefined>;
 	accessToken: Reactive<string | undefined>;
 	isLoading: Reactive<boolean>;
+	error: Reactive<{ code: string; message: string } | undefined>;
+	isError: Reactive<boolean>;
 };
 
 /**
@@ -79,6 +81,10 @@ export function useGitHubAccessToken(projectId: Reactive<string>): GitHubAccess
 	return {
 		host: reactive(() => host),
 		accessToken: reactive(() => aceessToken),
-		isLoading: reactive(() => ghUserResponse?.result.isLoading ?? false)
+		isLoading: reactive(() => ghUserResponse?.result.isLoading ?? false),
+		error: reactive(
+			() => ghUserResponse?.result.error as { code: string; message: string } | undefined
+		),
+		isError: reactive(() => ghUserResponse?.result.isError ?? false)
 	};
 }

apps/desktop/src/lib/forge/github/hooks.svelte.ts

@@ -124,6 +124,7 @@
 			baseBranch: baseBranchName,
 			githubAuthenticated: !!githubAccessToken.accessToken.current,
 			githubIsLoading: githubAccessToken.isLoading.current,
+			githubError: githubAccessToken.error.current,
 			gitlabAuthenticated: !!$gitlabConfigured,
 			detectedForgeProvider: baseBranch?.forgeRepoInfo?.forge ?? undefined,
 			forgeOverride: projects?.find((project) => project.id === projectId)?.forge_override

apps/desktop/src/routes/[projectId]/+layout.svelte

@@ -139,6 +139,7 @@ pub enum Code {
     SecretKeychainNotFound,
     MissingLoginKeychain,
     GitForcePushProtection,
+    NetworkError,
 }
 
 impl std::fmt::Display for Code {
@@ -157,6 +158,7 @@ impl std::fmt::Display for Code {
             Code::SecretKeychainNotFound => "errors.secret.keychain_notfound",
             Code::MissingLoginKeychain => "errors.secret.missing_login_keychain",
             Code::GitForcePushProtection => "errors.git.force_push_protection",
+            Code::NetworkError => "errors.network",
         };
         f.write_str(code)
     }

crates/but-error/src/lib.rs

@@ -18,8 +18,13 @@ serde.workspace = true
 but-settings.workspace = true
 but-secret.workspace = true
 but-forge-storage.workspace = true
+but-error.workspace = true
 anyhow.workspace = true
 gitbutler-user.workspace = true
 serde_json.workspace = true
 reqwest = { workspace = true, features = ["json"] }
 octorust = { version = "0.10.0", default-features = false }
+
+[dev-dependencies]
+reqwest = { workspace = true, features = ["blocking"] }
+http = "1"

crates/but-github/Cargo.toml

@@ -28,7 +28,7 @@ impl GitHubClient {
         Ok(Self { github })
     }
 
-    pub async fn get_authenticated(&self) -> Result<AuthenticatedUser> {
+    pub async fn get_authenticated(&self) -> Result<AuthenticatedUser, octorust::ClientError> {
         self.github
             .users()
             .get_authenticated()
@@ -57,7 +57,6 @@ impl GitHubClient {
                     }
                 }
             })
-            .map_err(Into::into)
     }
 
     pub async fn list_open_pulls(&self, owner: &str, repo: &str) -> Result<Vec<PullRequest>> {

crates/but-github/src/client.rs

@@ -226,10 +226,23 @@ pub async fn get_gh_user(
         let gh = account
             .client(&access_token)
             .context("Failed to create GitHub client")?;
-        let user = gh
-            .get_authenticated()
-            .await
-            .context("Failed to get authenticated user")?;
+        let user = match gh.get_authenticated().await {
+            Ok(user) => user,
+            Err(client_err) => {
+                // Check if this is a network error before converting to anyhow
+                if is_network_error(&client_err) {
+                    return Err(anyhow::Error::from(client_err).context(
+                        but_error::Context::new_static(
+                            but_error::Code::NetworkError,
+                            "Unable to connect to GitHub.",
+                        ),
+                    ));
+                }
+                return Err(
+                    anyhow::Error::from(client_err).context("Failed to get authenticated user")
+                );
+            }
+        };
         Ok(Some(AuthenticatedUser {
             access_token,
             login: user.login,
@@ -242,6 +255,14 @@ pub async fn get_gh_user(
     }
 }
 
+/// Check if an error is a network connectivity error.
+///
+/// This includes DNS resolution failures, connection timeouts, connection refused, etc.
+fn is_network_error(err: &octorust::ClientError) -> bool {
+    matches!(err, octorust::ClientError::ReqwestError(reqwest_err)
+        if reqwest_err.is_timeout() || reqwest_err.is_connect() || reqwest_err.is_request())
+}
+
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum CredentialCheckResult {
     Valid,
@@ -290,3 +311,75 @@ pub async fn list_known_github_accounts(
 pub fn clear_all_github_tokens(storage: &but_forge_storage::controller::Controller) -> Result<()> {
     token::clear_all_github_accounts(storage).context("Failed to clear all GitHub tokens")
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_is_network_error_with_reqwest_timeout() {
+        // Create a reqwest error by making an actual HTTP request that will timeout
+        let client = reqwest::blocking::Client::builder()
+            .timeout(std::time::Duration::from_millis(1))
+            .build()
+            .unwrap();
+
+        // Try to connect to a non-routable IP address (should timeout)
+        let result = client.get("http://192.0.2.1:80").send();
+
+        if let Err(reqwest_err) = result {
+            let client_err = octorust::ClientError::ReqwestError(reqwest_err);
+            assert!(
+                is_network_error(&client_err),
+                "Should detect timeout/connection errors"
+            );
+        } else {
+            panic!("Expected a network error but request succeeded");
+        }
+    }
+
+    #[test]
+    fn test_is_network_error_with_connection_error() {
+        // Create a reqwest error and wrap it in ClientError
+        let client = reqwest::blocking::Client::builder()
+            .timeout(std::time::Duration::from_millis(1))
+            .build()
+            .unwrap();
+
+        let result = client.get("http://192.0.2.1:80").send();
+
+        if let Err(reqwest_err) = result {
+            let client_err = octorust::ClientError::ReqwestError(reqwest_err);
+            assert!(
+                is_network_error(&client_err),
+                "Should detect ClientError wrapping reqwest network errors"
+            );
+        } else {
+            panic!("Expected a network error but request succeeded");
+        }
+    }
+
+    #[test]
+    fn test_is_not_network_error_http_error() {
+        // HTTP errors (like 401) are not network errors
+        let client_err = octorust::ClientError::HttpError {
+            status: http::StatusCode::UNAUTHORIZED,
+            headers: reqwest::header::HeaderMap::new(),
+            error: "Unauthorized".to_string(),
+        };
+        assert!(
+            !is_network_error(&client_err),
+            "Should not detect HTTP status errors as network errors"
+        );
+    }
+
+    #[test]
+    fn test_is_not_network_error_rate_limit() {
+        // Rate limit errors are not network errors
+        let client_err = octorust::ClientError::RateLimited { duration: 60 };
+        assert!(
+            !is_network_error(&client_err),
+            "Should not detect rate limit errors as network errors"
+        );
+    }
+}