fix(browser): keep draining load-delaying scripts after a page exception

[ntdatt812]

Fixes #699.

What changed

drive_load_delaying_scripts answered an event-loop error with return false, which abandons every still-pending load-delaying script. Ok(Err(_)) now logs and keeps draining. The absolute deadline immediately above was always the real bound and stays authoritative, so this cannot extend a run. Only the first error warns; the rest go to debug so a page that throws every tick cannot flood the log.

The question that decides this is whether an event-loop error means the page is finished or merely that one tick carried an exception. I measured it: four consecutive ticks per case, one fresh runtime each.

queueMicrotask throw : ERR(Event loop error: Error: boom ...)  then Ok, Ok, Ok
Promise.reject       : ERR(Event loop error: Error: boom ...)  then Ok, Ok, Ok
setTimeout throw     : Ok, Ok, Ok, Ok
dynamic import fail  : ERR(...)                                then Ok, Ok, Ok

It is transient. The tick carrying the exception fails and the next ones poll clean. So the pump was discarding the page's remaining work over a condition that had already cleared, and discarding it silently: nothing surfaces to CDP, the scripts simply never arrive, and the run ends on the script deadline instead. That silence is why the reporter needed instrumented Chromium to find it.

The same probe explains why the reporter's synthetic repros did not fail. setTimeout is already contained at bootstrap.js:881 (try { f(...args); } catch(e) { console.error("Timer error:", e); }), so a throwing timer behaves like Chromium. The throw that reaches the pump arrives through a microtask-class callback, matching their stack ending at eventLoopTick (ext:core/01_core.js).

I took @yinnho's second suggestion rather than their first: the dispatch-boundary containment they described is largely present here already (timers at bootstrap.js:881, an onunhandledrejection at bootstrap.js:82), and the remaining path is inside deno_core rather than in our JS, so the pump is where this has to be handled.

Validation

New test load_delaying_script_driver_survives_a_page_script_exception, next to the existing deadline test and using the same harness. It installs a real pending load-delaying script against the delayed-script server, defers a throw onto a pump tick, and asserts the script is still fetched and the pump completes.

Red with the old return false restored, green with the fix:

FAIL obscura-browser page::tests::load_delaying_script_driver_survives_a_page_script_exception
PASS obscura-browser page::tests::load_delaying_script_driver_survives_a_page_script_exception

Full four-configuration sequence from CONTRIBUTING, in a rust:latest container so the result matches CI rather than my Windows host:

Step	Result
cargo build --release -p obscura-cli --bins --features render	ok
cargo nextest run --release --features render --no-fail-fast	1507 passed, 4 skipped
cargo build --release -p obscura-cli --bins --no-default-features	ok
cargo nextest run --release -p obscura --no-fail-fast	23 passed
cargo nextest run --release --workspace --exclude obscura --exclude obscura-render --no-default-features --no-fail-fast	758 passed, 3 skipped
cargo check --release -p obscura-render --no-default-features	ok

A constraint on any test written here, which cost me two attempts

The throw has to be deferred onto a pump tick. Thrown during the installing script's own microtask drain, it clears the pending-script bookkeeping before the fetch even starts, so the pump completes trivially and the test passes for the wrong reason. That is a separate defect from this one and I have not touched it.

Rendering

Not applicable. This is the script-loading pump; no layout, paint, screenshot, screencast or PDF code is touched.

Performance

This can only shorten a run, never lengthen one. Before, a transient error ended the drive early and left the page waiting on the script deadline; now the pump keeps going and finishes when the work is actually done. The absolute deadline above the loop is unchanged and still bounds the worst case. The added cost per error is one counter increment and one log line, and the log is rate-limited to a single warn.

Two things I found and deliberately did not fold in

Promise.reject reaches the event loop despite the handler. bootstrap.js:82 sets globalThis.onunhandledrejection to call preventDefault(), and the probe above shows a plain Promise.reject still surfacing as Event loop error. That handler is not doing what it looks like it does. It is a real gap, it is not what breaks the reported page, and it belongs in its own issue.

The bookkeeping reset described above, where a throw during the installing script's microtask drain drops the pending set. Also separate.

Happy to file both, or to fold either in if you would rather have one change. Neither needs an answer before this merges.

Checklist

• The change is focused and does not remove existing behavior without justification.
• Tests cover the failure or feature.
• Existing tests pass, including render and no-render configurations when affected.
• I checked for CPU, latency, and memory regressions.
• Public API or user-facing behavior changes are documented.

[yinnho]

Reviewed the patch — the shape is right.

The deadline-as-only-bound tradeoff is the correct call: on a page that throws every tick, the pump now burns its remaining budget instead of abandoning still-pending work over an error that a fresh tick would clear, and that is bounded (the absolute deadline) rather than unbounded. yield_now() in the error arm keeps a pathological page from spinning the loop hot between polls, and warn-once/debug-after keeps a noisy page from flooding the log. The regression test matches the measurement table from #699 (throw deferred through a timer so it lands on a pump tick).

One question, from the test comment: "Thrown during the installing script's own microtask drain it would clear the pending-script bookkeeping before the fetch even starts, which is a different bug from the one under test here." Is that second failure mode tracked anywhere? We would like to check our tree for it — if it turns out to be real on our side too, it deserves its own issue rather than riding on this PR.

[ntdatt812]

Obstacle course, for the CONTRIBUTING pre-PR item I could not tick from a unit test. Companion repo at the ref your CI pins (6ebac82), same host, same invocation, base and candidate back to back:

revision	correctness	median latency
main (c138019)	32/33	3016.7ms
this branch	32/33	3016.0ms

No delta. The single failure is the same stage on both, and it fails on untouched main:

observer-intersection   modern-web   FAIL   expected 'io:50', got ''

So it is a property of this host rather than of the change, and I have not chased it further since it is outside the diff. It is deterministic rather than flaky here: re-running just that stage with --runs 3 --warmup 1 reproduces it exactly. Mentioning it only in case a 32/33 on a plain rust:latest Debian container is news to you; on your ubuntu-22.04 runner it may well be 33/33.

Everything else in this PR is unchanged.

[yinnho]

That stage is #671, and the back-to-back table here (no delta between main and this branch) is the expected shape of it — nothing in this PR touches the observer path. Two notes that might save the next person the chase:

• The failure is spec-correct withholding, not a missing computation step. The fixture observes the sentinel exactly once (its comment claims a fresh observation per batch, but io.observe() runs once), and nothing flips isIntersecting or the threshold index after the appends — so an engine following the spec delivers one entry and stops. Headless Chromium 151 produces the identical expected 'io:50', got '' through the same runner; measurement tables and the aff0172 → 10b8492 history are in Obstacle course stage observer-intersection fails on main, headless Chrome fails it identically, and CI does not catch it #671.
• For completeness: our obscura-lineage engine (diting) never returns empty on this stage either, because its IO is still the pre-10b8492 geometry-naive emulation (always-intersecting, mutation re-fire, burst schedule) — which is what the fixture was originally written against. So the stage currently measures engine history rather than browser behavior from both directions: spec-following engines (including Chrome) fail on the missing re-observation, and the fixture's comment describes an emulation that no longer matches main.

Until the fixture re-observes or toggles the sentinel, I'd treat the stage as known-fail per #671 rather than a regression signal for PRs like this one.