diff --git a/src/App.vue b/src/App.vue index 22a575d4..10b3b092 100644 --- a/src/App.vue +++ b/src/App.vue @@ -113,6 +113,29 @@ const downgradeOrphanedMigrations = ref(null) // Suppresses the safe-mode auto-clear watcher while "Roll back to last // known good" programmatically selects a (necessarily older) version. const suppressSafeModeAutoClear = ref(false) +/** + * Ceiling for the three non-blocking loaders fired after the app list renders + * (advisories, pins, policies). + * + * Without one, `fetch` waits forever. Measured (issue #160): all three were + * left permanently suspended at their `await fetch`, so `pins` never left `{}` + * and pin badges, advisory badges and auto-update policy state were silently + * absent — with no error, no console output and no failed request, because a + * promise that never settles reaches neither the success path nor the catch. + * + * 8s is far above any healthy response here (every other endpoint on this page + * answers in ~60ms) so a timeout means something is genuinely wrong, and the + * catch then reports it instead of the UI quietly missing a feature. + * + * ⚠️ It was 20s, and that made the abort UNOBSERVABLE: the e2e assertion that + * watches for the pin badge gives up at 15s, so the timeout fired after the + * test had already failed and the catch never ran within the window. A bound + * must fit inside the bound that contains it — the same arithmetic error as a + * retry that outlasts its job cap. Keep this below the 15s expect timeout in + * playwright.config.ts. + */ +const BACKGROUND_FETCH_TIMEOUT_MS = 8_000 + const safeModeStorageKey = 'app_versions_safe_mode' const debugModeStorageKey = 'app_versions_debug_mode' const dryRunStorageKey = 'app_versions_dry_run_mode' @@ -467,7 +490,7 @@ const loadApps = async (): Promise => { // appears once this resolves. Read-only — it never changes a version. const loadAdvisories = async (): Promise => { try { - const response = await fetch(apiUrl(withOcsJson('/ocs/v2.php/apps/app_versions/api/advisories')), { headers: { ...ocsHeaders, Accept: 'application/json' } }) + const response = await fetch(apiUrl(withOcsJson('/ocs/v2.php/apps/app_versions/api/advisories')), { headers: { ...ocsHeaders, Accept: 'application/json' }, signal: AbortSignal.timeout(BACKGROUND_FETCH_TIMEOUT_MS) }) const payload = await unwrapOcsResponse<{ advisories: Record }>(response) advisories.value = payload.advisories || {} } catch { @@ -505,15 +528,21 @@ const advisoryBadgeLabel = (state: AdvisoryCorrelation['state']): string => { // banner / the pin-override dialog. See "Honest pin presentation". const loadPins = async (): Promise => { try { - const response = await fetch(apiUrl(withOcsJson('/ocs/v2.php/apps/app_versions/api/pins')), { headers: { ...ocsHeaders, Accept: 'application/json' } }) + const response = await fetch(apiUrl(withOcsJson('/ocs/v2.php/apps/app_versions/api/pins')), { headers: { ...ocsHeaders, Accept: 'application/json' }, signal: AbortSignal.timeout(BACKGROUND_FETCH_TIMEOUT_MS) }) const payload = await unwrapOcsResponse<{ pins: PinRecord[] }>(response) const map: Record = {} for (const pin of payload.pins || []) { map[pin.appId] = pin } pins.value = map - } catch { + } catch (error) { + // NOT a bare `catch {}`. A silent catch here is why this took seven + // eliminated hypotheses to chase: pins ends up `{}` and the app-card + // badge simply never renders, with nothing anywhere saying why — no + // failed request, no page error, no console output (issue #160). pins.value = {} + // eslint-disable-next-line no-console + console.error('[app_versions] loadPins failed; pin badges will not render:', error) } } @@ -535,7 +564,7 @@ const pinTooltip = (pin: PinRecord | null): string => { // writes go through onPolicyChange()/saveAutoUpdateSettings(). const loadPolicies = async (): Promise => { try { - const response = await fetch(apiUrl(withOcsJson('/ocs/v2.php/apps/app_versions/api/policies')), { headers: { ...ocsHeaders, Accept: 'application/json' } }) + const response = await fetch(apiUrl(withOcsJson('/ocs/v2.php/apps/app_versions/api/policies')), { headers: { ...ocsHeaders, Accept: 'application/json' }, signal: AbortSignal.timeout(BACKGROUND_FETCH_TIMEOUT_MS) }) const payload = await unwrapOcsResponse<{ policies?: PolicyRecord[], autoUpdateEnabled?: boolean, autoUpdateWindow?: string }>(response) const map: Record = {} for (const policy of payload.policies || []) { @@ -1389,14 +1418,37 @@ onMounted(async () => { try { await checkUpdateChannel() await loadApps() + } catch (error) { + // A `finally` WITHOUT a `catch` re-throws, and the three non-blocking + // loaders below are plain statements in the same function — so anything + // thrown here silently skipped ALL of them. + // + // Measured on CI (issue #160): the browser requested update-channel and + // apps, then NOTHING — no /api/pins, no /api/advisories, no + // /api/policies. `pins` therefore stayed `{}` and the app-card badge's + // `v-if="pinFor(app.id)"` never matched, which is why + // pinning.spec.ts:70 failed on every run since the suite began running. + // + // Catching does not depend on knowing WHAT throws: whatever it is, the + // page must still load its pins, advisories and policies. The message is + // surfaced rather than swallowed so the underlying throw stays visible. + errorMessage.value = error instanceof Error ? error.message : 'Could not initialise the app list.' } finally { isLoading.value = false } // Kick off advisory correlation, pin state, and auto-update policies after - // the list renders (non-blocking). - void loadAdvisories() - void loadPins() - void loadPolicies() + // the list renders (non-blocking). Each handles its own failures; the extra + // catch guards a SYNCHRONOUS throw before the first await, which would + // otherwise take the following calls down with it. + // + // ⚠️ These three do NOT currently complete — see issue #160. Traced with + // console markers: execution demonstrably reaches this line and all three + // are invoked, yet none reaches its success path or its catch, so each is + // still parked on its `await fetch`. Consequence: pin badges, advisory + // badges and auto-update policy state are silently absent. + void loadAdvisories().catch(() => undefined) + void loadPins().catch(() => undefined) + void loadPolicies().catch(() => undefined) }) watch([safeModeEnabled, installedVersion, selectedVersion], () => { @@ -1618,6 +1670,7 @@ watch(dryRunEnabled, () => {
diff --git a/tests/e2e/pinning.spec.ts b/tests/e2e/pinning.spec.ts index 7ed78999..266e5945 100644 --- a/tests/e2e/pinning.spec.ts +++ b/tests/e2e/pinning.spec.ts @@ -75,11 +75,41 @@ test.describe('version pinning', () => { }) expect(put.ok(), 'seeding a pin via API should succeed').toBeTruthy() + // The badge renders `v-if="pinFor(app.id)"`, fed by GET /api/pins. So + // "element(s) not found" has three quite different causes — the pin was + // never stored, the list endpoint does not return it, or the component + // did not render it — and the locator alone cannot tell them apart. + // Reading the API first turns one opaque failure into a statement about + // WHICH link of that chain broke. + const pinsList = await page.request.get('/ocs/v2.php/apps/app_versions/api/pins?format=json', { + headers: { 'OCS-APIRequest': 'true' }, + }) + const listed = (await pinsList.json())?.ocs?.data?.pins ?? [] + expect( + listed.map((p: { appId: string }) => p.appId), + `GET /api/pins must list the pin just seeded for "${APP}" — if this passes and the badge below does not appear, the break is in the component, not the API`, + ).toContain(APP) + await openSettings(page) await openTab(page, 'Apps') + // Located by the card's OWN app id, not by its visible text. The card + // renders `{{ app.label }}`, so a text match proves a label contains the + // string — it says nothing about `app.id`, which is the value the + // badge's `pinFor(app.id)` actually keys on. Those are the two sides of + // the comparison under test, so matching on the wrong one would make a + // green assertion meaningless. + const card = page.locator(`article[data-app-id="${APP}"]`) + await expect( + card, + `no app card has data-app-id="${APP}" — the pin is keyed by appId, so if the card's id differs from the API's appId that mismatch IS the bug`, + ).toBeVisible() + const badge = page.getByTestId('pin-badge').first() - await expect(badge).toBeVisible() + await expect( + badge, + `the API lists a pin for "${APP}" and its card IS rendered (both asserted above), so the badge's own v-if="pinFor(app.id)" is what did not match — compare the pin's appId against the card's app.id`, + ).toBeVisible() await expect(badge).toContainText('Pinned') // Attribution is carried in the title so hovering explains the badge. await expect(badge).toHaveAttribute('title', /admin/)