Skip to content

feat(node): add BrowserContext shim and page default timeouts#108

Merged
suchintan merged 3 commits into
Skyvern-AI:mainfrom
Ralkage:feat/node-context-and-default-timeouts
Jul 22, 2026
Merged

feat(node): add BrowserContext shim and page default timeouts#108
suchintan merged 3 commits into
Skyvern-AI:mainfrom
Ralkage:feat/node-context-and-default-timeouts

Conversation

@Ralkage

@Ralkage Ralkage commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Summary

The Node binding bridges launch/newPage/goto/click/fill/title/textContent/evaluate/screenshot/close, but two commonly-used Playwright entry points were missing, so drop-in scripts fail early with browser.newContext is not a function / page.setDefaultTimeout is not a function. This adds them in the JS wrapper only — the native engine and prebuilt binary are unchanged.

Added:

  • browser.newContext(options)BrowserContext with newPage(), pages(), browser(), setDefaultTimeout(), setDefaultNavigationTimeout(), close(); plus browser.contexts().
  • page.setDefaultTimeout() / page.setDefaultNavigationTimeout().

Design notes / honest caveats

  • BrowserContext is a compatibility shim. The alpha Node engine drives a single Chromium process, so it does not provide the storage/cookie isolation of a real Playwright context. It exists to keep the familiar newContext().newPage() shape working and to propagate default timeouts to the pages it creates. Documented in code comments and the type declarations.
  • ignoreHTTPSErrors is accepted but not enforced. Certificate handling is a launch-time concern for the current engine and can't be toggled per-context after the browser is running. Rather than silently no-op a security-relevant flag, the shim emits a one-time console.warn pointing to launch({ args: ['--ignore-certificate-errors'] }).
  • Timeout precedence matches Playwright: explicit per-call timeout wins; otherwise the page default; otherwise undefined is passed through so the native layer applies its own default. Navigation falls back navigation-default → default.

Testing

  • Extended node/smoke.mjs to run the full newContext → setDefaultTimeout → newPage → setDefaultNavigationTimeout → goto/fill/click/textContent/evaluate/screenshot → context.close() flow; passes against the prebuilt binary.
  • Verified against the published rustwright@0.1.1 native binary by overlaying this wrapper: an independent API-coverage probe went from 10/12 to 12/12, with newContext and setDefaultTimeout now passing and existing methods unaffected.

Notes

Types in node/index.d.ts are regenerated via node/scripts/generate-types.mjs (the source of truth), which is updated in the same commit.

🤖 Generated with Claude Code

The Node binding exposed launch/newPage/goto/click/fill/title/textContent/
evaluate/screenshot/close but was missing two commonly-used Playwright entry
points, which made drop-in scripts fail early:

  - browser.newContext(...) / context.newPage() / context.close()
  - page.setDefaultTimeout() / page.setDefaultNavigationTimeout()

Both are implemented in the JS wrapper only; the native engine and prebuilt
binary are unchanged.

BrowserContext is a compatibility shim: the alpha Node engine drives a single
Chromium process, so it does NOT provide the storage/cookie isolation of a real
Playwright context. It exists to keep the familiar newContext().newPage() shape
working and to propagate default timeouts to pages it creates.

ignoreHTTPSErrors is accepted for API compatibility but cannot be toggled per
context after launch, so it emits a one-time warning pointing users to
launch({ args: ['--ignore-certificate-errors'] }) rather than silently
no-opping a security-relevant flag.

Page default timeouts follow Playwright precedence: an explicit per-call
timeout wins, otherwise the page default is used, otherwise undefined is passed
through so the native layer applies its own default.

Types are regenerated via scripts/generate-types.mjs and the smoke test now
exercises the context + default-timeout flow.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@suchintan

Copy link
Copy Markdown
Member

Thanks for this — contexts and default timeouts are exactly the right next surface for the Node binding, and the close-cascade handling here is careful work.

One architectural note before this lands: we just adopted a repo-wide thin-shim rule (see the new Shim Lightness Principle in CODE_ARCHITECTURE.md and the Thin-shim rule section in bindings/CONTRACT.md, both on main as of today). Timeout-precedence policy (explicit > page default > context default) is engine semantics, so the plan of record is a core-side default-timeout register — page.set_default_timeout / set_default_navigation_timeout and a lightweight context handle in rustwright-core, exposed through the C ABI and napi — so all seven bindings inherit contexts + default timeouts from one implementation instead of each shim re-implementing the precedence logic (that's roadmap track 10 in CODE_ARCHITECTURE.md).

Two ways forward, both fine by us:

  1. Rework this PR to add the register core-side (napi + C ABI exports included) with the Node wrapper reduced to marshalling — happy to help scope it; the core plumbing is straightforward and it makes this feature land for Go/Java/C#/Ruby/PHP at the same time.
  2. Land this as-is behind an explicit Node-only experimental gate (per the contract rule) with a follow-up issue to reclaim the logic into the core — the newContext no-op veneer and warning you added are a good fit for that gate.

If you'd rather we take the core-side piece, say so and we'll pick it up — your Node surface work carries over either way.

suchintan and others added 2 commits July 21, 2026 23:18
…educed to marshalling

Builds on the BrowserContext/default-timeout surface from this PR and
moves the policy into the engine per the thin-shim rule: each core page
owns a DefaultTimeoutRegister (page/context slots x general/navigation),
and resolution (explicit > page > context > 30s core default, with
navigation falling back through general) happens core-side when a call
passes no timeout. Exposed via four C ABI setters (NaN clears, per the
contract) and matching napi Page methods. The node shim keeps the public
surface (context veneer, setters, close cascade, warning) but deletes
its JS precedence math; setters marshal into native slots, context
setters cover current and future member pages. With no slots set,
behavior is byte-identical for every existing caller. Pure node tests
pin the marshalling boundary; Rust tests pin the full precedence
lattice.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@suchintan

Copy link
Copy Markdown
Member

Following up on the review note above — I've pushed the core-side rework onto this branch (your commit stays as the foundation; the surface API you designed is unchanged). What moved: the timeout-precedence policy now lives in the engine as a per-page DefaultTimeoutRegister (page/context slots x general/navigation), resolved core-side and exposed through four new C ABI setters + napi methods, so Go/Java/C#/Ruby/PHP inherit contexts + default timeouts from the same implementation. The node shim keeps your BrowserContext veneer, setters, close cascade, and warning — it just marshals values into the native slots instead of resolving precedence in JS.

Validation: core tests 62/62 incl. a full precedence-lattice suite; napi addon built; node --test 8/8 (your test structure extended with marshalling-boundary tests); back-compat pinned — with no slots set, every existing caller's behavior is byte-identical.

Merging once CI is green. Thanks for kicking this off — the cross-binding version of this feature exists because of your PR.

@suchintan suchintan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Surface design from the original PR + core-side precedence per the thin-shim rule; validated locally (core 62/62 incl. precedence lattice, napi build, node tests 8/8, back-compat pinned).

@suchintan
suchintan merged commit aaf9057 into Skyvern-AI:main Jul 22, 2026
12 checks passed
@github-actions

Copy link
Copy Markdown

Synced to rustwright-cloud: https://github.com/Skyvern-AI/rustwright-cloud/pull/118

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