Client.makeRequest used to call self.perform(0) after handing the transfer
to libcurl. That perform() does two things: drives curl_multi_perform (so
bytes hit the wire) AND drains curl_multi_info_read messages, which is
what fires the user-facing header/data/done callbacks.
The issue is that, even in non-cache cases, a request could be immediately
resolved in libcurl, and thus callbacks executed synchronously.
By only calling `curl_multi_perform` on a new request, we prevent this from
happening.
Adds two ways to opt out of dedicated Web Worker loading entirely. The
Worker constructor still returns a Worker object so calling pages don't
throw, but no script fetch is initiated and the worker scope's eval
never runs (postMessage from the page is queued indefinitely with no
handler to drain it).
* CDP method LP.configureLoading { worker: bool } -- per-session
toggleable at runtime, alongside the existing { subFrame: bool }.
Both fields are now optional so callers can flip one without
resetting the other to its default. Backwards-compatible.
* CLI flag --disable-workers -- process-wide default applying to every
session and to the fetch subcommand. Operators can flip it on without
any driver changes. Mirrors --disable-subframes (#2401) exactly.
## Motivation
Reliably-reproducible SIGABRT in Worker.loadInitialScript whenever a
page constructs a Web Worker AND lightpanda is launched with
--http_proxy. Crash signature:
$msg="V8 fatal callback" location=v8::Context::Exit()
message="Cannot exit non-entered context"
Stack:
_browser.webapi.Worker.loadInitialScript
_browser.webapi.Worker.httpDoneCallback
_network.layer.InterceptionLayer.InterceptContext.doneCallback
_browser.HttpClient.processMessages
_browser.HttpClient.perform
_browser.HttpClient.tick
The Zig-side Enter/Exit pair around the worker's eval doesn't match
v8's entered_contexts stack invariant under that timing -- something
upstream of the loadInitialScript Exit leaves an extra Enter on the
stack, so v8's Utils::ApiCheck (`isolate->context() == *env`) fires
and the process aborts.
Reproducible against any Shopify storefront PDP (e.g.
https://weareallbirds.myshopify.com/products/mens-wool-runners) when
served through any HTTP proxy -- the proxy just adds enough latency
to surface the race; the same code path runs without --http_proxy
but the timing window is too tight to reliably hit. The Allbirds
trigger script is the Shopify web-pixel-extension worker, but ANY
Worker the page constructs hits the same code path.
The proper fix needs the v8 entered-contexts invariant to be
restored end-to-end through the worker eval. That's a deeper dig
into how Worker.loadInitialScript / WorkerGlobalScope.importScript /
ls.local.runMacrotasks compose with v8's microtask queues across
multiple contexts; I tried three intermediate fixes (deferring
loadInitialScript via the frame scheduler when other scripts are
mid-eval, replacing the post-eval cross-context runMacrotasks with
worker-only PerformCheckpoint, and removing runMacrotasks entirely)
and none stopped the crash. The bug is fired from inside the
synchronous tick path before the post-eval microtask handling
runs, which means the leak happens during Script::Run itself and
needs more targeted investigation.
This PR is the workaround so users hitting the SIGABRT on
storefront / analytics-heavy pages have a clean opt-in escape today.
For our use case (product catalog extraction) Workers carry no
extraction signal -- web-pixel sandboxes, analytics SDKs, marketing
tag pixels, etc. -- so disabling them removes a fragile code path
without any downside.
## Implementation
`Session.worker_loading_enabled: bool = true` -- default matches
existing behavior.
`Worker.init` short-circuits AFTER constructing the Worker /
WorkerGlobalScope / arena bookkeeping (so the JS `new Worker(url)`
expression doesn't throw):
if (!session.worker_loading_enabled) {
log.debug(.browser, "worker disabled", .{ .url = resolved_url });
return self;
}
Two ways to flip the flag, mirroring the --disable-subframes pattern:
1. LP.configureLoading { worker: bool } -- both subFrame and worker
are now optional fields in the params struct, so existing callers
passing only { subFrame } continue to work unchanged.
2. --disable-workers CLI flag -- added to CommonOptions (so it
applies to serve, fetch, mcp). New Config.disableWorkers() getter;
Session.init reads it as the initial value.
Total diff: +88 / -3 across 4 files (src/Config.zig,
src/browser/Session.zig, src/browser/webapi/Worker.zig,
src/cdp/domains/lp.zig).
## Verification
Reproducer pattern (puppeteer-core 24.42.0 + tiny CONNECT-tunnel
proxy on 127.0.0.1:9999, scripts in cdp-repros/):
serve --host 127.0.0.1 --port 9222 --http_proxy http://127.0.0.1:9999
serve --host 127.0.0.1 --port 9222 --http_proxy http://127.0.0.1:9999 --disable-workers
Driving https://weareallbirds.myshopify.com/products/mens-wool-runners:
baseline (no --disable-workers): 5/5 SIGABRT in
Worker.loadInitialScript with the v8 fatal callback above.
with --disable-workers: 10/10 successful, returns full
HTML (~1MB), no crash.
Test suite:
make test -> 637 of 637 tests passed (was 636/636 + new
cdp.lp: configureLoading toggles subFrame and worker
independently regression test).
zig fmt --check ./*.zig ./**/*.zig -> clean.
## Notes
* The CDP method is the same domain (LP.configureLoading) and same
shape as --disable-subframes' driver-side opt-in, so existing
Playwright / puppeteer integrations that already toggle
subframes don't need a separate code path -- one CDP call can
flip both.
* worker_loading_enabled = false does NOT remove Worker from the
global namespace (so feature-detection like
`if (typeof Worker !== 'undefined')` still reports true). It just
makes constructed workers no-op. Pages that postMessage to a worker
and wait for a response will hang on that promise forever (or
until the page is torn down). For our extraction use case that's
fine -- we control the worklist timeout anyway -- but it's worth
noting if upstream wants to surface the disabled state more
strongly (e.g. throw from postMessage, or remove the global
entirely behind an even-stricter flag).
* Once the underlying v8 entered-contexts invariant is restored in
Worker.loadInitialScript, this flag becomes a perf / sandboxing
tool rather than a correctness workaround. Worth keeping anyway:
blocking analytics / pixel workers is a reasonable thing to want.
## Related
* #2400 -- the iframe analog to this issue (subframe nav invalidates
executionContextId); same workaround pattern.
* #2401 -- introduced --disable-subframes / LP.configureLoading
{ subFrame } that this PR mirrors exactly for workers.
The HTMLDialogElement constructor was exposed with `open` / `returnValue`
IDL accessors, but the three instance methods that drive the open/close
state were missing. Per HTML §4.11.4 (The dialog element), `show()` sets
the `open` attribute if absent; `showModal()` throws `InvalidStateError`
when the dialog is already open and otherwise sets `open`; `close()`
removes `open`, optionally updates `returnValue`, and fires a non-
bubbling `close` event. The non-rendering steps (focus trap, backdrop,
top-layer placement) are intentional no-ops here — `[open]` reflecting
through to selectors and the `close` event firing are what downstream
CDP clients rely on.
Closes#2434
The build script wrote the version line to stdout, polluting any pipeline
that captures program output via 'zig build run' or similar. Banners
belong on stderr.
1 - Track owner of a request (for simpler / more accurate abort (TBD))
2 - Create Transfer upfront, make everything work on Transfer (not Request)
This helps remove ambiguity about cleanup and simplifies layers. For example
Robots request is just another normal request, not a special case. This gives
everything a stable address (the *Transfer which can be looked up by id)
These are currently reachable in Worker via the OffscreenCanvas, so when used
they crash. WPT /html/canvas/offscreen/2d.conformance.requirements.basics.worker.html
`lightpanda <subcommand> help`, `lightpanda <subcommand> --help`
now print only the relevant subcommand options plus common options,
instead of the full text.
`lightpanda help <subcommand>` is also supported
(and that's what use internally).
Reciprocal pointers so humans landing in CONTRIBUTING.md discover
the agent/operational conventions in AGENTS.md, and agents landing
in AGENTS.md discover the CLA gate and pre-PR checks. Adds a short
Development section (test + fmt commands) and a Before-opening-a-PR
checklist to CONTRIBUTING.md; CLA paragraph preserved verbatim and
moved to its own section.
While this PR touches a lot of files, and isn't trivial, many of the changes
are either:
1 - removing guards added in previous PRs, e.g.
https://github.com/lightpanda-io/browser/pull/1969https://github.com/lightpanda-io/browser/pull/2172https://github.com/lightpanda-io/browser/pull/2313https://github.com/lightpanda-io/browser/pull/2366
2 - Adding the `.ce_reactions = true` flag to various WebAPIs
CustomElements have callbacks, e.g. connectedCallback. Also, many WebAPI calls
are implemented as a series of mutations, e.g. appendChild = remove from current
+ append to new.
These two things interact in an important way: when should callbacks execute?
Before this PR, we were invoking callbacks at each individual step. This is
(a) technically wrong and (b) breaks a lot of assumptions (the reason the above
4 PRs were needed to fix bugs).
This PR adds a `_ce_reactions` queue to the frame. And, instead of invoking
callbacks, we "enqueue" the reaction. At various boundaries, a scope is created
the DOM manipulation is done, and then we pop the scope, invoking all queued
reactions.
