From 62b5bbfc42885abcd4eea040a912fc73080051aa Mon Sep 17 00:00:00 2001 From: Eric Posen <563881+goeric@users.noreply.github.com> Date: Mon, 27 Jul 2026 13:26:22 -0700 Subject: [PATCH] Raise the target before capturing pixels from it "rodney screenshot" fails with "context deadline exceeded" whenever the page it targets is not the browser's foreground one: rodney open rodney screenshot a.png # works rodney newpage rodney page 1 # back to the heavy page rodney screenshot b.png # context deadline exceeded rodney's "active page" is only an index in its own state file. The browser is never told about it, so after "rodney page N" the foreground target is still whichever page was opened last. DOM and JS commands do not care -- they read a hidden target perfectly well, which is why url, js and text all keep reporting the switched-to page correctly -- but a backgrounded target has no compositor producing frames, and Page.captureScreenshot waits for one that never arrives. Page weight is a red herring: a heavy page captures in 0.23s when it is in the foreground, while a backgrounded example.com times out. What decides it is foreground, not size. Element capture is affected too, and worse: rod's stability wait ignores the page timeout, so screenshot-el on a hidden target hangs for over eight minutes rather than 30 seconds. cmdPage now switches the browser as well as the index, and both capture commands raise their target first, so a state file that has drifted out of sync with the browser cannot wedge a capture. Not a regression from the crash fixes: it reproduces identically on the pre-crash-fix binary with --single-process and old headless. Co-Authored-By: Claude Opus 5 (1M context) --- capture_test.go | 103 ++++++++++++++++++++++++++++++++++++++++++++++++ main.go | 26 ++++++++++++ 2 files changed, 129 insertions(+) create mode 100644 capture_test.go diff --git a/capture_test.go b/capture_test.go new file mode 100644 index 0000000..12e69f1 --- /dev/null +++ b/capture_test.go @@ -0,0 +1,103 @@ +package main + +import ( + "testing" + "time" + + "github.com/go-rod/rod" + "github.com/go-rod/rod/lib/proto" +) + +// visibilityState reports what the page thinks its own visibility is, which is +// how Chromium exposes whether a target is the foreground one. +func visibilityState(t *testing.T, p *rod.Page) string { + t.Helper() + res, err := p.Eval("() => document.visibilityState") + if err != nil { + t.Fatalf("reading visibilityState: %v", err) + } + return res.Value.String() +} + +// openTwoPages returns a page that has been pushed into the background by a +// second one, plus that second page. +func openTwoPages(t *testing.T) (background, foreground *rod.Page) { + t.Helper() + background = env.browser.MustPage(env.server.URL) + t.Cleanup(func() { background.MustClose() }) + foreground = env.browser.MustPage(env.server.URL + "/form") + t.Cleanup(func() { foreground.MustClose() }) + + if got := visibilityState(t, background); got != "hidden" { + t.Fatalf("first page visibilityState = %q, want %q -- opening a second page "+ + "is supposed to background the first, so this test proves nothing", got, "hidden") + } + return background, foreground +} + +func TestBringToFront_MakesTargetVisible(t *testing.T) { + background, _ := openTwoPages(t) + + bringToFront(background) + + if got := visibilityState(t, background); got != "visible" { + t.Errorf("visibilityState = %q after bringToFront, want %q", got, "visible") + } +} + +// The regression. A backgrounded target has no compositor producing frames, so +// Page.captureScreenshot waits for one that never arrives and the CLI reports +// "screenshot failed: context deadline exceeded". Without the bringToFront call +// this test hangs until the timeout below and fails. +func TestScreenshot_BackgroundedTargetDoesNotHang(t *testing.T) { + background, _ := openTwoPages(t) + + bringToFront(background) + + data, err := background.Timeout(20*time.Second).Screenshot(true, nil) + if err != nil { + t.Fatalf("full-page screenshot of a backgrounded target: %v", err) + } + if len(data) == 0 { + t.Error("screenshot returned no data") + } +} + +// Element capture goes through rod's visibility/stability wait as well, where a +// hidden target hangs for far longer than the screenshot deadline. +func TestScreenshotEl_BackgroundedTargetDoesNotHang(t *testing.T) { + background, _ := openTwoPages(t) + + bringToFront(background) + + // rod's element capture ignores the page timeout while it waits for the + // element to become stable, and on a hidden target that wait outlives the + // whole test binary. Bound it here so a regression fails this one test + // instead of panicking the suite ten minutes later. + type result struct { + data []byte + err error + } + done := make(chan result, 1) + go func() { + el, err := background.Element("h1") + if err != nil { + done <- result{err: err} + return + } + data, err := el.Screenshot(proto.PageCaptureScreenshotFormatPng, 0) + done <- result{data: data, err: err} + }() + + select { + case r := <-done: + if r.err != nil { + t.Fatalf("element screenshot of a backgrounded target: %v", r.err) + } + if len(r.data) == 0 { + t.Error("element screenshot returned no data") + } + case <-time.After(30 * time.Second): + t.Fatal("element screenshot of a backgrounded target did not finish within 30s") + } +} diff --git a/main.go b/main.go index a5188db..f14019b 100644 --- a/main.go +++ b/main.go @@ -334,6 +334,27 @@ func withPage() (*State, *rod.Browser, *rod.Page) { return s, browser, page } +// bringToFront makes page the browser's foreground target. +// +// rodney's "active page" is only an index in its own state file; the browser is +// never told about it, so after "rodney page N" the foreground target is still +// whichever one was opened last. DOM and JS commands do not care -- they read a +// hidden target perfectly well -- but a backgrounded target has no compositor +// producing frames, and Page.captureScreenshot simply waits for one that never +// arrives until the context deadline expires: +// +// error: screenshot failed: context deadline exceeded +// +// So every command that captures pixels has to raise its target first. +// +// Errors are deliberately ignored. Activation failing means the target or the +// browser is gone, in which case the capture that follows reports the real +// problem; turning that into an error here would only replace a good message +// with a worse one, and would risk failing captures that would have worked. +func bringToFront(page *rod.Page) { + _, _ = page.Activate() +} + // --- Commands --- // parseStartArgs parses the flags for the "start" command. @@ -1150,6 +1171,7 @@ func cmdScreenshot(args []string) { } _, _, page := withPage() + bringToFront(page) // Set viewport size viewportHeight := *height @@ -1184,6 +1206,7 @@ func cmdScreenshotEl(args []string) { file = args[1] } _, _, page := withPage() + bringToFront(page) el, err := page.Element(args[0]) if err != nil { fatal("element not found: %v", err) @@ -1252,6 +1275,9 @@ func cmdPage(args []string) { if err := saveState(s); err != nil { fatal("failed to save state: %v", err) } + // Switch the browser too, not just our own index, so the page a caller just + // selected is the one being rendered. + bringToFront(pages[idx]) info, _ := pages[idx].Info() if info != nil { fmt.Printf("Switched to [%d] %s - %s\n", idx, info.Title, info.URL)