diff --git a/src/config/comfyApi.test.ts b/src/config/comfyApi.test.ts new file mode 100644 index 00000000000..681428ca561 --- /dev/null +++ b/src/config/comfyApi.test.ts @@ -0,0 +1,101 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { refreshRemoteConfig } from '@/platform/remoteConfig/refreshRemoteConfig' +import { remoteConfig } from '@/platform/remoteConfig/remoteConfig' + +import { getComfyApiBaseUrl, getComfyPlatformBaseUrl } from './comfyApi' + +vi.mock('@/scripts/api', () => ({ + api: { + apiURL: (route: string) => `/api${route}`, + fetchApi: vi.fn() + } +})) + +vi.stubGlobal('fetch', vi.fn()) + +describe('getComfyApiBaseUrl', () => { + const originalConfig = remoteConfig.value + + beforeEach(() => { + remoteConfig.value = {} + }) + + afterEach(() => { + remoteConfig.value = originalConfig + }) + + it('honors the server-provided override', () => { + remoteConfig.value = { comfy_api_base_url: 'https://my-ephem.example.com' } + expect(getComfyApiBaseUrl()).toBe('https://my-ephem.example.com') + }) + + it('falls back to the build-time default when the key is absent', () => { + expect(getComfyApiBaseUrl()).toBe('https://stagingapi.comfy.org') + }) + + it('falls back to the build-time default when the value is empty', () => { + remoteConfig.value = { comfy_api_base_url: '' } + expect(getComfyApiBaseUrl()).toBe('https://stagingapi.comfy.org') + }) +}) + +describe('getComfyPlatformBaseUrl', () => { + const originalConfig = remoteConfig.value + + beforeEach(() => { + remoteConfig.value = {} + }) + + afterEach(() => { + remoteConfig.value = originalConfig + }) + + it('honors the server-provided override', () => { + remoteConfig.value = { + comfy_platform_base_url: 'https://my-ephem-platform.example.com' + } + expect(getComfyPlatformBaseUrl()).toBe( + 'https://my-ephem-platform.example.com' + ) + }) + + it('falls back to the build-time default when the key is absent', () => { + expect(getComfyPlatformBaseUrl()).toBe('https://stagingplatform.comfy.org') + }) + + it('falls back to the build-time default when the value is empty', () => { + remoteConfig.value = { comfy_platform_base_url: '' } + expect(getComfyPlatformBaseUrl()).toBe('https://stagingplatform.comfy.org') + }) +}) + +describe('compatibility with comfyui servers that predate the override keys', () => { + const originalConfig = remoteConfig.value + + beforeEach(() => { + vi.clearAllMocks() + remoteConfig.value = {} + }) + + afterEach(() => { + remoteConfig.value = originalConfig + }) + + it('falls back to build-time defaults when /features omits the URL keys', async () => { + // An older comfyui server has /features but doesn't know about + // comfy_api_base_url / comfy_platform_base_url yet. + vi.mocked(global.fetch).mockResolvedValue({ + ok: true, + json: async () => ({ + supports_preview_metadata: true, + max_upload_size: 104857600 + }) + } as Response) + + await refreshRemoteConfig({ useAuth: false }) + + expect(getComfyApiBaseUrl()).toBe('https://stagingapi.comfy.org') + expect(getComfyPlatformBaseUrl()).toBe('https://stagingplatform.comfy.org') + }) +}) diff --git a/src/config/comfyApi.ts b/src/config/comfyApi.ts index b6a8e375af4..86e3c5ce8c7 100644 --- a/src/config/comfyApi.ts +++ b/src/config/comfyApi.ts @@ -1,4 +1,3 @@ -import { isCloud } from '@/platform/distribution/types' import { configValueOrDefault, remoteConfig @@ -20,10 +19,6 @@ const BUILD_TIME_PLATFORM_BASE_URL = __USE_PROD_CONFIG__ STAGING_PLATFORM_BASE_URL) export function getComfyApiBaseUrl(): string { - if (!isCloud) { - return BUILD_TIME_API_BASE_URL - } - return configValueOrDefault( remoteConfig.value, 'comfy_api_base_url', @@ -32,10 +27,6 @@ export function getComfyApiBaseUrl(): string { } export function getComfyPlatformBaseUrl(): string { - if (!isCloud) { - return BUILD_TIME_PLATFORM_BASE_URL - } - return configValueOrDefault( remoteConfig.value, 'comfy_platform_base_url', diff --git a/src/config/firebase.test.ts b/src/config/firebase.test.ts new file mode 100644 index 00000000000..b96de30b9b6 --- /dev/null +++ b/src/config/firebase.test.ts @@ -0,0 +1,45 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' + +async function loadFirebase(useProdConfig: boolean) { + vi.resetModules() + vi.stubGlobal('__USE_PROD_CONFIG__', useProdConfig) + const { remoteConfig } = await import('@/platform/remoteConfig/remoteConfig') + const { getFirebaseConfig } = await import('./firebase') + return { getFirebaseConfig, remoteConfig } +} + +describe('getFirebaseConfig', () => { + afterEach(() => { + vi.unstubAllGlobals() + }) + + it('honors a full server-provided firebase_config (cloud builds)', async () => { + const cloud = { + apiKey: 'cloud-key', + authDomain: 'cloud.example.com', + projectId: 'some-cloud-project', + storageBucket: 'cloud.appspot.com', + messagingSenderId: '1', + appId: '1:1:web:abc' + } + const { getFirebaseConfig, remoteConfig } = await loadFirebase(true) + remoteConfig.value = { firebase_config: cloud } + expect(getFirebaseConfig()).toEqual(cloud) + }) + + it('uses the dev project when the server reports firebase_env "dev", even if the build-time fallback is prod', async () => { + const { getFirebaseConfig, remoteConfig } = await loadFirebase(true) + remoteConfig.value = { firebase_env: 'dev' } + expect(getFirebaseConfig().projectId).toBe('dreamboothy-dev') + }) + + it('falls back to the build-time config when the server reports no firebase_env', async () => { + const prod = await loadFirebase(true) + prod.remoteConfig.value = {} + expect(prod.getFirebaseConfig().projectId).toBe('dreamboothy') + + const dev = await loadFirebase(false) + dev.remoteConfig.value = {} + expect(dev.getFirebaseConfig().projectId).toBe('dreamboothy-dev') + }) +}) diff --git a/src/config/firebase.ts b/src/config/firebase.ts index e976a2ef605..90ad7e92e2b 100644 --- a/src/config/firebase.ts +++ b/src/config/firebase.ts @@ -1,6 +1,5 @@ import type { FirebaseOptions } from 'firebase/app' -import { isCloud } from '@/platform/distribution/types' import { remoteConfig } from '@/platform/remoteConfig/remoteConfig' const DEV_CONFIG: FirebaseOptions = { @@ -28,15 +27,12 @@ const PROD_CONFIG: FirebaseOptions = { const BUILD_TIME_CONFIG = __USE_PROD_CONFIG__ ? PROD_CONFIG : DEV_CONFIG /** - * Returns the Firebase configuration for the current environment. - * - Cloud builds use runtime configuration delivered via feature flags - * - OSS / localhost builds fall back to the build-time config determined by __USE_PROD_CONFIG__ + * Firebase config for the current backend: the server's firebase_config (cloud builds), + * else the bundled DEV_CONFIG when the server reports a dev-tier backend, else the build-time default. */ export function getFirebaseConfig(): FirebaseOptions { - if (!isCloud) { - return BUILD_TIME_CONFIG - } - const runtimeConfig = remoteConfig.value.firebase_config - return runtimeConfig ?? BUILD_TIME_CONFIG + if (runtimeConfig) return runtimeConfig + if (remoteConfig.value.firebase_env === 'dev') return DEV_CONFIG + return BUILD_TIME_CONFIG } diff --git a/src/main.ts b/src/main.ts index e120d704309..9e05dfaf57c 100644 --- a/src/main.ts +++ b/src/main.ts @@ -32,13 +32,12 @@ import { i18n } from './i18n' const isCloud = __DISTRIBUTION__ === 'cloud' const hasHostTelemetryBridge = Boolean(window.__comfyDesktop2?.Telemetry) -const requiresRemoteConfigBootstrap = isCloud || hasHostTelemetryBridge -if (requiresRemoteConfigBootstrap) { - const { refreshRemoteConfig } = - await import('@/platform/remoteConfig/refreshRemoteConfig') - await refreshRemoteConfig({ useAuth: false }) -} +// Load remote config before initializeApp() below, so getFirebaseConfig() resolves +// against the server's runtime values instead of the build-time defaults. +const { refreshRemoteConfig } = + await import('@/platform/remoteConfig/refreshRemoteConfig') +await refreshRemoteConfig({ useAuth: false }) if (isCloud) { const { initTelemetry } = await import('@/platform/telemetry/initTelemetry') diff --git a/src/platform/remoteConfig/refreshRemoteConfig.test.ts b/src/platform/remoteConfig/refreshRemoteConfig.test.ts index f947b822cb2..53a5fde8b8f 100644 --- a/src/platform/remoteConfig/refreshRemoteConfig.test.ts +++ b/src/platform/remoteConfig/refreshRemoteConfig.test.ts @@ -7,7 +7,8 @@ import { remoteConfig } from './remoteConfig' vi.mock('@/scripts/api', () => ({ api: { - fetchApi: vi.fn() + fetchApi: vi.fn(), + apiURL: vi.fn((route: string) => `/ComfyUI/api${route}`) } })) @@ -43,9 +44,10 @@ describe('refreshRemoteConfig', () => { await refreshRemoteConfig({ useAuth: true }) - expect(api.fetchApi).toHaveBeenCalledWith('/features', { - cache: 'no-store' - }) + expect(api.fetchApi).toHaveBeenCalledWith( + '/features', + expect.objectContaining({ cache: 'no-store' }) + ) expect(global.fetch).not.toHaveBeenCalled() expect(remoteConfig.value).toEqual(mockConfig) expect(window.__CONFIG__).toEqual(mockConfig) @@ -59,23 +61,56 @@ describe('refreshRemoteConfig', () => { expect(api.fetchApi).toHaveBeenCalled() expect(global.fetch).not.toHaveBeenCalled() }) + + it('does not pass an abort signal on the authed branch (so it is never aborted)', async () => { + vi.mocked(api.fetchApi).mockResolvedValue(mockSuccessResponse()) + + await refreshRemoteConfig({ useAuth: true }) + + const init = vi.mocked(api.fetchApi).mock.calls[0][1] + expect(init?.signal).toBeUndefined() + }) }) describe('without auth', () => { - it('uses raw fetch when useAuth is false', async () => { + it('builds the no-auth url via api.apiURL so a path prefix is respected', async () => { vi.mocked(global.fetch).mockResolvedValue(mockSuccessResponse()) await refreshRemoteConfig({ useAuth: false }) - expect(global.fetch).toHaveBeenCalledWith('/api/features', { - cache: 'no-store' - }) + expect(api.apiURL).toHaveBeenCalledWith('/features') + expect(global.fetch).toHaveBeenCalledWith( + '/ComfyUI/api/features', + expect.objectContaining({ cache: 'no-store' }) + ) expect(api.fetchApi).not.toHaveBeenCalled() expect(remoteConfig.value).toEqual(mockConfig) expect(window.__CONFIG__).toEqual(mockConfig) }) }) + describe('timeout', () => { + it('passes an AbortSignal so a wedged /features cannot hang startup', async () => { + vi.mocked(global.fetch).mockResolvedValue(mockSuccessResponse()) + + await refreshRemoteConfig({ useAuth: false }) + + const init = vi.mocked(global.fetch).mock.calls[0][1] + expect(init?.signal).toBeInstanceOf(AbortSignal) + }) + + it('falls back to empty config when the request aborts', async () => { + vi.mocked(global.fetch).mockRejectedValue( + new DOMException('Aborted', 'AbortError') + ) + + await refreshRemoteConfig({ useAuth: false }) + + expect(remoteConfig.value).toEqual({}) + expect(window.__CONFIG__).toEqual({}) + }) + }) + describe('error handling', () => { it('clears config on 401 response', async () => { vi.mocked(api.fetchApi).mockResolvedValue( diff --git a/src/platform/remoteConfig/refreshRemoteConfig.ts b/src/platform/remoteConfig/refreshRemoteConfig.ts index 449be328fd3..effaf4e60d0 100644 --- a/src/platform/remoteConfig/refreshRemoteConfig.ts +++ b/src/platform/remoteConfig/refreshRemoteConfig.ts @@ -4,6 +4,11 @@ import { remoteConfigState } from './remoteConfig' +// Cap the bootstrap fetch so a wedged /features endpoint can never block app.mount indefinitely. +// A same-origin GET against the local comfyui server should resolve in well under a second; +// on timeout the catch below clears remoteConfig and consumers fall back to build-time defaults. +const FEATURES_FETCH_TIMEOUT_MS = 5_000 + interface RefreshRemoteConfigOptions { /** * Whether to use authenticated API (default: true). @@ -12,10 +17,14 @@ interface RefreshRemoteConfigOptions { useAuth?: boolean } -async function fetchRemoteConfig(useAuth: boolean): Promise { - if (!useAuth) return fetch('/api/features', { cache: 'no-store' }) - +async function fetchRemoteConfig( + useAuth: boolean, + signal?: AbortSignal +): Promise { const { api } = await import('@/scripts/api') + if (!useAuth) { + return fetch(api.apiURL('/features'), { cache: 'no-store', signal }) + } return api.fetchApi('/features', { cache: 'no-store' }) } @@ -33,8 +42,13 @@ export async function refreshRemoteConfig( ): Promise { const { useAuth = true } = options + const controller = useAuth ? null : new AbortController() + const timeoutId = controller + ? setTimeout(() => controller.abort(), FEATURES_FETCH_TIMEOUT_MS) + : null + try { - const response = await fetchRemoteConfig(useAuth) + const response = await fetchRemoteConfig(useAuth, controller?.signal) if (response.ok) { const config = await response.json() @@ -59,5 +73,7 @@ export async function refreshRemoteConfig( window.__CONFIG__ = {} remoteConfig.value = {} remoteConfigState.value = 'error' + } finally { + if (timeoutId !== null) clearTimeout(timeoutId) } } diff --git a/src/platform/remoteConfig/types.ts b/src/platform/remoteConfig/types.ts index a2c65fc0511..c967406d205 100644 --- a/src/platform/remoteConfig/types.ts +++ b/src/platform/remoteConfig/types.ts @@ -93,6 +93,7 @@ export type RemoteConfig = { comfy_api_base_url?: string comfy_platform_base_url?: string firebase_config?: FirebaseRuntimeConfig + firebase_env?: 'dev' telemetry_disabled_events?: TelemetryEventName[] enable_telemetry?: boolean model_upload_button_enabled?: boolean