From 9d47eb2e955041f581cdf3ca40dde7fac8d06986 Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Mon, 11 May 2026 20:37:24 -0700 Subject: [PATCH] studio/tests: AbortSignal-bound in-page fetches and wall-clock watchdog for Playwright probes (#5391) * studio/tests: AbortSignal-bound in-page fetches + wall-clock watchdog Run 25696797934 / job 75446949358 on PR #5387 cancelled the "Chat UI Tests" macos-14 job at 30 min: studio.log went idle after the chat surface mounted, no further requests reached the server, and Playwright silently sat on a `page.evaluate(async () => fetch( /api/inference/load))` for 27+ minutes before the runner-level timeout fired. The two other Chat UI Tests jobs on the same SHA passed in 5-17 min, so this was a transient renderer wedge under --single-process Chromium, not a regression from the security bumps in that PR. Root cause: Playwright's `page.evaluate(...)` has no `timeout=` argument. If the JS body awaits a fetch whose promise never settles (the renderer's network thread stalls behind the busy main thread on the free macos-14 runner), the entire Python script hangs until something external kills it. Add two helpers in `_playwright_robust.py`: - `evaluate_fetch(page, url, *, method, headers, body, timeout_ms)` wraps `fetch()` in an `AbortController` so the JS resolves either with a real response or with `{status: 0, error: "AbortError..."}` after the budget elapses. Callers fail loud on a non-None `error` and the wedge surfaces as a one-line diagnostic instead of a 30-min cancel. - `install_wall_clock_watchdog(deadline_s)` starts a daemon Timer that hard-exits the process at the deadline. Belt-and- suspenders for any wedge inside the browser that the per- action timeouts cannot bound. Default 720s (12 min); healthy runs measure 5-9 min on macos-14 so the headroom is small without amplifying a wedge to the 30-min runner cap. Wire both into `playwright_chat_ui.py` and `playwright_extra_ui.py`: - Replace every `page.evaluate(async () => fetch(...))` site with `evaluate_fetch(...)`: refresh-token exchange, defaults fetch, inference load, health probe, post-rotation refresh. Five sites in chat_ui, two in extra_ui. - Arm the watchdog at the top of `with sync_playwright()` and cancel it on clean exit. Knobs (all default-safe, override only for slow runners): STUDIO_UI_WALL_TIMEOUT_S (default 720s) STUDIO_UI_FETCH_TIMEOUT_MS (default 30000ms) STUDIO_UI_LOAD_TIMEOUT_MS (default 180000ms) Verified locally with `python -c "ast.parse(...)"` on all three files and a unit smoke that confirms `evaluate_fetch`'s JS argument shape and that `install_wall_clock_watchdog` returns a daemonised Timer that responds to `.cancel()`. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- tests/studio/_playwright_robust.py | 141 ++++++++++++++++++++++++++++ tests/studio/playwright_chat_ui.py | 130 +++++++++++++++---------- tests/studio/playwright_extra_ui.py | 61 +++++++----- 3 files changed, 263 insertions(+), 69 deletions(-) diff --git a/tests/studio/_playwright_robust.py b/tests/studio/_playwright_robust.py index 928fa242eb..3deeb38cda 100644 --- a/tests/studio/_playwright_robust.py +++ b/tests/studio/_playwright_robust.py @@ -21,7 +21,9 @@ It does NOT depend on pytest -- both consumers run as plain Python. from __future__ import annotations import json +import os import sys +import threading import time import urllib.error import urllib.request @@ -404,3 +406,142 @@ def dump_diagnostics( except Exception as exc: if info is not None: info(f"diagnostics: json sidecar {name} failed: {exc}") + + +# ───────────────────────────────────────────────────────────────────── +# Bounded in-page fetch. +# ───────────────────────────────────────────────────────────────────── +# +# Playwright's `page.evaluate(...)` has no `timeout=` argument. If the +# JS body awaits a fetch that never resolves (the renderer's network +# thread wedges, the server accepts the connection but never replies, +# the macos-14 free runner under --single-process Chromium loses its +# IPC pipe), the entire Python script hangs until the runner-level +# timeout fires. Run 25696797934 / job 75446949358 on PR #5387 showed +# this exact failure: studio.log went idle after the chat surface +# mounted, no further requests reached the server, and Playwright +# burned 27+ minutes on a single page.evaluate(fetch /api/inference/ +# load) before the 30-min runner cancel. +# +# `evaluate_fetch` wraps the fetch in an AbortController.signal so the +# JS side resolves either with a real response or with a synthetic +# `{status: 0, error: "AbortError..."}` after `timeout_ms` ms. Either +# way page.evaluate returns and the script proceeds (or fails) with +# a debuggable signal instead of a silent wedge. +def evaluate_fetch( + page: Any, + url: str, + *, + method: str = "GET", + headers: dict[str, str] | None = None, + body: Any = None, + timeout_ms: int = 20_000, +) -> dict[str, Any]: + """Run `fetch(url, opts)` inside the page with an AbortSignal deadline. + + Returns `{"status": int, "body": parsed_or_text, "error": str|None}`. + On AbortSignal timeout returns `{"status": 0, "body": None, "error": + "AbortError: ..."}`. Callers should treat `status == 0` (or any + non-None `error`) as a transport failure rather than an HTTP + response. + + `body` may be a `str` (sent verbatim) or a `dict`/`list` (JSON- + encoded here). Pass headers explicitly when you need + `Content-Type: application/json` or an `Authorization` bearer. + """ + body_arg: str | None + if body is None: + body_arg = None + elif isinstance(body, (str, bytes)): + body_arg = body if isinstance(body, str) else body.decode("utf-8") + else: + body_arg = json.dumps(body) + js = """ + async ({url, method, headers, body, timeoutMs}) => { + const ctrl = new AbortController(); + const t = setTimeout(() => ctrl.abort(), timeoutMs); + try { + const opts = {method: method, headers: headers, signal: ctrl.signal}; + if (body !== null) opts.body = body; + const r = await fetch(url, opts); + clearTimeout(t); + let parsed; + try { + parsed = await r.json(); + } catch (_e) { + try { + parsed = await r.text(); + } catch (_e2) { + parsed = null; + } + } + return {status: r.status, body: parsed, error: null}; + } catch (e) { + clearTimeout(t); + return {status: 0, body: null, error: String(e)}; + } + } + """ + return page.evaluate( + js, + { + "url": url, + "method": method, + "headers": headers or {}, + "body": body_arg, + "timeoutMs": int(timeout_ms), + }, + ) + + +# ───────────────────────────────────────────────────────────────────── +# Wall-clock watchdog. +# ───────────────────────────────────────────────────────────────────── +# +# Even with every action and fetch bounded, a sufficiently strange +# wedge inside the browser (a CPU-pinned JS infinite loop, a renderer +# crash that doesn't propagate to Playwright, an asyncio deadlock in +# the sync wrapper) can still hang the script. The watchdog is a +# daemon Timer that calls `os._exit(2)` after `deadline_s` seconds, +# printing the wedge location to stderr so the CI log shows where the +# script was at force-kill time. The exit code matches "test failure +# by deadline" so the workflow's `set -e` propagates correctly. +# +# Pick `deadline_s` generously enough to cover the slowest healthy +# run -- macos-14 free runners with cold caches measure ~7-9 min for +# the comprehensive chat UI test. 12 minutes (720 s) leaves headroom +# without amplifying every real wedge to the 30-min runner-level cap. +def install_wall_clock_watchdog( + deadline_s: float, + *, + label: str = "playwright", + info: Callable[[str], None] | None = None, +) -> threading.Timer: + """Start a daemon Timer that hard-exits the process at `deadline_s`. + + Returns the Timer so the caller can `.cancel()` it on clean exit. + The Timer is daemonised; if the script exits normally before the + deadline the Timer dies with the process even without an explicit + cancel. + """ + + def _kaboom() -> None: + msg = ( + f"[{label}] WATCHDOG: hit {deadline_s:.0f}s wall-clock " + f"deadline; forcing exit(2). The script wedged somewhere " + f"the per-action timeouts could not bound. Inspect the " + f"most recent step printed above to localise." + ) + try: + sys.stderr.write(msg + "\n") + sys.stderr.flush() + except Exception: + pass + os._exit(2) + + timer = threading.Timer(deadline_s, _kaboom) + timer.daemon = True + timer.start() + if info is not None: + info(f"watchdog armed: hard-exit at {deadline_s:.0f}s") + return timer diff --git a/tests/studio/playwright_chat_ui.py b/tests/studio/playwright_chat_ui.py index 8f7dafa2a4..aa1d38c4e1 100644 --- a/tests/studio/playwright_chat_ui.py +++ b/tests/studio/playwright_chat_ui.py @@ -55,7 +55,9 @@ sys.path.insert(0, str(Path(__file__).resolve().parent)) from _playwright_robust import ( # noqa: E402 chromium_launch_args, click_and_wait_for_response, + evaluate_fetch, install_view_transition_killer, + install_wall_clock_watchdog, is_benign_console_error, is_benign_page_error, recover_or_replace_page, @@ -85,6 +87,17 @@ STRICT = os.environ.get("STUDIO_UI_STRICT", "0") == "1" # CI bump this without hard-coding a Mac branch in the test. TURN_TIMEOUT_MS = int(os.environ.get("STUDIO_UI_TURN_TIMEOUT_MS", "180000")) +# Wall-clock cap for the entire script. A healthy comprehensive run is +# 5-9 min; 12 min leaves headroom. Tunable via STUDIO_UI_WALL_TIMEOUT_S. +# See _playwright_robust.install_wall_clock_watchdog for rationale. +WALL_TIMEOUT_S = float(os.environ.get("STUDIO_UI_WALL_TIMEOUT_S", "720")) + +# Per-fetch budget for in-page fetches. The /api/inference/load call is +# usually the slowest legitimate request: it pulls the model into the +# llama.cpp worker. Give it ~3 min on a cold cache, less elsewhere. +FETCH_TIMEOUT_MS = int(os.environ.get("STUDIO_UI_FETCH_TIMEOUT_MS", "30000")) +LOAD_FETCH_TIMEOUT_MS = int(os.environ.get("STUDIO_UI_LOAD_TIMEOUT_MS", "180000")) + _n = [0] @@ -132,6 +145,11 @@ def parse_rgb(s): with sync_playwright() as p: + _watchdog = install_wall_clock_watchdog( + WALL_TIMEOUT_S, + label = "ui", + info = info, + ) # Pre-flight: bash-side wait_for already gated on /api/health # before launching us, but the macos-14 free runner has been # observed to surface a 200 /api/health while the auth DB is @@ -424,18 +442,18 @@ with sync_playwright() as p: "() => localStorage.getItem('unsloth_auth_refresh_token')", ) if refresh_token: - refresh = page.evaluate( - f"""async (rt) => {{ - const r = await fetch("{BASE}/api/auth/refresh", {{ - method: "POST", - headers: {{"Content-Type": "application/json"}}, - body: JSON.stringify({{refresh_token: rt}}), - }}); - return await r.json(); - }}""", - refresh_token, + refresh_resp = evaluate_fetch( + page, + f"{BASE}/api/auth/refresh", + method = "POST", + headers = {"Content-Type": "application/json"}, + body = {"refresh_token": refresh_token}, + timeout_ms = FETCH_TIMEOUT_MS, ) - token = refresh.get("access_token") + if refresh_resp.get("error"): + fail(f"/api/auth/refresh wedged: {refresh_resp['error']!r}") + refresh = refresh_resp.get("body") or {} + token = (refresh or {}).get("access_token") if not token: fail("could not obtain auth token after change-password") @@ -450,15 +468,18 @@ with sync_playwright() as p: "EXPECTED_DEFAULT_MODEL", "unsloth/gemma-4-E2B-it-GGUF", ) - defaults = page.evaluate( - f"""async (token) => {{ - const r = await fetch("{BASE}/api/models/list", {{ - headers: {{ "Authorization": "Bearer " + token }}, - }}); - return await r.json(); - }}""", - token, + defaults_resp = evaluate_fetch( + page, + f"{BASE}/api/models/list", + headers = {"Authorization": f"Bearer {token}"}, + timeout_ms = FETCH_TIMEOUT_MS, ) + if defaults_resp.get("error") or defaults_resp.get("status") != 200: + fail( + f"/api/models/list failed: status={defaults_resp.get('status')!r} " + f"error={defaults_resp.get('error')!r}" + ) + defaults = defaults_resp["body"] or {} if not defaults.get("default_models"): fail(f"/api/models/list returned no default_models: {defaults}") if defaults["default_models"][0] != EXPECTED_DEFAULT: @@ -499,27 +520,35 @@ with sync_playwright() as p: # ───────────────────────────────────────────────────── step("load GGUF via /api/inference/load (uses session cookie)") # Token already fetched above; reuse it for the load call. - load_resp = page.evaluate(f"""async () => {{ - const r = await fetch("{BASE}/api/inference/load", {{ - method: "POST", - headers: {{ - "Authorization": "Bearer {token}", - "Content-Type": "application/json", - }}, - body: JSON.stringify({{ - model_path: "{GGUF_REPO}", - gguf_variant: "{GGUF_VARIANT}", - is_lora: false, - max_seq_length: 2048, - }}), - }}); - return {{status: r.status, body: await r.json()}}; - }}""") + # AbortSignal-bounded: the macos-14 --single-process Chromium had been + # observed wedging on this exact in-page fetch (run 25696797934 / job + # 75446949358) with zero further requests reaching the server. The + # 3-min budget is generous for a cold-cache GGUF load; on a wedge we + # surface a clean failure instead of a 30-min runner cancel. + load_resp = evaluate_fetch( + page, + f"{BASE}/api/inference/load", + method = "POST", + headers = { + "Authorization": f"Bearer {token}", + "Content-Type": "application/json", + }, + body = { + "model_path": GGUF_REPO, + "gguf_variant": GGUF_VARIANT, + "is_lora": False, + "max_seq_length": 2048, + }, + timeout_ms = LOAD_FETCH_TIMEOUT_MS, + ) + if load_resp.get("error"): + fail(f"/api/inference/load wedged: {load_resp['error']!r}") if load_resp["status"] != 200: fail( - f"/api/inference/load returned {load_resp['status']}: {load_resp.get('body')!r}" + f"/api/inference/load returned {load_resp['status']}: " + f"{load_resp.get('body')!r}" ) - info(f"loaded model: {load_resp['body'].get('display_name')}") + info(f"loaded model: {(load_resp['body'] or {}).get('display_name')}") # Studio caches the per-context model state in zustand; reload # to make the chat composer pick up the loaded model. @@ -1185,10 +1214,13 @@ with sync_playwright() as p: # ───────────────────────────────────────────────────── # 14. /api/health stays healthy throughout. # ───────────────────────────────────────────────────── - health = page.evaluate(f"""async () => {{ - const r = await fetch("{BASE}/api/health"); - return {{status: r.status, body: await r.text()}}; - }}""") + health = evaluate_fetch( + page, + f"{BASE}/api/health", + timeout_ms = FETCH_TIMEOUT_MS, + ) + if health.get("error"): + fail(f"/api/health wedged: {health['error']!r}") if health["status"] != 200: fail(f"/api/health returned {health['status']}") @@ -1275,13 +1307,14 @@ with sync_playwright() as p: # The browser still has the pre-rotation access token. Refresh # tokens were revoked server-side by /change-password (auth.py), # so /api/auth/refresh from the browser context must now fail. - refresh_after = page.evaluate(f"""async () => {{ - const r = await fetch("{BASE}/api/auth/refresh", {{ - method: "POST", - credentials: "include", - }}); - return {{status: r.status}}; - }}""") + refresh_after = evaluate_fetch( + page, + f"{BASE}/api/auth/refresh", + method = "POST", + timeout_ms = FETCH_TIMEOUT_MS, + ) + if refresh_after.get("error"): + fail(f"/api/auth/refresh wedged: {refresh_after['error']!r}") if refresh_after["status"] == 200: fail(f"/api/auth/refresh should fail after CLI rotation; got 200") info( @@ -1392,4 +1425,5 @@ with sync_playwright() as p: ) info("PASS comprehensive UI flow") + _watchdog.cancel() browser.close() diff --git a/tests/studio/playwright_extra_ui.py b/tests/studio/playwright_extra_ui.py index 92025ed555..dccd2e423d 100644 --- a/tests/studio/playwright_extra_ui.py +++ b/tests/studio/playwright_extra_ui.py @@ -40,7 +40,9 @@ sys.path.insert(0, str(Path(__file__).resolve().parent)) from _playwright_robust import ( # noqa: E402 chromium_launch_args, click_and_wait_for_response, + evaluate_fetch, install_view_transition_killer, + install_wall_clock_watchdog, is_benign_page_error, recover_or_replace_page, wait_for_health, @@ -59,6 +61,9 @@ STRICT = os.environ.get("STUDIO_UI_STRICT", "0") == "1" # turn timeout because gemma-3-270m CPU inference is 3-5x slower than # ubuntu-latest's. TURN_TIMEOUT_MS = int(os.environ.get("STUDIO_UI_TURN_TIMEOUT_MS", "180000")) +WALL_TIMEOUT_S = float(os.environ.get("STUDIO_UI_WALL_TIMEOUT_S", "720")) +FETCH_TIMEOUT_MS = int(os.environ.get("STUDIO_UI_FETCH_TIMEOUT_MS", "30000")) +LOAD_FETCH_TIMEOUT_MS = int(os.environ.get("STUDIO_UI_LOAD_TIMEOUT_MS", "180000")) _n = [0] _failed: list[str] = [] @@ -94,6 +99,11 @@ def runtime_warn(m: str) -> None: with sync_playwright() as p: + _watchdog = install_wall_clock_watchdog( + WALL_TIMEOUT_S, + label = "ui-extra", + info = info, + ) # Health pre-flight (best-effort). Same rationale as in # playwright_chat_ui.py: bash-side health wait can succeed before # the auth DB has finished migrating on macos-14 free runners. @@ -261,36 +271,44 @@ with sync_playwright() as p: if not token: fail("no access token after change-password") sys.exit(1) - load_resp = page.evaluate(f"""async () => {{ - const r = await fetch("{BASE}/api/inference/load", {{ - method: "POST", - headers: {{ - "Authorization": "Bearer {token}", - "Content-Type": "application/json", - }}, - body: JSON.stringify({{ - model_path: "{GGUF_REPO}", - gguf_variant: "{GGUF_VARIANT}", - is_lora: false, - max_seq_length: 2048, - }}), - }}); - return {{status: r.status, body: await r.json()}}; - }}""") + load_resp = evaluate_fetch( + page, + f"{BASE}/api/inference/load", + method = "POST", + headers = { + "Authorization": f"Bearer {token}", + "Content-Type": "application/json", + }, + body = { + "model_path": GGUF_REPO, + "gguf_variant": GGUF_VARIANT, + "is_lora": False, + "max_seq_length": 2048, + }, + timeout_ms = LOAD_FETCH_TIMEOUT_MS, + ) + if load_resp.get("error"): + fail(f"/api/inference/load wedged: {load_resp['error']!r}") + sys.exit(1) if load_resp["status"] != 200: fail(f"/api/inference/load -> {load_resp['status']}: {load_resp.get('body')!r}") sys.exit(1) - info(f"loaded model: {load_resp['body'].get('display_name')}") + info(f"loaded model: {(load_resp['body'] or {}).get('display_name')}") page.reload() composer = page.locator('textarea[aria-label="Message input"]') composer.wait_for(state = "visible", timeout = 60_000) # Detect chat-only mode: /api/health.chat_only is the source of truth. # In chat-only mode, /studio + /export redirect to /chat. - health = page.evaluate(f"""async () => {{ - const r = await fetch("{BASE}/api/health"); - return await r.json(); - }}""") + health_resp = evaluate_fetch( + page, + f"{BASE}/api/health", + timeout_ms = FETCH_TIMEOUT_MS, + ) + if health_resp.get("error"): + fail(f"/api/health wedged: {health_resp['error']!r}") + sys.exit(1) + health = health_resp.get("body") or {} chat_only = bool(health.get("chat_only")) info(f"chat_only mode: {chat_only}") @@ -588,4 +606,5 @@ with sync_playwright() as p: info(f" - {m}") sys.exit(1) info("PASS extra UI flow") + _watchdog.cancel() browser.close()