[fix] #4468 gracefully handle Firebase auth failure (#5144)

* [fix] gracefully handle Firebase auth failure

* [test] Add failing tests to reproduce Firebase Auth network issue #4468

Add test cases that demonstrate the current problematic behavior where
Firebase Auth makes network requests when offline without graceful error
handling, causing toast error messages and degraded offline experience.

Tests reproduce:
- getIdToken() throwing auth/network-request-failed instead of returning null
- getAuthHeader() failing to fallback gracefully when Firebase token refresh fails

These tests currently pass by expecting the error to be thrown. After
implementing the fix, the tests should be updated to verify graceful
handling (returning null instead of throwing).

Related to issue #4468: Firebase Auth makes network requests when offline
without evicting token

🤖 Generated with Claude Code

Co-Authored-By: Claude <noreply@anthropic.com>

* [test] update firebaseAuthStore tests

They match the behavior of the implemented solution now

* [test] add firebaseAuthStore.getTokenId test for non-network errors

* [chore] code review feedback

* [test] use FirebaseError

Co-authored-by: Alexander Brown <drjkl@comfy.org>

* [fix] remove indentation and fix test

---------

Co-authored-by: snomiao <snomiao@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Alexander Brown <drjkl@comfy.org>

src/scripts/app.ts

@@ -1303,8 +1303,7 @@ export class ComfyApp {
     const executionStore = useExecutionStore()
     executionStore.lastNodeErrors = null
 
-    let comfyOrgAuthToken =
-      (await useFirebaseAuthStore().getIdToken()) ?? undefined
+    let comfyOrgAuthToken = await useFirebaseAuthStore().getIdToken()
     let comfyOrgApiKey = useApiKeyAuthStore().getApiKey()
 
     try {

src/stores/firebaseAuthStore.ts

@@ -1,5 +1,7 @@
+import { FirebaseError } from 'firebase/app'
 import {
   type Auth,
+  AuthErrorCodes,
   GithubAuthProvider,
   GoogleAuthProvider,
   type User,
@@ -20,6 +22,7 @@ import { useFirebaseAuth } from 'vuefire'
 
 import { COMFY_API_BASE_URL } from '@/config/comfyApi'
 import { t } from '@/i18n'
+import { useDialogService } from '@/services/dialogService'
 import { useApiKeyAuthStore } from '@/stores/apiKeyAuthStore'
 import { type AuthHeader } from '@/types/authTypes'
 import { operations } from '@/types/comfyRegistryTypes'
@@ -88,11 +91,27 @@ export const useFirebaseAuthStore = defineStore('firebaseAuth', () => {
     lastBalanceUpdateTime.value = null
   })
 
-  const getIdToken = async (): Promise<string | null> => {
-    if (currentUser.value) {
-      return currentUser.value.getIdToken()
+  const getIdToken = async (): Promise<string | undefined> => {
+    if (!currentUser.value) return
+    try {
+      return await currentUser.value.getIdToken()
+    } catch (error: unknown) {
+      if (
+        error instanceof FirebaseError &&
+        error.code === AuthErrorCodes.NETWORK_REQUEST_FAILED
+      ) {
+        console.warn(
+          'Could not authenticate with Firebase. Features requiring authentication might not work.'
+        )
+        return
+      }
+
+      useDialogService().showErrorDialog(error, {
+        title: t('errorDialog.defaultTitle'),
+        reportType: 'authenticationError'
+      })
+      console.error(error)
     }
-    return null
   }
 
   /**

tests-ui/tests/store/firebaseAuthStore.test.ts

@@ -1,8 +1,10 @@
+import { FirebaseError } from 'firebase/app'
 import * as firebaseAuth from 'firebase/auth'
 import { createPinia, setActivePinia } from 'pinia'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 import * as vuefire from 'vuefire'
 
+import { useDialogService } from '@/services/dialogService'
 import { useFirebaseAuthStore } from '@/stores/firebaseAuthStore'
 
 // Mock fetch
@@ -46,21 +48,24 @@ vi.mock('vue-i18n', () => ({
   })
 }))
 
-vi.mock('firebase/auth', () => ({
-  signInWithEmailAndPassword: vi.fn(),
-  createUserWithEmailAndPassword: vi.fn(),
-  signOut: vi.fn(),
-  onAuthStateChanged: vi.fn(),
-  signInWithPopup: vi.fn(),
-  GoogleAuthProvider: class {
-    setCustomParameters = vi.fn()
-  },
-  GithubAuthProvider: class {
-    setCustomParameters = vi.fn()
-  },
-  browserLocalPersistence: 'browserLocalPersistence',
-  setPersistence: vi.fn().mockResolvedValue(undefined)
-}))
+vi.mock('firebase/auth', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('firebase/auth')>()
+  return {
+    ...actual,
+    signInWithEmailAndPassword: vi.fn(),
+    createUserWithEmailAndPassword: vi.fn(),
+    signOut: vi.fn(),
+    onAuthStateChanged: vi.fn(),
+    signInWithPopup: vi.fn(),
+    GoogleAuthProvider: class {
+      setCustomParameters = vi.fn()
+    },
+    GithubAuthProvider: class {
+      setCustomParameters = vi.fn()
+    },
+    setPersistence: vi.fn().mockResolvedValue(undefined)
+  }
+})
 
 // Mock useToastStore
 vi.mock('@/stores/toastStore', () => ({
@@ -70,11 +75,7 @@ vi.mock('@/stores/toastStore', () => ({
 }))
 
 // Mock useDialogService
-vi.mock('@/services/dialogService', () => ({
-  useDialogService: () => ({
-    showSettingsDialog: vi.fn()
-  })
-}))
+vi.mock('@/services/dialogService')
 
 describe('useFirebaseAuthStore', () => {
   let store: ReturnType<typeof useFirebaseAuthStore>
@@ -93,6 +94,12 @@ describe('useFirebaseAuthStore', () => {
   beforeEach(() => {
     vi.resetAllMocks()
 
+    // Setup dialog service mock
+    vi.mocked(useDialogService, { partial: true }).mockReturnValue({
+      showSettingsDialog: vi.fn(),
+      showErrorDialog: vi.fn()
+    })
+
     // Mock useFirebaseAuth to return our mock auth object
     vi.mocked(vuefire.useFirebaseAuth).mockReturnValue(mockAuth as any)
 
@@ -297,7 +304,7 @@ describe('useFirebaseAuthStore', () => {
 
       const token = await store.getIdToken()
 
-      expect(token).toBeNull()
+      expect(token).toBeUndefined()
     })
 
     it('should return null for token after login and logout sequence', async () => {
@@ -329,7 +336,75 @@ describe('useFirebaseAuthStore', () => {
 
       // Verify token is null after logout
       const tokenAfterLogout = await store.getIdToken()
-      expect(tokenAfterLogout).toBeNull()
+      expect(tokenAfterLogout).toBeUndefined()
+    })
+
+    it('should handle network errors gracefully when offline (reproduces issue #4468)', async () => {
+      // This test reproduces the issue where Firebase Auth makes network requests when offline
+      // and fails without graceful error handling, causing toast error messages
+
+      // Simulate a user with an expired token that requires network refresh
+      mockUser.getIdToken.mockReset()
+
+      // Mock network failure (auth/network-request-failed error from Firebase)
+      const networkError = new FirebaseError(
+        firebaseAuth.AuthErrorCodes.NETWORK_REQUEST_FAILED,
+        'mock error'
+      )
+
+      mockUser.getIdToken.mockRejectedValue(networkError)
+
+      const token = await store.getIdToken()
+      expect(token).toBeUndefined() // Should return undefined instead of throwing
+    })
+
+    it('should show error dialog when getIdToken fails with non-network error', async () => {
+      // This test verifies that non-network errors trigger the error dialog
+      mockUser.getIdToken.mockReset()
+
+      // Mock a non-network error using actual Firebase Auth error code
+      const authError = new FirebaseError(
+        firebaseAuth.AuthErrorCodes.USER_DISABLED,
+        'User account is disabled.'
+      )
+
+      mockUser.getIdToken.mockRejectedValue(authError)
+
+      // Should call the error dialog instead of throwing
+      const token = await store.getIdToken()
+      const dialogService = useDialogService()
+
+      expect(dialogService.showErrorDialog).toHaveBeenCalledWith(authError, {
+        title: 'errorDialog.defaultTitle',
+        reportType: 'authenticationError'
+      })
+      expect(token).toBeUndefined()
+    })
+  })
+
+  describe('getAuthHeader', () => {
+    it('should handle network errors gracefully when getting Firebase token (reproduces issue #4468)', async () => {
+      // This test reproduces the issue where getAuthHeader fails due to network errors
+      // when Firebase Auth tries to refresh tokens offline
+
+      // Mock useApiKeyAuthStore to return null (no API key fallback)
+      const mockApiKeyStore = {
+        getAuthHeader: vi.fn().mockReturnValue(null)
+      }
+      vi.doMock('@/stores/apiKeyAuthStore', () => ({
+        useApiKeyAuthStore: () => mockApiKeyStore
+      }))
+
+      // Setup user with network error on token refresh
+      mockUser.getIdToken.mockReset()
+      const networkError = new FirebaseError(
+        firebaseAuth.AuthErrorCodes.NETWORK_REQUEST_FAILED,
+        'mock error'
+      )
+      mockUser.getIdToken.mockRejectedValue(networkError)
+
+      const authHeader = await store.getAuthHeader()
+      expect(authHeader).toBeNull() // Should fallback gracefully
     })
   })