fix(chrome): honor the caller context in Launch

Launch and clearState ran against d.tabCtx, so a target that accepts the
connection and never answers wedged the process past its own --duration and
through SIGTERM, needing SIGKILL. Unattended that is a campaign worker lost for
the rest of the sweep with no diagnostic.

The browser is still allocated against d.tabCtx first, because chromedp starts
Chrome under whichever context calls Run first and allocating under a caller
deadline would kill the browser when Launch returns. Everything after
allocation goes through runCtx.

Claude-Session: https://claude.ai/code/session_01A5KmftdEJ49A9z5mF5ESrX
This commit is contained in:
pj committed 2026-08-12 21:38:09 +05:30
1 parent 40cdb455bd
commit 557bd9d815
2 files changed
+96 -8

No files matched your search

+20 -8
View File
@@ -97,19 +97,31 @@ func New() *Driver {
} }
func (d *Driver) Launch(ctx context.Context, bundleID string, clearState bool, _ map[string]string) error { func (d *Driver) Launch(ctx context.Context, bundleID string, clearState bool, _ map[string]string) error {
// Allocate the browser against the driver's own context before anything
// caller-bound runs. chromedp starts Chrome under whichever context first
// calls Run, so allocating under a caller deadline would tie the browser
// process to this one call and kill it the moment Launch returns.
if err := chromedp.Run(d.tabCtx); err != nil {
return err
}
// Everything after allocation goes through runCtx, so a caller deadline or
// a SIGTERM aborts a launch that would otherwise wait forever on a target
// that accepts the connection and never answers.
runCtx, cancel := d.runCtx(ctx)
defer cancel()
if clearState { if clearState {
if err := d.clearState(bundleID); err != nil { if err := d.clearState(runCtx, bundleID); err != nil {
return err return err
} }
} }
if err := chromedp.Run(d.tabCtx, chromedp.Navigate(bundleID)); err != nil { if err := chromedp.Run(runCtx, chromedp.Navigate(bundleID)); err != nil {
return err return err
} }
// After navigation, read CSS custom properties --frame-w / --frame-h (common // After navigation, read CSS custom properties --frame-w / --frame-h (common
// mobile-frame convention) so screenshots fit the app without grey borders. // mobile-frame convention) so screenshots fit the app without grey borders.
// Falls back to the body scroll dimensions if the properties are absent. // Falls back to the body scroll dimensions if the properties are absent.
var dims [2]int64 var dims [2]int64
if err := chromedp.Run(d.tabCtx, chromedp.Evaluate(` if err := chromedp.Run(runCtx, chromedp.Evaluate(`
(function() { (function() {
const s = getComputedStyle(document.documentElement); const s = getComputedStyle(document.documentElement);
const pw = parseInt(s.getPropertyValue('--frame-w'), 10); const pw = parseInt(s.getPropertyValue('--frame-w'), 10);
@@ -118,7 +130,7 @@ func (d *Driver) Launch(ctx context.Context, bundleID string, clearState bool, _
const h = isNaN(ph) ? document.body.scrollHeight : ph; const h = isNaN(ph) ? document.body.scrollHeight : ph;
return [w, h]; return [w, h];
})()`, &dims)); err == nil && dims[0] > 0 && dims[1] > 0 { })()`, &dims)); err == nil && dims[0] > 0 && dims[1] > 0 {
_ = chromedp.Run(d.tabCtx, chromedp.EmulateViewport(dims[0], dims[1])) _ = chromedp.Run(runCtx, chromedp.EmulateViewport(dims[0], dims[1]))
} }
return nil return nil
} }
@@ -130,8 +142,8 @@ func (d *Driver) Launch(ctx context.Context, bundleID string, clearState bool, _
// which needs no navigation. sessionStorage is per-tab and outside that // which needs no navigation. sessionStorage is per-tab and outside that
// domain's reach; it only survives when a relaunch reuses a tab already on // domain's reach; it only survives when a relaunch reuses a tab already on
// the target origin, which is the one case where script can reach it. // the target origin, which is the one case where script can reach it.
func (d *Driver) clearState(bundleID string) error { func (d *Driver) clearState(runCtx context.Context, bundleID string) error {
if err := chromedp.Run(d.tabCtx, network.ClearBrowserCookies()); err != nil { if err := chromedp.Run(runCtx, network.ClearBrowserCookies()); err != nil {
return fmt.Errorf("clear cookies: %w", err) return fmt.Errorf("clear cookies: %w", err)
} }
origin := securityOrigin(bundleID) origin := securityOrigin(bundleID)
@@ -139,12 +151,12 @@ func (d *Driver) clearState(bundleID string) error {
return nil return nil
} }
clearForOrigin := storage.ClearDataForOrigin(origin, string(storage.TypeAll)) clearForOrigin := storage.ClearDataForOrigin(origin, string(storage.TypeAll))
if err := chromedp.Run(d.tabCtx, clearForOrigin); err != nil { if err := chromedp.Run(runCtx, clearForOrigin); err != nil {
return fmt.Errorf("clear storage for %s: %w", origin, err) return fmt.Errorf("clear storage for %s: %w", origin, err)
} }
script := fmt.Sprintf( script := fmt.Sprintf(
`location.origin === %q && (sessionStorage.clear(), true)`, origin) `location.origin === %q && (sessionStorage.clear(), true)`, origin)
return chromedp.Run(d.tabCtx, chromedp.ActionFunc(func(ctx context.Context) error { return chromedp.Run(runCtx, chromedp.ActionFunc(func(ctx context.Context) error {
_, exception, err := runtime.Evaluate(script).Do(ctx) _, exception, err := runtime.Evaluate(script).Do(ctx)
if err != nil { if err != nil {
return fmt.Errorf("clear session storage: %w", err) return fmt.Errorf("clear session storage: %w", err)
+76
View File
@@ -6,6 +6,7 @@ import (
"context" "context"
"encoding/json" "encoding/json"
"errors" "errors"
"net"
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"testing" "testing"
@@ -398,3 +399,78 @@ func TestHierarchy_RootsAtDocumentElementWithoutHead(t *testing.T) {
} }
} }
} }
// TestLaunch_HonorsCallerDeadline points the driver at a listener that accepts
// the connection and never answers. Chrome has no page-load deadline of its
// own, so a Launch that ignored its caller context waited forever: an
// unattended campaign worker aimed at an unreachable target wedged with no
// diagnostic and did not even answer SIGTERM.
func TestLaunch_HonorsCallerDeadline(t *testing.T) {
listener, err := net.Listen("tcp", "127.0.0.1:0")
if err != nil {
t.Fatalf("listen: %v", err)
}
defer listener.Close()
held := make(chan net.Conn, 8)
go func() {
for {
conn, err := listener.Accept()
if err != nil {
return
}
held <- conn
}
}()
defer func() {
close(held)
for conn := range held {
conn.Close()
}
}()
d := New()
defer d.Terminate(context.Background())
ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
defer cancel()
done := make(chan error, 1)
go func() { done <- d.Launch(ctx, "http://"+listener.Addr().String(), false, nil) }()
select {
case err := <-done:
if err == nil {
t.Fatal("Launch returned nil against a target that never answers")
}
if !errors.Is(err, context.DeadlineExceeded) && !errors.Is(err, context.Canceled) {
t.Fatalf("Launch error = %v, want a context error", err)
}
case <-time.After(45 * time.Second):
t.Fatal("Launch ignored the caller deadline and blocked")
}
}
// TestLaunch_KeepsBrowserAliveAfterCallerContextEnds guards the trap that made
// Launch use the driver context in the first place: chromedp starts Chrome
// under whichever context runs first, so allocating under the caller's
// short-lived context would kill the browser as soon as Launch returned.
func TestLaunch_KeepsBrowserAliveAfterCallerContextEnds(t *testing.T) {
d := New()
defer d.Terminate(context.Background())
ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second)
if err := d.Launch(ctx, "data:text/html,<body>alive</body>", false, nil); err != nil {
t.Fatalf("Launch: %v", err)
}
cancel()
var text string
if err := chromedp.Run(d.tabCtx,
chromedp.Evaluate(`document.body.textContent`, &text)); err != nil {
t.Fatalf("browser died with the caller context: %v", err)
}
if text != "alive" {
t.Errorf("body text = %q, want \"alive\"", text)
}
if err := d.Launch(context.Background(), "data:text/html,<body>again</body>", false, nil); err != nil {
t.Fatalf("second Launch after the first caller context ended: %v", err)
}
}