diff --git a/src/scripts/app.ts b/src/scripts/app.ts index ca784b66fc..debcc26278 100644 --- a/src/scripts/app.ts +++ b/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 { diff --git a/src/stores/firebaseAuthStore.ts b/src/stores/firebaseAuthStore.ts index b8befb0932..65b468001a 100644 --- a/src/stores/firebaseAuthStore.ts +++ b/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 => { - if (currentUser.value) { - return currentUser.value.getIdToken() + const getIdToken = async (): Promise => { + 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 } /** diff --git a/tests-ui/tests/store/firebaseAuthStore.test.ts b/tests-ui/tests/store/firebaseAuthStore.test.ts index 7d51685b4c..ffe7a8d995 100644 --- a/tests-ui/tests/store/firebaseAuthStore.test.ts +++ b/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() + 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 @@ -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 }) })