Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 101 additions & 0 deletions src/config/comfyApi.test.ts
Original file line number Diff line number Diff line change
@@ -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 })
Comment thread
bigcat88 marked this conversation as resolved.

expect(getComfyApiBaseUrl()).toBe('https://stagingapi.comfy.org')
expect(getComfyPlatformBaseUrl()).toBe('https://stagingplatform.comfy.org')
})
})
9 changes: 0 additions & 9 deletions src/config/comfyApi.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { isCloud } from '@/platform/distribution/types'
import {
configValueOrDefault,
remoteConfig
Expand All @@ -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',
Expand All @@ -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',
Expand Down
45 changes: 45 additions & 0 deletions src/config/firebase.test.ts
Original file line number Diff line number Diff line change
@@ -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')
})
})
14 changes: 5 additions & 9 deletions src/config/firebase.ts
Original file line number Diff line number Diff line change
@@ -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 = {
Expand Down Expand Up @@ -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
}
11 changes: 5 additions & 6 deletions src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 })
Comment thread
bigcat88 marked this conversation as resolved.

if (isCloud) {
const { initTelemetry } = await import('@/platform/telemetry/initTelemetry')
Expand Down
51 changes: 43 additions & 8 deletions src/platform/remoteConfig/refreshRemoteConfig.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`)
}
}))

Expand Down Expand Up @@ -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)
Expand All @@ -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', () => {
Comment thread
bigcat88 marked this conversation as resolved.
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(
Expand Down
24 changes: 20 additions & 4 deletions src/platform/remoteConfig/refreshRemoteConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand All @@ -12,10 +17,14 @@ interface RefreshRemoteConfigOptions {
useAuth?: boolean
}

async function fetchRemoteConfig(useAuth: boolean): Promise<Response> {
if (!useAuth) return fetch('/api/features', { cache: 'no-store' })

async function fetchRemoteConfig(
useAuth: boolean,
signal?: AbortSignal
): Promise<Response> {
const { api } = await import('@/scripts/api')
if (!useAuth) {
return fetch(api.apiURL('/features'), { cache: 'no-store', signal })
}
return api.fetchApi('/features', { cache: 'no-store' })
}

Expand All @@ -33,8 +42,13 @@ export async function refreshRemoteConfig(
): Promise<void> {
const { useAuth = true } = options

const controller = useAuth ? null : new AbortController()
Comment thread
bigcat88 marked this conversation as resolved.
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()
Expand All @@ -59,5 +73,7 @@ export async function refreshRemoteConfig(
window.__CONFIG__ = {}
remoteConfig.value = {}
remoteConfigState.value = 'error'
} finally {
if (timeoutId !== null) clearTimeout(timeoutId)
}
}
1 change: 1 addition & 0 deletions src/platform/remoteConfig/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading