diff --git a/server/e2e/e2e_display_resize_window_test.go b/server/e2e/e2e_display_resize_window_test.go index 527e1202..2162e42e 100644 --- a/server/e2e/e2e_display_resize_window_test.go +++ b/server/e2e/e2e_display_resize_window_test.go @@ -12,6 +12,7 @@ import ( "time" instanceoapi "github.com/kernel/kernel-images/server/lib/oapi" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -457,7 +458,6 @@ func TestDisplayResizeChromiumWindow(t *testing.T) { } } - // navigateBlank points the active page at about:blank via playwright so the // renderer is alive before we query window dimensions. func navigateBlank(t *testing.T, ctx context.Context, c *TestContainer) { @@ -465,13 +465,15 @@ func navigateBlank(t *testing.T, ctx context.Context, c *TestContainer) { client, err := c.APIClient() require.NoError(t, err) timeout := 5 - rsp, err := client.ExecutePlaywrightCodeWithResponse(ctx, instanceoapi.ExecutePlaywrightRequest{ - Code: `await page.goto('about:blank'); return true;`, - TimeoutSec: &timeout, - }) - require.NoError(t, err) - require.NotNil(t, rsp.JSON200, "playwright navigate response missing") - require.True(t, rsp.JSON200.Success, "playwright navigate to about:blank failed: %s", string(rsp.Body)) + require.EventuallyWithT(t, func(collect *assert.CollectT) { + rsp, err := client.ExecutePlaywrightCodeWithResponse(ctx, instanceoapi.ExecutePlaywrightRequest{ + Code: `await page.goto('about:blank'); return true;`, + TimeoutSec: &timeout, + }) + require.NoError(collect, err) + require.NotNil(collect, rsp.JSON200, "missing JSON200 status=%s body=%s", rsp.Status(), string(rsp.Body)) + require.True(collect, rsp.JSON200.Success, "playwright navigate to about:blank failed: %s", string(rsp.Body)) + }, 30*time.Second, 500*time.Millisecond) } // patchDisplayExpectingOK issues PATCH /display and requires a 200. The diff --git a/server/e2e/e2e_playwright_test.go b/server/e2e/e2e_playwright_test.go index 14ee03de..03999fb0 100644 --- a/server/e2e/e2e_playwright_test.go +++ b/server/e2e/e2e_playwright_test.go @@ -6,6 +6,7 @@ import ( "fmt" "net/http" "os/exec" + "strings" "testing" "time" @@ -80,6 +81,67 @@ func TestPlaywrightExecuteAPI(t *testing.T) { t.Log("playwright execute API test passed") } +func TestPlaywrightExecuteTimeoutReturnsPromptlyAndRecovers(t *testing.T) { + t.Parallel() + + if _, err := exec.LookPath("docker"); err != nil { + t.Skipf("docker not available: %v", err) + } + + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) + defer cancel() + + c := NewTestContainer(t, headlessImage) + require.NoError(t, c.Start(ctx, ContainerConfig{}), "failed to start container") + defer c.Stop(ctx) + + require.NoError(t, c.WaitReady(ctx), "api not ready") + + client, err := c.APIClient() + require.NoError(t, err) + + timeoutSec := 2 + timeoutReq := instanceoapi.ExecutePlaywrightCodeJSONRequestBody{ + Code: `await new Promise(r => setTimeout(r, 30000));`, + TimeoutSec: &timeoutSec, + } + + t.Log("executing playwright code expected to exceed timeout") + start := time.Now() + timeoutRsp, err := client.ExecutePlaywrightCodeWithResponse(ctx, timeoutReq) + elapsed := time.Since(start) + + require.NoError(t, err, "playwright timeout request should return an API response") + require.Less(t, elapsed, 5*time.Second, "timeout response should arrive before the API socket read deadline") + require.Equal(t, http.StatusOK, timeoutRsp.StatusCode(), "unexpected status for timed-out playwright execute: %s body=%s", timeoutRsp.Status(), string(timeoutRsp.Body)) + require.NotNil(t, timeoutRsp.JSON200, "expected JSON200 timeout response, got nil") + require.False(t, timeoutRsp.JSON200.Success, "expected success=false for timed-out playwright execution") + require.NotNil(t, timeoutRsp.JSON200.Error, "expected timeout error message") + + errorMsg := strings.ToLower(*timeoutRsp.JSON200.Error) + require.True(t, strings.Contains(errorMsg, "timeout") || strings.Contains(errorMsg, "timed out"), "expected timeout error, got %q", *timeoutRsp.JSON200.Error) + require.NotContains(t, errorMsg, "i/o timeout", "daemon should return a timeout response before the API socket read deadline") + + recoveryReq := instanceoapi.ExecutePlaywrightCodeJSONRequestBody{ + Code: `return await page.evaluate(() => navigator.userAgent);`, + } + + t.Log("executing normal playwright code after timed-out request") + recoveryRsp, err := client.ExecutePlaywrightCodeWithResponse(ctx, recoveryReq) + require.NoError(t, err, "playwright recovery request error: %v", err) + require.Equal(t, http.StatusOK, recoveryRsp.StatusCode(), "unexpected status for playwright recovery execute: %s body=%s", recoveryRsp.Status(), string(recoveryRsp.Body)) + require.NotNil(t, recoveryRsp.JSON200, "expected JSON200 recovery response, got nil") + require.True(t, recoveryRsp.JSON200.Success, "expected recovery request success=true, got error: %s", func() string { + if recoveryRsp.JSON200.Error != nil { + return *recoveryRsp.JSON200.Error + } + return "nil" + }()) + require.NotNil(t, recoveryRsp.JSON200.Result, "expected recovery result to be non-nil") + + t.Log("playwright timeout regression test passed") +} + // TestPlaywrightDaemonRecovery tests that the playwright daemon recovers after chromium is restarted. // The daemon maintains a warm CDP connection, but when chromium restarts, that connection breaks. // The daemon should detect the disconnection and reconnect on the next request. diff --git a/server/runtime/playwright-daemon.ts b/server/runtime/playwright-daemon.ts index ed1e6ed5..6ce6c61d 100644 --- a/server/runtime/playwright-daemon.ts +++ b/server/runtime/playwright-daemon.ts @@ -39,6 +39,19 @@ interface ExecuteResponse { stack?: string; } +function withTimeout(promise: Promise, signal: AbortSignal): Promise { + if (signal.aborted) { + return Promise.reject(signal.reason); + } + + return Promise.race([ + promise, + new Promise((_, reject) => { + signal.addEventListener('abort', () => reject(signal.reason), { once: true }); + }), + ]); +} + async function transformCode(code: string): Promise { // Wrap in async function so top-level await/return are valid for esbuild const wrapped = `async function __userCode__() {\n${code}\n}`; @@ -88,23 +101,26 @@ async function ensureBrowserConnection(): Promise { } console.error(`[playwright-daemon] Connecting to CDP: ${CDP_ENDPOINT}`); - browser = await chromium.connectOverCDP(CDP_ENDPOINT); + const connectedBrowser = await chromium.connectOverCDP(CDP_ENDPOINT); + browser = connectedBrowser; reconnectAttempts = 0; - browser.on('disconnected', () => { + connectedBrowser.on('disconnected', () => { console.error('[playwright-daemon] Browser disconnected'); - browser = null; + if (browser === connectedBrowser) { + browser = null; + } }); console.error('[playwright-daemon] CDP connection established'); - return browser; + return connectedBrowser; } finally { connecting = false; } } -async function executeCode(request: ExecuteRequest): Promise { - const { id, code, timeout_ms = 60000 } = request; +async function executeCode(request: ExecuteRequest, signal: AbortSignal): Promise { + const { id, code } = request; try { let jsCode: string; @@ -118,7 +134,6 @@ async function executeCode(request: ExecuteRequest): Promise { stack: transformError.stack, }; } - let browserInstance: Browser; try { browserInstance = await ensureBrowserConnection(); @@ -142,7 +157,6 @@ async function executeCode(request: ExecuteRequest): Promise { }; } } - const contexts = browserInstance.contexts(); const context = contexts.length > 0 ? contexts[0] : await browserInstance.newContext(); const pages = context.pages(); @@ -150,15 +164,9 @@ async function executeCode(request: ExecuteRequest): Promise { const AsyncFunction = Object.getPrototypeOf(async function () {}).constructor; const userFunction = new AsyncFunction('page', 'context', 'browser', jsCode); + signal.throwIfAborted(); - const timeoutPromise = new Promise((_, reject) => { - setTimeout(() => reject(new Error(`Execution timed out after ${timeout_ms}ms`)), timeout_ms); - }); - - const result = await Promise.race([ - userFunction(page, context, browserInstance), - timeoutPromise, - ]); + const result = await userFunction(page, context, browserInstance); return { id, @@ -201,7 +209,18 @@ function handleConnection(socket: Socket): void { continue; } - const response = await executeCode(request); + const signal = AbortSignal.timeout(request.timeout_ms ?? 60000); + let response: ExecuteResponse; + try { + response = await withTimeout(executeCode(request, signal), signal); + } catch (error: any) { + response = { + id: request.id, + success: false, + error: error.message, + stack: error.stack, + }; + } socket.write(JSON.stringify(response) + '\n'); } });