Distinguish between transient and permanent network errors

Addresses PR review feedback about retrying permanent network errors.

Changes:
- Split isTransientNetworkError (for retry logic) from isNetworkError (for error classification)
- isTransientNetworkError: Only includes retryable errors (timeouts, DNS failures, connection issues)
- isNetworkError: Includes all network errors (transient + permanent SSL/TLS/certificate errors)
- Updated fetchApiVersions to use isNetworkError for error classification
- Eliminated code duplication by having isNetworkError call isTransientNetworkError

Impact:
- Certificate validation failures are still reported as user-facing AbortErrors (not bugs)
- But they are no longer retried unnecessarily
- All existing tests pass

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>

Author: amcaplan

packages/cli-kit/src/private/node/api.ts

@@ -69,14 +69,19 @@ type VerboseResponse<T> =
   | UnauthorizedErrorResponse
 
 /**
- * Checks if an error is a transient network error (connection issues, timeouts, DNS failures, TLS errors, etc.)
- * rather than an application logic error. These errors are typically:
- * - Worth retrying (when retry logic is configured)
- * - Should be reported as user-facing errors (AbortError) rather than bugs (BugError)
+ * Checks if an error is a transient network error that is likely to recover with retries.
+ *
+ * Use this function for retry logic. Use isNetworkError for error classification.
+ *
+ * Examples of transient errors (worth retrying):
+ * - Connection timeouts, resets, and aborts
+ * - DNS failures (enotfound, getaddrinfo, eai_again) - can be temporary
+ * - Socket disconnects and hang ups
+ * - Premature connection closes
  */
 export function isTransientNetworkError(error: unknown): boolean {
   if (error instanceof Error) {
-    const networkErrorMessages = [
+    const transientErrorMessages = [
       'socket hang up',
       'econnreset',
       'econnaborted',
@@ -90,21 +95,43 @@ export function isTransientNetworkError(error: unknown): boolean {
       'the operation was aborted',
       'timeout',
       'premature close',
-      'certificate',
-      'cert',
-      'tls',
-      'ssl',
-      'altnames',
       'getaddrinfo',
     ]
     const errorMessage = error.message.toLowerCase()
-    const anyMatches = networkErrorMessages.some((issueMessage) => errorMessage.includes(issueMessage))
+    const anyMatches = transientErrorMessages.some((issueMessage) => errorMessage.includes(issueMessage))
     const missingReason = /^request to .* failed, reason:\s*$/.test(errorMessage)
     return anyMatches || missingReason
   }
   return false
 }
 
+/**
+ * Checks if an error is any kind of network-related error (connection issues, timeouts, DNS failures,
+ * TLS/certificate errors, etc.) rather than an application logic error.
+ *
+ * These errors should be reported as user-facing errors (AbortError) rather than bugs (BugError),
+ * regardless of whether they are transient or permanent.
+ *
+ * Examples include:
+ * - Transient: connection timeouts, socket hang ups, temporary DNS failures
+ * - Permanent: certificate validation failures, misconfigured SSL
+ */
+export function isNetworkError(error: unknown): boolean {
+  // First check if it's a transient network error
+  if (isTransientNetworkError(error)) {
+    return true
+  }
+
+  // Then check for permanent network errors (SSL/TLS/certificate issues)
+  if (error instanceof Error) {
+    const permanentNetworkErrorMessages = ['certificate', 'cert', 'tls', 'ssl', 'altnames']
+    const errorMessage = error.message.toLowerCase()
+    return permanentNetworkErrorMessages.some((issueMessage) => errorMessage.includes(issueMessage))
+  }
+
+  return false
+}
+
 async function runRequestWithNetworkLevelRetry<T extends {headers: Headers; status: number}>(
   requestOptions: RequestOptions<T>,
 ): Promise<T> {

packages/cli-kit/src/public/node/api/admin.ts

@@ -14,7 +14,7 @@ import {
   restRequestUrl,
   isThemeAccessSession,
 } from '../../../private/node/api/rest.js'
-import {isTransientNetworkError} from '../../../private/node/api.js'
+import {isNetworkError} from '../../../private/node/api.js'
 import {RequestModeInput, shopifyFetch} from '../http.js'
 import {PublicApiVersions} from '../../../cli/api/graphql/admin/generated/public_api_versions.js'
 
@@ -185,9 +185,10 @@ async function fetchApiVersions(session: AdminSession, preferredBehaviour?: Requ
       )
     }
 
-    // Check for network-level errors (connection issues, timeouts, DNS failures, TLS errors, etc.)
-    // These are transient errors that may have been retried already by lower-level retry logic
-    if (isTransientNetworkError(error)) {
+    // Check for network-level errors (connection issues, timeouts, DNS failures, TLS/certificate errors, etc.)
+    // All network errors should be treated as user-facing errors, not CLI bugs
+    // Note: Some of these may have been retried already by lower-level retry logic
+    if (isNetworkError(error)) {
       throw new AbortError(
         `Network error connecting to your store ${session.storeFqdn}: ${
           error instanceof Error ? error.message : String(error)