The three non-blocking loaders at the end of onMounted never run (pins, advisories, policies)

[rubenvdlinde]

Measured, not inferred

Captured every app_versions URL the browser requested while loading the admin settings page (tests/e2e/pinning.spec.ts, PR #159):

200 /index.php/settings/admin/app_versions
200 /apps/app_versions/css/app_versions-main.css
200 /apps/app_versions/css/main-*.chunk.css
200 /apps/app_versions/js/app_versions-main.mjs
200 /ocs/v2.php/apps/app_versions/api/pats?format=json
200 /ocs/v2.php/apps/app_versions/api/sources?format=json
200 /ocs/v2.php/apps/app_versions/api/cache?format=json
200 /ocs/v2.php/apps/app_versions/api/update-channel?format=json
200 /apps/app_versions/img/app.svg
200 /ocs/v2.php/apps/app_versions/api/apps?format=json

Absent: /api/pins, /api/advisories, /api/policies — which are exactly the last three statements of onMounted:

try {
    await checkUpdateChannel()
    await loadApps()
} 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()

Everything before that point fires (update-channel, then apps). Everything after it does not.

Visible consequence

pinning.spec.ts:70 has failed on every run: pins stays {}, so the app-card badge's v-if="pinFor(app.id)" never matches and no pin badge renders — even though the pin exists and GET /api/pins returns it when called directly. Advisory badges and auto-update policy state are presumably affected the same way.

What this is NOT

Each ruled out by reading the source, so nobody re-checks:

• not the API — a direct GET /api/pins returns the pin (asserted in the test before the UI is touched)
• not Pin::toArray() — it carries no appId, so the + ['appId' => …] union is safe (PHP's + keeps the LEFT operand)
• not an id mismatch — /api/apps returns 'id' => $appId and /api/pins returns 'appId' => $appId, both the same app id
• not core-app filtering — coreAppsVisibility defaults to 'show'
• not a throwing loader — checkUpdateChannel, loadApps and loadAdvisories each wrap their whole body in try/catch
• not the test harness — the capture is proven by a positive control asserting /api/apps was seen, and the wait uses expect.poll, not a bare assertion

Why static reading did not finish the job

The try has a finally but no catch, so anything thrown by the awaited calls would run finally and then propagate, skipping the three void statements. That is the obvious mechanism — but all three loaders catch internally, so what throws is not visible from the source. It needs a browser console/stack trace from a running instance.

Filed rather than guessed at: changing onMounted blind risks breaking the 92 specs that pass.

[github-actions]

Thank you for opening this issue! It has been assigned to @rubenvdlinde for review.

What happens next:

1. A maintainer will review this issue and determine the appropriate action
2. If accepted, it may be converted into an OpenSpec change for structured development
3. You will be notified of any status changes

Triage labels:

• triage:accept-feature — accepted as a new feature
• triage:accept-bugfix — accepted as a bug fix
• triage:needs-info — more information needed
• triage:wontfix — will not be addressed
• triage:duplicate — duplicate of another issue

[rubenvdlinde]

The missing-catch hypothesis is DISPROVEN

I added a catch to the try/finally in onMounted and .catch() to each of the three void calls, on the reasoning that a finally without a catch re-throws and would skip them.

It changed nothing. /api/pins is still never requested, and pinning.spec.ts:70 still fails.

The run genuinely exercised the new code — the E2E job logged Prebuilt frontend artifact carried no JavaScript — building here instead, so it rebuilt the bundle rather than reusing a stale one. That was worth confirming before trusting the negative result.

So the cause is not an exception propagating past those statements. With a catch in place nothing can propagate, and they still do not run.

What that leaves

The remaining shape that fits the evidence is that execution never reaches them — i.e. one of the awaited calls never settles:

await checkUpdateChannel()   // /api/update-channel returned 200
await loadApps()             // /api/apps returned 200, and the list renders
// ↓ never reached
void loadAdvisories(); void loadPins(); void loadPolicies()

Both requests return 200 and apps.value is demonstrably populated (the card is located by data-app-id), so the bodies complete. Whether the returned promise then settles is the open question, and that needs a debugger or a console trace on a running instance — it is not visible from the source or from network capture.

Alternatively the request is made but never fires a response event (an aborted request fires requestfailed instead), which the current capture would not see. Adding a requestfailed listener would separate those two cases cheaply.

Status of the change

The added catch is defensible hardening — an unhandled rejection in onMounted should not be able to take out later initialisation — but it is not a fix for this issue and is not claimed as one.

[rubenvdlinde]

Not aborted either — the request is genuinely never issued

Added a requestfailed listener, because an aborted request fires that and never response, so the earlier capture could not tell "never issued" from "issued and died".

Result: no FAILED(...) entries at all. The calls seen are unchanged:

200 /ocs/v2.php/apps/app_versions/api/pats
200 /ocs/v2.php/apps/app_versions/api/sources
200 /ocs/v2.php/apps/app_versions/api/cache
200 /ocs/v2.php/apps/app_versions/api/update-channel
200 /ocs/v2.php/apps/app_versions/api/apps

So /api/pins, /api/advisories and /api/policies are not requested, not aborted, and not failing — they simply never happen. All three absent together is the important part: they are three consecutive statements, so this is one control-flow fact, not three coincidences.

Where this stands against the source

try {
    await checkUpdateChannel()   // 200
    await loadApps()             // 200, and apps.value is demonstrably populated
} catch (error) { … } finally { isLoading.value = false }
void loadAdvisories().catch(…)   // ← none of these three fire
void loadPins().catch(…)
void loadPolicies().catch(…)

Re-read after the change to be sure the edit landed where intended: there is no early return, the catch is present, and the three calls are unconditional at the end of the callback. loadApps() assigns apps.value as its last statement, and the card IS rendered — so its body completes, and an async function whose body completes must settle.

By the source, those three lines should run. By the network, they do not. I cannot reconcile that from static reading or from request capture, and I am not going to invent a fifth hypothesis to fill the gap.

What would settle it

A browser console/stack trace from a running instance, or a console.log immediately before and after await loadApps() to see which side execution reaches. That needs an instance this box does not have.

Hypotheses eliminated so far

1. API does not return the pin — no, a direct GET returns it
2. Pin::toArray() clobbers appId via PHP's + union — no, it carries no such key
3. app id vs pin appId mismatch — no, both are $appId
4. core apps hidden by the visibility filter — no, defaults to 'show'
5. exception propagating past the void calls — no, adding a catch changed nothing (on a verified fresh bundle)
6. request aborted rather than absent — no, requestfailed never fires

[rubenvdlinde]

Cause identified: the three requests HANG — loadPins never resumes past its await fetch

The decisive run placed console.info markers around the dispatch and immediately after pins.value = map inside loadPins, then compared which printed:

marker	printed?
onMounted: reached post-load section	✅
onMounted: dispatched advisories/pins/policies	✅
loadPins keys: […] (after pins.value = map)	❌
loadPins failed: … (the catch)	❌

Neither the success path nor the catch runs. An async function that reaches neither is still suspended at an await — so loadPins is parked on await fetch('/api/pins') for the life of the page.

That single fact explains every earlier observation, including the ones that made this look contradictory:

• no response event — the request never completes, and page.on('response') only fires on completion
• no requestfailed — it is not aborted either, just pending
• no page error, no console error — nothing throws
• pins stays {} — its initial value is never overwritten, so v-if="pinFor(app.id)" never matches and no badge renders
• the direct GET /api/pins in the test succeeds — it runs before the page mounts, with nothing else in flight

Why all three, and why only these three

loadAdvisories(), loadPins() and loadPolicies() are dispatched together, unawaited, immediately after await loadApps(). All three hang; every endpoint requested before them returns 200.

The shape that fits is PHP session locking: Nextcloud holds an exclusive session lock per request, so concurrent requests from one session serialise. If the first of the three (/api/advisories, which correlates against upstream advisory data) blocks on a slow or unreachable upstream while holding that lock, the two behind it wait indefinitely — which is exactly the observed pattern, and exactly why a standalone request with nothing else in flight succeeds.

Why this matters beyond the failing test

Pin badges, advisory badges and auto-update policy state are all silently missing in normal use whenever that first call is slow — not just under Playwright. Nothing surfaces it: no error, no spinner, no log.

Suggested direction (not applied)

Either await the three in sequence rather than firing them concurrently, or give each fetch an AbortSignal.timeout(...) so a hang becomes a caught error instead of an eternal pending promise. The second is the smaller change and turns this class of failure visible everywhere, not only here.

The catch added in this branch already ensures that once a hang becomes an error, it is logged rather than swallowed.

[rubenvdlinde]

ROOT CAUSE FOUND — /api/advisories hangs, and it is not the pins endpoint at all

Reproduced on a live instance (localhost:8080, app enabled, real browser). The browser DOES issue all three requests:

203. GET /ocs/v2.php/apps/app_versions/api/advisories   ← NO STATUS: still pending
204. GET /ocs/v2.php/apps/app_versions/api/pins         => 200 OK
205. GET /ocs/v2.php/apps/app_versions/api/policies     => 200 OK

Measured directly:

endpoint	time
/api/pins	0.74s
/api/apps	1.81s
/api/advisories	60s, no response, http=000

Why it hangs

AdvisoryService::correlateAll():

foreach ($this->appManager->getEnabledApps() as $appId) {
    $results[$appId] = $this->correlate($appId);   // resolves a source per app
}

One source resolution per enabled app, serially, with no bound. This instance has 88 enabled apps, and the default binding is the App Store — so a single page load triggers 88 sequential catalogue lookups.

Why this made the pin badge fail

loadAdvisories() is dispatched first of the three. In CI its hang holds the PHP session lock, so /api/pins and /api/policies never get to run — which is why the network capture showed them absent, why no error was ever logged, and why pins stayed {} so v-if="pinFor(app.id)" never matched.

Locally the other two squeeze through (hence 200s above), so the badge would render here — the failure is load-dependent, which is exactly why it looked like flakiness.

Corrections to earlier comments on this issue

• "the app never requested /api/pins" — wrong. It does. My page.on('response') capture only fires on completion, so a pending request was invisible to it.
• "loadPins never completes" — true but not the cause. It is blocked behind advisories, not broken itself.
• The endpoint being cheap (isAdmin, one pin, getAppVersion) was correct and remains so — /api/pins is fast.

Fix direction

The real defect is correlateAll() doing N unbounded external calls on a page-load path. Options, in order of preference:

1. batch/cache the catalogue once per call instead of resolving per app;
2. bound the loop and paginate;
3. move correlation to a background job and have the endpoint read cached results.

The AbortSignal.timeout(8s) already merged in #159 stops the frontend hanging forever regardless, and the logged catch makes the failure visible — but neither addresses the N-call loop.