diff --git a/internal/chrome/cdp.go b/internal/chrome/cdp.go index 2daba66..e25145f 100644 --- a/internal/chrome/cdp.go +++ b/internal/chrome/cdp.go @@ -1397,10 +1397,13 @@ func (c *CDP) Wait(ctx context.Context, id string, cond WaitCond) (map[string]an default: return nil, fmt.Errorf("wait needs one of --url, --visible, --gone, --text, --stable, --idle, --for") } - if err := c.run(ctx, id, action); err != nil { + // Also read the URL the tab settled at, so the envelope's target reflects + // where a --url / redirect wait actually landed, not the pre-wait URL. + var loc string + if err := c.run(ctx, id, action, chromedp.Location(&loc)); err != nil { return nil, err } - return map[string]any{"waited": what}, nil + return map[string]any{"waited": what, "url": loc}, nil } // waitURL polls location.href until it contains substr (or the context ends). @@ -1473,37 +1476,58 @@ func waitStable(window time.Duration) chromedp.Action { }) } -// waitIdle returns once network activity has settled: no in-flight requests for -// `window`. It tracks requestWillBeSent vs loadingFinished/Failed — for SPA -// loads (Outlook, Workday) where "DOM loaded" ≠ "content rendered". +// waitIdle returns once network activity has settled. It considers the page +// settled when EITHER no requests are in flight for `window` (the clean path, +// for pages whose requests all complete), OR requests remain open but the +// connection has gone silent for `idleStall` (the stalled path). The stalled +// path is what makes --idle usable on SPAs (Outlook, Workday) that hold a +// websocket / long-poll / EventSource stream open indefinitely: such a request +// fires requestWillBeSent but never loadingFinished, so inflight never returns +// to zero and a strict "inflight == 0" wait would hang until --timeout. Progress +// events (response/data), not just start/finish, keep the clock live, so an +// in-progress download is never mistaken for a silent held-open stream. func waitIdle(window time.Duration) chromedp.Action { + // A still-open request is treated as idle after this much network silence. + // Longer than `window` so a normally-completing load always settles via the + // clean path first; short enough that a held-open stream doesn't wait out + // the whole --timeout. + const idleStall = 2 * time.Second return chromedp.ActionFunc(func(ctx context.Context) error { if err := network.Enable().Do(ctx); err != nil { return err } var mu sync.Mutex inflight := 0 - lastZero := time.Now() + // lastActivity is the last time any request started, progressed, or + // ended — set only for network events (ListenTarget also delivers page/ + // DOM events, which must not count as network activity). + lastActivity := time.Now() chromedp.ListenTarget(ctx, func(ev interface{}) { mu.Lock() defer mu.Unlock() switch ev.(type) { case *network.EventRequestWillBeSent: inflight++ + lastActivity = time.Now() case *network.EventLoadingFinished, *network.EventLoadingFailed: if inflight > 0 { inflight-- } - if inflight == 0 { - lastZero = time.Now() - } + lastActivity = time.Now() + case *network.EventResponseReceived, *network.EventDataReceived: + // bytes moving on an open request — keep it counted as active + lastActivity = time.Now() } }) t := time.NewTicker(100 * time.Millisecond) defer t.Stop() for { mu.Lock() - idle := inflight == 0 && time.Since(lastZero) >= window + threshold := window + if inflight > 0 { + threshold = idleStall // still-open requests need the longer stall window + } + idle := time.Since(lastActivity) >= threshold mu.Unlock() if idle { return nil diff --git a/internal/chrome/wait_idle_test.go b/internal/chrome/wait_idle_test.go index 65d847b..5131333 100644 --- a/internal/chrome/wait_idle_test.go +++ b/internal/chrome/wait_idle_test.go @@ -55,3 +55,55 @@ addEventListener('load', () => setTimeout(() => { t.Errorf("window.__done = %q after wait --idle, want true (idle returned too early, in %v)", got, time.Since(start)) } } + +// wait --idle must still settle when a request is held open indefinitely (a +// websocket / long-poll / EventSource — the shape that made --idle hang on +// Workday): inflight never returns to zero, so it settles via the stalled path +// once the connection goes silent, rather than waiting out --timeout. +func TestWaitIdleStalledStream(t *testing.T) { + if testing.Short() { + t.Skip("skipping live-Chrome integration in -short mode") + } + ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) + defer cancel() + + // Build the server BEFORE launching Chrome so its Close() defer runs LAST + // (LIFO), after b.Close() tears Chrome down. Otherwise srv.Close() blocks on + // the still-open /hang request, which only unblocks when Chrome disconnects + // — a teardown deadlock. /hang stays open until the request context is + // cancelled (Chrome dropping the connection at b.Close()). + mux := http.NewServeMux() + mux.HandleFunc("/hang", func(_ http.ResponseWriter, r *http.Request) { + <-r.Context().Done() + }) + mux.HandleFunc("/", func(w http.ResponseWriter, _ *http.Request) { + fmt.Fprint(w, `Stalled`) + }) + srv := httptest.NewServer(mux) + defer srv.Close() + + b, err := launch(true, tmpProfile(t), 0) + if err != nil { + t.Skipf("cannot launch a managed headless Chrome here: %v", err) + } + defer b.Close() + + id := firstTab(ctx, t, b) + if _, err := b.Navigate(ctx, id, srv.URL); err != nil { + t.Fatalf("Navigate: %v", err) + } + + // Bound the wait well under the 30s+ a held-open request would otherwise + // cost: if the fix works it settles via the stalled path in ~2s. + wctx, wcancel := context.WithTimeout(ctx, 10*time.Second) + defer wcancel() + start := time.Now() + if _, err := b.Wait(wctx, id, WaitCond{Idle: true}); err != nil { + t.Fatalf("Wait --idle hung on a held-open request: %v", err) + } + if elapsed := time.Since(start); elapsed >= 8*time.Second { + t.Errorf("wait --idle took %v — it waited for the held-open request instead of settling on silence", elapsed) + } +} diff --git a/internal/cli/commands.go b/internal/cli/commands.go index 12bcc36..93d3d2f 100644 --- a/internal/cli/commands.go +++ b/internal/cli/commands.go @@ -753,6 +753,13 @@ func (a *App) targetAction(command string, fn func(ctx context.Context, b chrome a.emitErr(command, code, msg, details) return nil } + // If the action settled at a new URL (nav, wait --url / redirect), report + // that in the envelope's target rather than the pre-action URL. + if m, ok := res.(map[string]any); ok { + if u, ok := m["url"].(string); ok && u != "" { + tgt.URL = u + } + } // --wait-text folds "act, then confirm the write landed" into one call: // after the action succeeds, block until the page contains the text. if a.actWaitText != "" { diff --git a/internal/cli/target_url_test.go b/internal/cli/target_url_test.go new file mode 100644 index 0000000..c480a78 --- /dev/null +++ b/internal/cli/target_url_test.go @@ -0,0 +1,56 @@ +package cli + +// Regression: the envelope's target must reflect the URL the tab settled at +// after a nav / redirect wait, not the pre-action URL. + +import ( + "context" + "testing" + + "github.com/sanketsudake/chrome-cdp-cli/internal/chrome" + "github.com/sanketsudake/chrome-cdp-cli/internal/target" +) + +// navWaitBrowser returns a settled URL from nav/wait so the CLI can echo the +// post-action URL in the envelope's target. +type navWaitBrowser struct { + fakeBrowser + settled string +} + +func (n *navWaitBrowser) Navigate(_ context.Context, _, _ string) (map[string]any, error) { + return map[string]any{"url": n.settled}, nil +} +func (n *navWaitBrowser) Wait(_ context.Context, _ string, _ chrome.WaitCond) (map[string]any, error) { + return map[string]any{"waited": "url:after", "url": n.settled}, nil +} + +func TestNavReportsSettledURL(t *testing.T) { + t.Parallel() + b := &navWaitBrowser{ + fakeBrowser: fakeBrowser{tabs: []target.Info{{ID: "aa11", Title: "A", URL: "https://before/"}}}, + settled: "https://after/redirected", + } + env, _, code := run(t, b, "nav", "https://before/", "--target", "aa11", "--json") + if code != 0 { + t.Fatalf("exit = %d, want 0", code) + } + if got := env["target"].(map[string]any)["url"]; got != "https://after/redirected" { + t.Errorf("target.url = %v, want the settled URL (not the pre-nav URL)", got) + } +} + +func TestWaitReportsSettledURL(t *testing.T) { + t.Parallel() + b := &navWaitBrowser{ + fakeBrowser: fakeBrowser{tabs: []target.Info{{ID: "aa11", Title: "A", URL: "https://before/"}}}, + settled: "https://after/home", + } + env, _, code := run(t, b, "wait", "--url", "home", "--target", "aa11", "--json") + if code != 0 { + t.Fatalf("exit = %d, want 0", code) + } + if got := env["target"].(map[string]any)["url"]; got != "https://after/home" { + t.Errorf("target.url = %v, want the settled URL", got) + } +}