fix(windows): chrome process idle no rendering

browser_e2e_test.go

@@ -0,0 +1,48 @@
+package main
+
+import (
+	"context"
+	"os"
+	"testing"
+	"time"
+
+	"github.com/go-rod/rod/lib/proto"
+)
+
+// TestStartBrowser launches a real browser through startBrowser, connects to
+// it, opens a page, and shuts everything down again.
+//
+// It requires a browser; if none is installed on the machine one is
+// downloaded, which is why it is opt-in: set VHS_TEST_BROWSER=1 to run it.
+func TestStartBrowser(t *testing.T) {
+	if os.Getenv("VHS_TEST_BROWSER") == "" {
+		t.Skip("set VHS_TEST_BROWSER=1 to run this test")
+	}
+
+	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute)
+	defer cancel()
+
+	browser, closer, err := startBrowser(ctx)
+	if err != nil {
+		t.Fatalf("startBrowser failed: %v", err)
+	}
+
+	page, err := browser.Page(proto.TargetCreateTarget{URL: "about:blank"})
+	if err != nil {
+		t.Errorf("could not open page: %v", err)
+	}
+	if err := page.WaitLoad(); err != nil {
+		t.Errorf("page did not load: %v", err)
+	}
+
+	res, err := page.Eval("() => 1 + 1")
+	if err != nil {
+		t.Errorf("could not evaluate: %v", err)
+	} else if got := res.Value.Int(); got != 2 {
+		t.Errorf("expected 2, got %d", got)
+	}
+
+	if err := closer(); err != nil {
+		t.Errorf("closer failed: %v", err)
+	}
+}

browser_test.go

@@ -0,0 +1,119 @@
+package main
+
+import (
+	"context"
+	"net"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+	"time"
+)
+
+func TestProbeDevTools(t *testing.T) {
+	t.Run("ready endpoint", func(t *testing.T) {
+		ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+			w.WriteHeader(http.StatusOK)
+		}))
+		defer ts.Close()
+
+		_, port, err := net.SplitHostPort(ts.Listener.Addr().String())
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		if err := probeDevTools(mustAtoi(t, port)); err != nil {
+			t.Errorf("expected the endpoint to be ready, got: %v", err)
+		}
+	})
+
+	t.Run("error status code", func(t *testing.T) {
+		ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+			w.WriteHeader(http.StatusNotFound)
+		}))
+		defer ts.Close()
+
+		_, port, err := net.SplitHostPort(ts.Listener.Addr().String())
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		if err := probeDevTools(mustAtoi(t, port)); err == nil {
+			t.Error("expected an error for a not ready endpoint")
+		}
+	})
+
+	t.Run("nothing listening", func(t *testing.T) {
+		addr, err := net.Listen("tcp", "127.0.0.1:0") //nolint:gosec
+		if err != nil {
+			t.Fatal(err)
+		}
+		port := addr.Addr().(*net.TCPAddr).Port
+		if err := addr.Close(); err != nil {
+			t.Fatal(err)
+		}
+
+		if err := probeDevTools(port); err == nil {
+			t.Error("expected an error when nothing is listening")
+		}
+	})
+}
+
+func TestWaitForDevTools(t *testing.T) {
+	t.Run("becomes ready", func(t *testing.T) {
+		ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+			w.WriteHeader(http.StatusOK)
+		}))
+		defer ts.Close()
+
+		_, port, err := net.SplitHostPort(ts.Listener.Addr().String())
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		if !waitForDevTools(context.Background(), mustAtoi(t, port), nil, time.Second) {
+			t.Error("expected the endpoint to become ready")
+		}
+	})
+
+	t.Run("times out when nothing is listening", func(t *testing.T) {
+		addr, err := net.Listen("tcp", "127.0.0.1:0") //nolint:gosec
+		if err != nil {
+			t.Fatal(err)
+		}
+		port := addr.Addr().(*net.TCPAddr).Port
+		if err := addr.Close(); err != nil {
+			t.Fatal(err)
+		}
+
+		if waitForDevTools(context.Background(), port, nil, 250*time.Millisecond) {
+			t.Error("expected waitForDevTools to give up")
+		}
+	})
+
+	t.Run("gives up when the browser exits", func(t *testing.T) {
+		exited := make(chan struct{})
+		close(exited)
+
+		if waitForDevTools(context.Background(), 1, exited, time.Second) {
+			t.Error("expected waitForDevTools to give up on exit")
+		}
+	})
+
+	t.Run("respects the context", func(t *testing.T) {
+		ctx, cancel := context.WithCancel(context.Background())
+		cancel()
+
+		if waitForDevTools(ctx, 1, nil, time.Second) {
+			t.Error("expected waitForDevTools to give up on a canceled context")
+		}
+	})
+}
+
+func mustAtoi(tb testing.TB, s string) int {
+	tb.Helper()
+	port, err := net.LookupPort("tcp", s)
+	if err != nil {
+		tb.Fatal(err)
+	}
+	return port
+}

vhs.go

@@ -6,6 +6,7 @@
 	"fmt"
 	"log"
 	"math"
+	"net/http"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -62,6 +63,18 @@
 	fontsSeparator       = ","
 	defaultCursorBlink   = true
 	defaultWaitTimeout   = 15 * time.Second
+
+	// browserStartTimeout is how long to wait for Chrome to start and expose its
+	// DevTools endpoint before giving up.
+	browserStartTimeout = 30 * time.Second
+
+	// browserPollInterval is how often the DevTools HTTP endpoint is probed
+	// while waiting for the browser to start.
+	browserPollInterval = 100 * time.Millisecond
+
+	// browserCloseTimeout is how long to wait for Chrome to shut down cleanly
+	// before it gets killed.
+	browserCloseTimeout = 5 * time.Second
 )
 
 var defaultWaitPattern = regexp.MustCompile(">$")
@@ -121,6 +134,153 @@
 	}
 }
 
+// startBrowser launches a headless Chrome and returns a connected go-rod
+// browser.
+//
+// It does not rely on the "DevTools listening on ws://..." line that Chrome
+// prints to stderr, as recent Chrome versions do not reliably print it
+// (see https://github.com/charmbracelet/vhs/issues/754), which caused VHS to
+// hang forever waiting for it. Instead, Chrome is started on a reserved
+// port and the DevTools HTTP endpoint is polled until it responds.
+func startBrowser(ctx context.Context) (*rod.Browser, func() error, error) {
+	binPath, _ := launcher.LookPath()
+	if binPath == "" {
+		// No browser found on the system, fall back to downloading one.
+		var err error
+		binPath, err = launcher.NewBrowser().Get()
+		if err != nil {
+			return nil, nil, fmt.Errorf("could not find or download a browser: %w", err)
+		}
+	}
+
+	debugPort := randomPort()
+
+	userDataDir, err := os.MkdirTemp("", "vhs-")
+	if err != nil {
+		return nil, nil, fmt.Errorf("could not create temporary user data directory: %w", err)
+	}
+
+	l := launcher.New().
+		Leakless(false).
+		Bin(binPath).
+		UserDataDir(userDataDir).
+		RemoteDebuggingPort(debugPort).
+		NoSandbox(os.Getenv("VHS_NO_SANDBOX") != "")
+
+	cmd := exec.CommandContext(ctx, binPath, l.FormatArgs()...)
+	if err := cmd.Start(); err != nil {
+		_ = os.RemoveAll(userDataDir)
+		return nil, nil, fmt.Errorf("could not start browser: %w", err)
+	}
+
+	exited := make(chan struct{})
+	go func() {
+		_ = cmd.Wait()
+		close(exited)
+	}()
+
+	if !waitForDevTools(ctx, debugPort, exited, browserStartTimeout) {
+		killBrowser(cmd, userDataDir, exited)
+		if ctx.Err() != nil {
+			return nil, nil, fmt.Errorf("could not start browser: %w", ctx.Err())
+		}
+		select {
+		case <-exited:
+			return nil, nil, errors.New("browser exited unexpectedly before its debugging endpoint was ready")
+		default:
+			return nil, nil, fmt.Errorf("browser debugging endpoint did not become ready within %s", browserStartTimeout)
+		}
+	}
+
+	wsURL, err := launcher.ResolveURL(fmt.Sprintf("127.0.0.1:%d", debugPort))
+	if err != nil {
+		killBrowser(cmd, userDataDir, exited)
+		return nil, nil, fmt.Errorf("could not resolve browser debugging endpoint: %w", err)
+	}
+
+	browser := rod.New().ControlURL(wsURL)
+	if err := browser.Connect(); err != nil {
+		killBrowser(cmd, userDataDir, exited)
+		return nil, nil, fmt.Errorf("could not connect to browser: %w", err)
+	}
+
+	closer := func() error {
+		if err := browser.Close(); err != nil {
+			// The browser could not be closed gracefully (e.g. it is already
+			// gone), make sure the process is terminated.
+			_ = cmd.Process.Kill()
+		}
+		select {
+		case <-exited:
+		case <-time.After(browserCloseTimeout):
+			_ = cmd.Process.Kill()
+		}
+		_ = os.RemoveAll(userDataDir)
+		return nil
+	}
+
+	return browser, closer, nil
+}
+
+// waitForDevTools polls the DevTools HTTP endpoint until it is ready to accept
+// connections. It returns true once the endpoint is ready, and false if the
+// context is canceled, the browser exits first, or the timeout elapses.
+//
+// It is the only reliable way to know the browser is ready, regardless of what
+// the browser prints to stdout or stderr.
+func waitForDevTools(ctx context.Context, port int, exited <-chan struct{}, timeout time.Duration) bool {
+	ticker := time.NewTicker(browserPollInterval)
+	defer ticker.Stop()
+	timeoutCh := time.After(timeout)
+	for {
+		select {
+		case <-ctx.Done():
+			return false
+		case <-exited:
+			return false
+		case <-timeoutCh:
+			return false
+		case <-ticker.C:
+			if probeDevTools(port) == nil {
+				return true
+			}
+		}
+	}
+}
+
+// probeDevTools checks whether the DevTools HTTP endpoint of a browser
+// listening on the given port is ready to accept connections.
+func probeDevTools(port int) error {
+	probeCtx, cancel := context.WithTimeout(context.Background(), time.Second)
+	defer cancel()
+	req, err := http.NewRequestWithContext(probeCtx, http.MethodGet, fmt.Sprintf("http://127.0.0.1:%d/json/version", port), nil)
+	if err != nil {
+		return fmt.Errorf("could not probe browser debugging endpoint: %w", err)
+	}
+	resp, err := http.DefaultClient.Do(req)
+	if err != nil {
+		return fmt.Errorf("could not probe browser debugging endpoint: %w", err)
+	}
+	defer func() { _ = resp.Body.Close() }()
+	if resp.StatusCode != http.StatusOK {
+		return fmt.Errorf("unexpected status code from browser debugging endpoint: %d", resp.StatusCode)
+	}
+	return nil
+}
+
+// killBrowser terminates a browser process and removes its user data
+// directory.
+func killBrowser(cmd *exec.Cmd, userDataDir string, exited <-chan struct{}) {
+	if cmd.Process != nil {
+		_ = cmd.Process.Kill()
+	}
+	select {
+	case <-exited:
+	case <-time.After(browserCloseTimeout):
+	}
+	_ = os.RemoveAll(userDataDir)
+}
+
 // Start starts ttyd, browser and everything else needed to create the gif.
 func (vhs *VHS) Start(ctx context.Context) error {
 	vhs.mutex.Lock()
@@ -136,21 +296,25 @@
 		return fmt.Errorf("could not start tty: %w", err)
 	}
 
-	path, _ := launcher.LookPath()
-	enableNoSandbox := os.Getenv("VHS_NO_SANDBOX") != ""
-	u, err := launcher.New().Leakless(false).Bin(path).NoSandbox(enableNoSandbox).Launch()
+	browser, closeBrowser, err := startBrowser(ctx)
 	if err != nil {
-		return fmt.Errorf("could not launch browser: %w", err)
+		_ = vhs.tty.Process.Kill()
+		return fmt.Errorf("could not start browser: %w", err)
 	}
-	browser := rod.New().ControlURL(u).MustConnect()
 	page, err := browser.Page(proto.TargetCreateTarget{URL: fmt.Sprintf("http://localhost:%d", port)})
 	if err != nil {
+		_ = closeBrowser()
+		_ = vhs.tty.Process.Kill()
 		return fmt.Errorf("could not open ttyd: %w", err)
 	}
 
 	vhs.browser = browser
 	vhs.Page = page
-	vhs.close = vhs.browser.Close
+	vhs.close = func() error {
+		_ = closeBrowser()
+		_ = vhs.tty.Process.Kill()
+		return nil
+	}
 	vhs.started = true
 	return nil
 }
@@ -204,7 +368,10 @@
 	time.Sleep(cleanupWaitTime)
 
 	// Tear down the processes we started.
-	vhs.browser.MustClose()
+	//
+	// The browser may have already exited, in which case closing it fails and
+	// its process gets killed as a fallback, so errors are ignored here.
+	_ = vhs.browser.Close()
 	return vhs.tty.Process.Kill()
 }