Skip to content

bc-l8ub: Uncaught TypeError - #689

Open
mordam wants to merge 2 commits into
mainfrom
worktree-sw-5xx-cache-l8ub
Open

bc-l8ub: Uncaught TypeError#689
mordam wants to merge 2 commits into
mainfrom
worktree-sw-5xx-cache-l8ub

Conversation

@mordam

@mordam mordam commented Aug 24, 2026

Copy link
Copy Markdown
Owner

public/sw.js is network-first with the cache as the offline fallback, but "the network did not answer" was implemented as "the fetch rejected" — a refused connection, a reset socket, a phone in a tunnel. A 502 or a 503 is a fulfilled promise, so it went straight to the page. The one moment this app reliably produces them is a merge self-deploying: bin/router.js holds the port across the swap and answers 503 for the second or two before the new backend is up. A page loading through that window gets some of its scripts and not others, with the real files sitting in the cache the whole time. cachedInstead closes that: on 5xx alone, ask the cache for this address (with the same ignoreSearch widening fallback uses) and serve the hit; hand back the server's own error when there is no hit. It is a separate function from fallback rather than a reuse of it because fallback's last resort is caches.match('/') — right for an offline navigation, and precisely wrong here, since the index page answered to a <script src> is a SyntaxError and the same missing global reached by a longer road.

That is the diagnosis bc-l8ub needed, and it is on the bead in full. window.beadcause.route was undefined at viewbar.js:291 not because of load order — all twelve pages load /hashroute.js immediately above /viewbar.js and nothing assigns window.beadcause flat — but because /hashroute.js never arrived. bc-y8wf recorded GET /api/poll failed from the same browser three minutes earlier, and main took merges at 18:33Z and 18:36Z. The message being (reading 'viewOfPath') rather than (reading 'route') is itself evidence: /console loads stream.js and presence.js above viewbar.js and both create the namespace, so the object survived and only route was missing.

The second, smaller half is that the crash said nothing about its cause. viewbar.js still refuses to draw without the grammar — that decision is argued in the file and it was the honest half of this bug — but it now throws a message naming /hashroute.js instead of a TypeError on a line that does not mention it, and reads route with ?. so the two shapes of the absence (namespace present, namespace absent — which of the two you get depends only on whether the page loads stream.js or presence.js above the row) give one message rather than two fingerprints for one bug. test/panes.mjs had pinned that line character for character, so the optional read read as the requirement being dropped; its assertion now checks the requirement — a throw that names the file — rather than the exact source text.

What I am least sure of is whether a 5xx was really what came back, as opposed to a stream reset. A reset already falls to the cache, so it would mean this fix does not cover the actual occurrence. I have argued it both ways on the bead: a reset would have made the missing script a fetch failure that fallback handled, and no accompanying SyntaxError or (reading 'route') bead exists to suggest anything else was broken on that load. Either way the 5xx hole is real and was reachable from the same event.

Tests: bin/b7e-gate over the final tree: all 422 suites passed, exit 0. test/swreport.mjs grew seven checks for the new path (503 and 502 served from cache; a 5xx on an uncached path handed back as itself with the index page never reached for; a 404 passed straight through; the ignoreSearch widening; a rejecting cache still answering the 5xx and relaying; and a 200 control proving the others are not passing against a cache-everything worker) — 35/35. test/panes.mjs 38/38 after its static assertion was rewritten.

Worth knowing: The 5xx branch cannot be exercised without a real deploy racing a real page load, so it is proven by the vm harness rather than in a browser. The narrowness is the thing to watch: if a cached entry ever went stale under a path that then started 5xx-ing, this would serve the stale copy for as long as the outage lasted. Judged the better failure than serving an error page to a script tag, and 5xx-only keeps it away from 404s and the login redirect. Second, the error viewbar.js now throws has a different message and line from the one on this bead, so a recurrence files a fresh bead rather than commenting here — unavoidable either way, since lib/errors.js keys on file:line and any edit above line 291 moves it.

Left undone: Nothing was done about the router answering 503 at all — batching the swap so the port never serves an error is a different and much larger change, and the cache is the right place to absorb a two-second outage. Also left alone: viewbar.js still refuses to draw without the grammar rather than degrading to a plain link row. That is a documented decision (the paragraph above panes), it was the honest half of this crash, and softening it unattended would have been the wrong call.

Files changed — 4 files · +196 −13 · against `main`
test/swreport.mjs  +89 −1
public/sw.js       +58 −2
public/viewbar.js  +36 −7
test/panes.mjs     +13 −3

Opened by a beadcause worker session on bc-l8ub — Uncaught TypeError: Cannot read properties of undefined (reading 'viewOfPath') — viewbar.js:291. It merges itself once the checks report; merging is what closes the bead. If this is still open, something stopped that, and the reason is on bc-l8ub and in Adam's inbox.

bead: bc-l8ub

public/sw.js is network-first with the cache as the offline fallback, but
`fetchAndStore` only rejects when the *fetch* fails. A 502 or 503 is a
fulfilled promise, so it went to the page untouched — and the one moment
this app reliably produces them is a merge self-deploying, with
bin/router.js holding the port across the swap. A page loading through
that window got some of its scripts and not others, with the real files
sitting in the cache the whole time.

`cachedInstead` covers 5xx alone, and is deliberately narrower than
`fallback`: it asks only whether the cache holds this address (plus the
same `ignoreSearch` widening) and hands the server's own error back when
it does not, because `fallback`'s last resort is `caches.match('/')` and
the index page served to a script tag is a SyntaxError and the same
missing global by a longer road.

public/viewbar.js still refuses to draw without the grammar — that is the
documented decision and it was the honest half of the crash — but it says
which file is missing rather than throwing a bare TypeError on a line
that does not mention hashroute.js. test/panes.mjs pinned that line
character for character; it now asserts the requirement instead.
@mordam

mordam commented Aug 24, 2026

Copy link
Copy Markdown
Owner Author

A beadcause worker opened this and does not merge its own work. It is on the merge queue as bc-p88t0.

@mordam

mordam commented Aug 24, 2026

Copy link
Copy Markdown
Owner Author

This branch is green. The test check's red is a CI timeout, not a failure in the diff.

Run locally on worktree-sw-5xx-cache-l8ub at 72d00c7e, the head of this PR:

all 422 suites passed
[exited with code 0]

Wall clock ~26 minutes on a loaded Mac (four Claude sessions and a daemon alongside it), against the ~14 minutes CI normally spends.

For the record, since this is the second PR to hit it today:

  • Run 32729224643 was cancelled at the 20-minute cap ("The job has exceeded the maximum execution time of 20m0s") — no assertion failed and no suite reported red.
  • bc-y8wf: GET /api/poll failed — Failed to fetch — api/poll #688's run (32728820701) was cancelled the same way within five minutes of it, on a branch sharing no files with this one.
  • The runs immediately before and after both went green in ~14 minutes, so the cap is not chronically tight.
  • The two suites this PR actually changes, test/swreport.mjs (35/35) and test/panes.mjs (38/38), each complete in under a second.

I have no admin rights on this repository, so gh run rerun refuses and the re-run has to come from Adam or from a push to the branch.

Nothing here is a review verdict — bc-p88t0 still reads "waiting on a review: nothing has reviewed this pull request yet." This is only the check evidence.

@mordam

mordam commented Aug 24, 2026

Copy link
Copy Markdown
Owner Author

The beadcause merge queue tried to merge this and could not: it has been tried 3 times and stopped at the same place each time. 1 check failing (test). A merge queue will not merge over a check the branch broke — if it is a flake, that is your call. Tried 3 times — that was the last. It is Adam's call now — see bc-p88t0.

…-l8ub

# Conflicts:
#	public/viewbar.js
#	test/panes.mjs
mordam added a commit that referenced this pull request Aug 27, 2026
…vocate… (#831)

The conflict-resolver brief told an unattended session to run the repo's
own gate after the
merge and push afterwards. The *afterwards* is what this changes.

Beadcause's gate serialises across every worktree on this Mac. On the
run that prompted
this — resolving #689 — `npm test` printed `3 gates ahead of you, oldest
started 53m ago`
and was still queued when the session gave up on it; the GitHub Actions
run over the same
merge, started by the push, was green in about forty seconds. Two agents
were being asked
the same question, and the slower one was holding the window open for
it.

**Nothing here is trusted to nobody.** `lib/mergeadvocate.js` refuses to
merge over a red
rollup, a pending one, or a head commit with no checks reported for it
at all — that last
arm is bc-ysqd.1, added after #480 for exactly the cost of conflating
"nothing ran" with
"passed". So a push that has not been verified cannot land by being
ignored, and the verdict
survives the resolver's window closing, which a local run's does not.

**The local gate keeps a paragraph rather than losing one.** Two cases
where it is still the
only answer, and the brief names both: a workspace with no CI wired up —
which the
advocate's own `baseHasChecks !== false` arm exists because it is a real
configuration — and
a check that comes back red on something the diff does not explain,
where finding out which
suite is worth the wait.

Step 5 is now push-and-read-the-checks; step 6 is the unlock alone.
`test/prfull.mjs` pinned
the old sentence character for character, so its assertion now pins what
the step is *for*
rather than how it was worded. README's six-point summary of the brief
follows.

**Tests:** test/prfull.mjs (67), sweepcard (169), mergegate (36),
sweepanswer (32), prsweep (39), documents — all green locally; the full
suite is CI's, which is the point of the change

**Worth knowing:** Prompt and README prose plus one test's assertions.
The claim it rests on is that a workspace beadcause resolves for has PR
checks — where one does not, the brief still says run the local gate
before pushing.

<details><summary><b>Files changed</b> — 3 files · +45 −14 · against
`main`</summary>

```
lib/session.js   +24 −8
test/prfull.mjs  +12 −3
README.md        +9 −3
```

</details>

---

_Opened by a beadcause worker session on bc-cz5wx — *The resolver waits
on a local gate that CI and the merge advocate already answer*. It
merges itself once the checks report; merging is what closes the bead.
If this is still open, something stopped that, and the reason is on
bc-cz5wx and in Adam's inbox._

bead: bc-cz5wx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants