Skip to content

Commit 450cc7f

Browse files
christian-byrneclaudeactions-user
authored
fix: bound auth wait in startStoreBootstrap + catch-all for unknown routes (IR-105) (#15063)
## Summary Two durable fixes for the IR-105 splash-screen hang incident. ### Fix 1 — Bootstrap auth wait timeout (`bootstrapStore.ts`) **Root cause:** `until(isInitialized).toBe(true)` and `until(isAuthenticated).toBe(true)` in `startStoreBootstrap` have no timeout. A stale Firebase session or broken auth response hangs bootstrap silently and indefinitely — no error, no log, just a forever splash screen. **Fix:** Extract `waitForCloudAuth()` with: - 16 s timeout (matching `router.ts:176` and `WorkspaceAuthGate.vue:78` — existing convention) - One retry after 3 s backoff - On second timeout: `captureException` to Sentry + **continue bootstrap** rather than blocking This means the worst case is now 35 s to a usable (logged-out) state instead of infinity. ### Fix 2 — Catch-all route for unknown paths (`router.ts`) **Root cause:** A request to e.g. `cloud.comfy.org/woiadawd` gets a 200 + app shell from the static host, Vue boots, finds **no matching route**, and the global auth guard's `until(isInitialized)` then hangs (see Fix 1). Even with Fix 1, a no-match route leaves the app on the splash screen. **Fix:** Add `{ path: '/:pathMatch(.*)*', redirect: '/' }` at the end of the route list. Unknown paths redirect to root; the global auth guard then sends unauthenticated users to `/cloud/login` as normal. ## Red-Green Verification | Commit | Purpose | |--------|---------| | `test: add failing tests for unbounded auth wait in startStoreBootstrap` | 🔴 Proves test catches the bug — the "gives up" test times out in 5 s against the unpatched code | | `fix: bound auth wait in startStoreBootstrap and add catch-all route` | 🟢 Both new tests pass, all 18 tests in the two affected files pass | ## Relation to open PRs - Supersedes / closes the scope of #15032 (`fix/bootstrap-auth-wait-timeout`) — same fix, same tests. If reviewers prefer to land #15032 first this PR can drop Fix 1. - Companion to #15022 (marketing-site redirect) and #15026 (`/login` → `cloud-login` route, already on main). ## Test Plan - [x] `pnpm vitest run src/stores/bootstrapStore.test.ts` — 7/7 pass (including 2 new timeout tests) - [x] `pnpm vitest run src/platform/cloud/onboarding/onboardingCloudRoutes.test.ts` — 11/11 pass - [x] `pnpm typecheck` — clean - [x] `pnpm exec eslint src/stores/bootstrapStore.ts src/router.ts` — clean - [ ] CI green (Playwright, Storybook, Codecov) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: GitHub Action <action@github.com>
1 parent b72bdea commit 450cc7f

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
@@ -1,7 +1,7 @@
11
import type { AxiosResponse } from 'axios'
22
import { AxiosError } from 'axios'
33
import { beforeEach, describe, expect, it, vi } from 'vitest'
4-
import { nextTick, ref } from 'vue'
4+
import { ref } from 'vue'
55

66
import { mergeCustomNodesI18n } from '@/i18n'
77
import { useSettingStore } from '@/platform/settings/settingStore'
@@ -66,6 +66,16 @@ const mockDistributionTypes = vi.hoisted(() => ({
6666
}))
6767
vi.mock('@/platform/distribution/types', () => mockDistributionTypes)
6868

69+
const mockCaptureException = vi.hoisted(() => vi.fn())
70+
vi.mock('@sentry/vue', () => ({
71+
captureException: mockCaptureException
72+
}))
73+
74+
const mockAddError = vi.hoisted(() => vi.fn())
75+
vi.mock('@datadog/browser-rum', () => ({
76+
datadogRum: { addError: mockAddError }
77+
}))
78+
6979
function requestFailure(status: number) {
7080
const error = new AxiosError(`Request failed with status code ${status}`)
7181
error.response = { status } as AxiosResponse
@@ -131,31 +141,77 @@ describe('bootstrapStore', () => {
131141
describe('cloud mode', () => {
132142
beforeEach(() => {
133143
mockDistributionTypes.isCloud = true
144+
mockCaptureException.mockReset()
145+
mockAddError.mockReset()
134146
})
135147

136-
it('waits for Firebase auth before loading stores', async () => {
148+
it('waits for Firebase init before loading stores, then proceeds regardless of auth state', async () => {
137149
const store = useBootstrapStore()
138150
const settingStore = useSettingStore()
139151
const bootstrapPromise = store.startStoreBootstrap()
140152

141153
expect(store.isI18nReady).toBe(false)
142154
expect(settingStore.isReady).toBe(false)
143155

144-
// Firebase initialized but user not yet authenticated
156+
// Firebase resolves with no user (signed-out) — bootstrap must unblock.
157+
// Previously it also waited for isAuthenticated, which made every
158+
// signed-out load wait 35s and fire a false Sentry timeout.
145159
mockIsAuthInitialized.value = true
146-
await nextTick()
147-
148-
expect(store.isI18nReady).toBe(false)
149-
expect(settingStore.isReady).toBe(false)
150-
151-
// User authenticates (e.g. signs in on login page)
152-
mockIsAuthAuthenticated.value = true
153160
await bootstrapPromise
154161

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

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)