Skip to content

Commit 4657873

Browse files
comfy-pr-botchristian-byrneclaudeactions-user
authored
[backport cloud/1.50] fix: bound auth wait in startStoreBootstrap + catch-all for unknown routes (IR-105) (#15180)
Backport of #15063 to `cloud/1.50` Automatically created by backport workflow. Co-authored-by: Christian Byrne <cbyrne@comfy.org> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: GitHub Action <action@github.com>
1 parent 2655a5a commit 4657873

4 files changed

Lines changed: 151 additions & 14 deletions

File tree

browser_tests/tests/cloud.spec.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,4 +46,25 @@ test.describe('Cloud distribution UI', { tag: '@cloud' }, () => {
4646
// Verify cloud-specific login UI is rendered
4747
await expect(page.getByRole('button', { name: /google/i })).toBeVisible()
4848
})
49+
50+
test('unknown paths redirect to cloud login instead of hanging on splash screen', async ({
51+
page
52+
}) => {
53+
// Load the SPA at root (the backend serves index.html for /).
54+
// Then push an unknown path client-side so Vue Router handles it.
55+
// The backend serves ComfyUI file responses for arbitrary paths, so a
56+
// direct page.goto('/woiadawd') would trigger a download rather than the SPA.
57+
await page.goto(APP_URL)
58+
await expect(page).toHaveURL(/\/cloud\/login/, { timeout: 10_000 })
59+
60+
// Navigate client-side to an unknown path. The catch-all route redirects to /,
61+
// then the auth guard sends unauthenticated users back to /cloud/login.
62+
// Regression: before the catch-all was added, the SPA found no route and
63+
// startStoreBootstrap hung indefinitely on the splash screen.
64+
await page.evaluate(() => {
65+
window.history.pushState(null, '', '/woiadawd')
66+
window.dispatchEvent(new PopStateEvent('popstate'))
67+
})
68+
await expect(page).toHaveURL(/\/cloud\/login/, { timeout: 10_000 })
69+
})
4970
})

src/router.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,11 @@ const router = createRouter({
8383
component: () => import('@/views/UserSelectView.vue')
8484
}
8585
]
86-
}
86+
},
87+
// Catch-all: unknown paths redirect to root rather than hanging on the
88+
// splash screen with no route match. The global auth guard then routes
89+
// unauthenticated users to /cloud/login as normal.
90+
{ path: '/:pathMatch(.*)*', redirect: '/' }
8791
],
8892

8993
scrollBehavior(_to, _from, savedPosition) {

src/stores/bootstrapStore.test.ts

Lines changed: 66 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import type { AxiosResponse } from 'axios'
33
import { AxiosError } from 'axios'
44
import { setActivePinia } from 'pinia'
55
import { beforeEach, describe, expect, it, vi } from 'vitest'
6-
import { nextTick, ref } from 'vue'
6+
import { ref } from 'vue'
77

88
import { mergeCustomNodesI18n } from '@/i18n'
99
import { useSettingStore } from '@/platform/settings/settingStore'
@@ -68,6 +68,16 @@ const mockDistributionTypes = vi.hoisted(() => ({
6868
}))
6969
vi.mock('@/platform/distribution/types', () => mockDistributionTypes)
7070

71+
const mockCaptureException = vi.hoisted(() => vi.fn())
72+
vi.mock('@sentry/vue', () => ({
73+
captureException: mockCaptureException
74+
}))
75+
76+
const mockAddError = vi.hoisted(() => vi.fn())
77+
vi.mock('@datadog/browser-rum', () => ({
78+
datadogRum: { addError: mockAddError }
79+
}))
80+
7181
function requestFailure(status: number) {
7282
const error = new AxiosError(`Request failed with status code ${status}`)
7383
error.response = { status } as AxiosResponse
@@ -135,31 +145,77 @@ describe('bootstrapStore', () => {
135145
describe('cloud mode', () => {
136146
beforeEach(() => {
137147
mockDistributionTypes.isCloud = true
148+
mockCaptureException.mockReset()
149+
mockAddError.mockReset()
138150
})
139151

140-
it('waits for Firebase auth before loading stores', async () => {
152+
it('waits for Firebase init before loading stores, then proceeds regardless of auth state', async () => {
141153
const store = useBootstrapStore()
142154
const settingStore = useSettingStore()
143155
const bootstrapPromise = store.startStoreBootstrap()
144156

145157
expect(store.isI18nReady).toBe(false)
146158
expect(settingStore.isReady).toBe(false)
147159

148-
// Firebase initialized but user not yet authenticated
160+
// Firebase resolves with no user (signed-out) — bootstrap must unblock.
161+
// Previously it also waited for isAuthenticated, which made every
162+
// signed-out load wait 35s and fire a false Sentry timeout.
149163
mockIsAuthInitialized.value = true
150-
await nextTick()
151-
152-
expect(store.isI18nReady).toBe(false)
153-
expect(settingStore.isReady).toBe(false)
154-
155-
// User authenticates (e.g. signs in on login page)
156-
mockIsAuthAuthenticated.value = true
157164
await bootstrapPromise
158165

159166
await vi.waitFor(() => {
160167
expect(store.isI18nReady).toBe(true)
161168
expect(settingStore.isReady).toBe(true)
162169
})
163170
})
171+
172+
it('retries once and proceeds if Firebase init resolves during the backoff', async () => {
173+
vi.useFakeTimers()
174+
try {
175+
const store = useBootstrapStore()
176+
const settingStore = useSettingStore()
177+
const bootstrapPromise = store.startStoreBootstrap()
178+
179+
// First wait times out with Firebase still not initialized.
180+
await vi.advanceTimersByTimeAsync(16_001)
181+
expect(settingStore.isReady).toBe(false)
182+
183+
// Firebase resolves during the retry backoff.
184+
mockIsAuthInitialized.value = true
185+
await vi.advanceTimersByTimeAsync(3_001)
186+
await bootstrapPromise
187+
188+
expect(settingStore.isReady).toBe(true)
189+
expect(mockCaptureException).not.toHaveBeenCalled()
190+
} finally {
191+
vi.useRealTimers()
192+
}
193+
})
194+
195+
it('gives up after a second timeout, reports it, and continues bootstrap unauthenticated', async () => {
196+
vi.useFakeTimers()
197+
try {
198+
const store = useBootstrapStore()
199+
const settingStore = useSettingStore()
200+
const bootstrapPromise = store.startStoreBootstrap()
201+
202+
// Firebase never resolves through the initial wait, the backoff, or the retry.
203+
await vi.advanceTimersByTimeAsync(16_000 + 3_000 + 16_001)
204+
await bootstrapPromise
205+
206+
expect(mockCaptureException).toHaveBeenCalledOnce()
207+
expect(mockCaptureException).toHaveBeenCalledWith(expect.any(Error), {
208+
tags: { error_type: 'bootstrap_auth_wait_timeout' }
209+
})
210+
expect(mockAddError).toHaveBeenCalledOnce()
211+
expect(mockAddError).toHaveBeenCalledWith(expect.any(Error), {
212+
error_type: 'bootstrap_auth_wait_timeout'
213+
})
214+
// Bootstrap must not stay stuck: stores load even when Firebase never fires.
215+
expect(settingStore.isReady).toBe(true)
216+
} finally {
217+
vi.useRealTimers()
218+
}
219+
})
164220
})
165221
})

src/stores/bootstrapStore.ts

Lines changed: 59 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { datadogRum } from '@datadog/browser-rum'
2+
import { captureException } from '@sentry/vue'
13
import { until, useAsyncState } from '@vueuse/core'
24
import axios from 'axios'
35
import { defineStore, storeToRefs } from 'pinia'
@@ -23,6 +25,62 @@ async function fetchCustomNodesI18n(): Promise<CustomNodesI18n | undefined> {
2325
}
2426
}
2527

28+
// Matches the Firebase-auth-wait timeout used elsewhere (router.ts,
29+
// WorkspaceAuthGate.vue) so a broken/stale session fails this bounded wait
30+
// on the same schedule those already fail theirs.
31+
const AUTH_WAIT_TIMEOUT_MS = 16_000
32+
const AUTH_WAIT_RETRY_DELAY_MS = 3_000
33+
34+
/**
35+
* Waits for Firebase auth initialization to complete, bounded so a stale
36+
* token or a broken auth response can never hang bootstrap forever.
37+
*
38+
* Only isInitialized is awaited — onAuthStateChanged fires with null for
39+
* signed-out users, which sets isInitialized but not isAuthenticated.
40+
* Awaiting isAuthenticated here would make every signed-out page load wait
41+
* 35s and fire a false Sentry timeout. The router guard handles the
42+
* login redirect for unauthenticated users separately.
43+
*
44+
* Retries once after a short delay; if auth is still unresolved, reports it
45+
* to Sentry and lets bootstrap continue rather than leaving the caller stuck.
46+
*/
47+
async function waitForCloudAuth(): Promise<void> {
48+
const { isInitialized } = storeToRefs(useAuthStore())
49+
const waitForResolution = () =>
50+
until(isInitialized).toBe(true, {
51+
timeout: AUTH_WAIT_TIMEOUT_MS,
52+
throwOnTimeout: true
53+
})
54+
55+
try {
56+
await waitForResolution()
57+
} catch (error) {
58+
console.warn(
59+
'[bootstrapStore] Auth did not resolve in time, retrying once',
60+
error
61+
)
62+
await new Promise((resolve) =>
63+
setTimeout(resolve, AUTH_WAIT_RETRY_DELAY_MS)
64+
)
65+
try {
66+
await waitForResolution()
67+
} catch (retryError) {
68+
console.error(
69+
'[bootstrapStore] Auth still unresolved after retry; continuing bootstrap without confirmed auth',
70+
retryError
71+
)
72+
const err =
73+
retryError instanceof Error ? retryError : new Error(String(retryError))
74+
// Report to both Datadog RUM and Sentry so the error surfaces in
75+
// whichever observability platform is being monitored.
76+
datadogRum.addError(err, { error_type: 'bootstrap_auth_wait_timeout' })
77+
captureException(err, {
78+
tags: { error_type: 'bootstrap_auth_wait_timeout' }
79+
})
80+
}
81+
}
82+
}
83+
2684
export const useBootstrapStore = defineStore('bootstrap', () => {
2785
const settingStore = useSettingStore()
2886
const workflowStore = useWorkflowStore()
@@ -52,9 +110,7 @@ export const useBootstrapStore = defineStore('bootstrap', () => {
52110

53111
async function startStoreBootstrap() {
54112
if (isCloud) {
55-
const { isInitialized, isAuthenticated } = storeToRefs(useAuthStore())
56-
await until(isInitialized).toBe(true)
57-
await until(isAuthenticated).toBe(true)
113+
await waitForCloudAuth()
58114
}
59115

60116
const userStore = useUserStore()

0 commit comments

Comments
 (0)