From 9c305eaac2dc2f7002cc52a39cd55430edafcb32 Mon Sep 17 00:00:00 2001 From: rgarcia <72655+rgarcia@users.noreply.github.com> Date: Tue, 27 Oct 2026 13:34:18 +0000 Subject: [PATCH] Exit nonzero on failed playwright and process execution, accept headless memory sizes --- cmd/browser_pools.go | 4 +- cmd/browsers.go | 43 +++++++++++++------ cmd/browsers_playwright.go | 42 +++++++++++++++---- cmd/browsers_playwright_test.go | 74 +++++++++++++++++++++------------ cmd/browsers_test.go | 27 +++++++++++- 5 files changed, 138 insertions(+), 52 deletions(-) diff --git a/cmd/browser_pools.go b/cmd/browser_pools.go index e8881004..32167862 100644 --- a/cmd/browser_pools.go +++ b/cmd/browser_pools.go @@ -869,7 +869,7 @@ func init() { browserPoolsCreateCmd.Flags().Bool("stealth", false, "Enable stealth mode") browserPoolsCreateCmd.Flags().Bool("headless", false, "Enable headless mode") browserPoolsCreateCmd.Flags().Bool("kiosk", false, "Enable kiosk mode") - browserPoolsCreateCmd.Flags().String("memory", "", "Memory for headful browsers in the pool: '8GiB' (default) or '16GiB'") + browserPoolsCreateCmd.Flags().String("memory", "", "Memory for browsers in the pool. Headless: '1GiB' (default), '2GiB' or '4GiB'. Headful: '8GiB' (default) or '16GiB'") browserPoolsCreateCmd.Flags().Bool("refresh-on-profile-update", false, "Flush idle browsers when the pool's profile is updated") browserPoolsCreateCmd.Flags().String("profile-id", "", "Profile ID") browserPoolsCreateCmd.Flags().String("profile-name", "", "Profile name") @@ -896,7 +896,7 @@ func init() { browserPoolsUpdateCmd.Flags().Bool("stealth", false, "Enable stealth mode") browserPoolsUpdateCmd.Flags().Bool("headless", false, "Enable headless mode") browserPoolsUpdateCmd.Flags().Bool("kiosk", false, "Enable kiosk mode") - browserPoolsUpdateCmd.Flags().String("memory", "", "Memory for newly-warmed headful browsers in the pool: '8GiB' or '16GiB'. Existing browsers keep their allocation; use --discard-all-idle to replace idle browsers") + browserPoolsUpdateCmd.Flags().String("memory", "", "Memory for newly-warmed browsers in the pool. Headless: '1GiB', '2GiB' or '4GiB'. Headful: '8GiB' or '16GiB'. Existing browsers keep their allocation; use --discard-all-idle to replace idle browsers") browserPoolsUpdateCmd.Flags().Bool("refresh-on-profile-update", false, "Flush idle browsers when the pool's profile is updated") browserPoolsUpdateCmd.Flags().String("profile-id", "", "Profile ID") browserPoolsUpdateCmd.Flags().String("profile-name", "", "Profile name") diff --git a/cmd/browsers.go b/cmd/browsers.go index 97da389f..8aa800e0 100644 --- a/cmd/browsers.go +++ b/cmd/browsers.go @@ -180,15 +180,15 @@ func parseRegionFlag(region string) (string, error) { return "", fmt.Errorf("invalid --region value: %s (must be one of %s)", region, strings.Join(availableRegions(), ", ")) } -// availableMemorySizes returns the memory sizes the API accepts when creating a -// browser session. Only headful, non-GPU sessions can request memory; every -// other configuration gets a fixed allocation. +// availableMemorySizes returns the memory sizes the API accepts for non-GPU +// browsers: 1GiB, 2GiB or 4GiB when headless, 8GiB or 16GiB when headful. The +// API rejects a size that does not match the browser's mode. func availableMemorySizes() []string { - return []string{"8GiB", "16GiB"} + return []string{"1GiB", "2GiB", "4GiB", "8GiB", "16GiB"} } // parseMemoryFlag validates a --memory value. An empty value means the flag was -// not set, which lets the API apply its default (8GiB). +// not set, which lets the API apply its default (1GiB headless, 8GiB headful). func parseMemoryFlag(memory string) (kernel.BrowserMemoryRequest, error) { if memory == "" { return "", nil @@ -2137,10 +2137,6 @@ func (b BrowsersCmd) ProcessExec(ctx context.Context, in BrowsersProcessExecInpu pterm.Error.Println("process service not available") return nil } - br, err := b.browsers.Get(ctx, in.Identifier, kernel.BrowserGetParams{}) - if err != nil { - return util.CleanedUpSdkError{Err: err} - } params := kernel.BrowserProcessExecParams{Command: in.Command} if len(in.Args) > 0 { params.Args = in.Args @@ -2164,13 +2160,16 @@ func (b BrowsersCmd) ProcessExec(ctx context.Context, in BrowsersProcessExecInpu if len(env) > 0 { params.Env = env } - res, err := b.process.Exec(ctx, br.SessionID, params) + res, err := b.process.Exec(ctx, in.Identifier, params) if err != nil { return util.CleanedUpSdkError{Err: err} } if in.Output == "json" { - return util.PrintPrettyJSON(res) + if err := util.PrintPrettyJSON(res); err != nil { + return err + } + return processExitError(res.ExitCode) } rows := pterm.TableData{{"Property", "Value"}, {"Exit Code", fmt.Sprintf("%d", res.ExitCode)}, {"Duration (ms)", fmt.Sprintf("%d", res.DurationMs)}} @@ -2199,7 +2198,16 @@ func (b BrowsersCmd) ProcessExec(ctx context.Context, in BrowsersProcessExecInpu } } } - return nil + return processExitError(res.ExitCode) +} + +// processExitError exits nonzero when the remote process did. The exit code has +// already been printed with the rest of the result. +func processExitError(exitCode int64) error { + if exitCode == 0 { + return nil + } + return silentExitError{fmt.Errorf("process exited with code %d", exitCode)} } func (b BrowsersCmd) ProcessSpawn(ctx context.Context, in BrowsersProcessSpawnInput) error { @@ -3365,7 +3373,7 @@ unrestricted code execution inside the browser VM and is not sandboxed.`, browsersCreateCmd.Flags().BoolP("stealth", "s", false, "Launch browser in stealth mode to avoid detection") browsersCreateCmd.Flags().BoolP("headless", "H", false, "Launch browser without GUI access") browsersCreateCmd.Flags().Bool("gpu", false, "Launch browser with hardware-accelerated GPU rendering") - browsersCreateCmd.Flags().String("memory", "", "Memory for a headful, non-GPU browser session: '8GiB' (default) or '16GiB'") + browsersCreateCmd.Flags().String("memory", "", "Memory for a non-GPU browser session. Headless: '1GiB' (default), '2GiB' or '4GiB'. Headful: '8GiB' (default) or '16GiB'") browsersCreateCmd.Flags().String("video-memory", "", "Video memory (VRAM) for a GPU browser session (requires --gpu): '2GiB' (default, 4 vCPU/6GiB memory) or '4GiB' (8 vCPU/12GiB memory)") browsersCreateCmd.Flags().String("invocation-id", "", "Associate the browser session with an invocation") browsersCreateCmd.Flags().Bool("kiosk", false, "Launch browser in kiosk mode") @@ -4364,6 +4372,15 @@ type BrowsersCurlInput struct { Silent bool } +// silentExitError gives a nonzero exit status for a failure whose result has +// already been printed. +type silentExitError struct { + err error +} + +func (e silentExitError) Error() string { return e.err.Error() } +func (e silentExitError) Silent() bool { return true } + type silentCurlError struct { err error } diff --git a/cmd/browsers_playwright.go b/cmd/browsers_playwright.go index 1911bc91..fa016b65 100644 --- a/cmd/browsers_playwright.go +++ b/cmd/browsers_playwright.go @@ -75,10 +75,6 @@ func (b BrowsersCmd) PlaywrightExecute(ctx context.Context, in BrowsersPlaywrigh pterm.Error.Println("playwright service not available") return nil } - br, err := b.browsers.Get(ctx, in.Identifier, kernel.BrowserGetParams{}) - if err != nil { - return util.CleanedUpSdkError{Err: err} - } params := kernel.BrowserPlaywrightExecuteParams{Code: in.Code} if in.Executor != "" { params.Executor = kernel.Opt(in.Executor) @@ -86,13 +82,19 @@ func (b BrowsersCmd) PlaywrightExecute(ctx context.Context, in BrowsersPlaywrigh if in.Timeout > 0 { params.TimeoutSec = kernel.Opt(in.Timeout) } - res, err := b.playwright.Execute(ctx, br.SessionID, params) + res, err := b.playwright.Execute(ctx, in.Identifier, params) if err != nil { return playwrightExecuteError(err, in) } if in.Output == "json" { - return util.PrintPrettyJSON(res) + if err := util.PrintPrettyJSON(res); err != nil { + return err + } + if !res.Success { + return silentExitError{errors.New("playwright execution failed")} + } + return nil } rows := pterm.TableData{{"Property", "Value"}, {"Success", fmt.Sprintf("%t", res.Success)}} @@ -105,6 +107,19 @@ func (b BrowsersCmd) PlaywrightExecute(ctx context.Context, in BrowsersPlaywrigh pterm.Info.Println("stdout:") fmt.Println(res.Stdout) } + if !res.Success { + msg := res.Error + if msg == "" { + msg = "playwright execution failed" + } + // Printed here rather than returned: the root error handler would + // reflow the multi-line call log Playwright errors carry. + pterm.Error.WithWriter(os.Stderr).Println(msg) + if frames := stackFrames(res.Stderr); frames != "" { + fmt.Fprintln(os.Stderr, frames) + } + return silentExitError{errors.New(msg)} + } if res.Stderr != "" { pterm.Info.Println("stderr:") fmt.Fprintln(os.Stderr, res.Stderr) @@ -116,12 +131,21 @@ func (b BrowsersCmd) PlaywrightExecute(ctx context.Context, in BrowsersPlaywrigh fmt.Println(string(bs)) } } - if !res.Success && res.Error != "" { - pterm.Error.Printf("error: %s\n", res.Error) - } return nil } +// stackFrames returns the "at" frames of a JavaScript stack trace, dropping the +// leading message lines that repeat the error. +func stackFrames(stack string) string { + lines := strings.Split(stack, "\n") + for i, line := range lines { + if strings.HasPrefix(strings.TrimSpace(line), "at ") { + return strings.Join(lines[i:], "\n") + } + } + return "" +} + // playwrightExecuteError points at the executors commands when a named // executor call is rejected with 409 because the browser is at its executor limit. func playwrightExecuteError(err error, in BrowsersPlaywrightExecuteInput) error { diff --git a/cmd/browsers_playwright_test.go b/cmd/browsers_playwright_test.go index 4efedd98..a8f2a671 100644 --- a/cmd/browsers_playwright_test.go +++ b/cmd/browsers_playwright_test.go @@ -86,9 +86,7 @@ func TestPlaywrightExecuteExecutorParam(t *testing.T) { calls++ w.Header().Set("Content-Type", "application/json") switch { - case r.Method == http.MethodGet && r.URL.Path == "/browsers/my-browser": - fmt.Fprint(w, `{"session_id":"session123"}`) - case r.Method == http.MethodPost && r.URL.Path == "/browsers/session123/playwright/execute": + case r.Method == http.MethodPost && r.URL.Path == "/browsers/my-browser/playwright/execute": require.NoError(t, json.NewDecoder(r.Body).Decode(&body)) fmt.Fprint(w, `{"success":true,"result":{"title":"Example"},"tab":{"target_id":"ABCDEF0123456789","created":true}}`) default: @@ -96,7 +94,7 @@ func TestPlaywrightExecuteExecutorParam(t *testing.T) { } }, append([]string{"execute", "my-browser", "return await page.title()", "--timeout", "30"}, tc.flags...)...) require.NoError(t, err) - assert.Equal(t, 2, calls) + assert.Equal(t, 1, calls) assert.Equal(t, "return await page.title()", body.Code) if tc.executor == "" { assert.Nil(t, body.Executor) @@ -119,11 +117,9 @@ func TestPlaywrightExecuteOutput(t *testing.T) { name string response string json bool - wantTab bool }{ - {"without tab", `{"success":false,"error":"boom"}`, false, false}, - {"with tab", `{"success":true,"tab":{"target_id":"ABCDEF0123456789","created":false}}`, false, true}, - {"json", `{"success":true,"result":42,"tab":{"target_id":"ABCDEF0123456789","created":true}}`, true, false}, + {"with tab", `{"success":true,"tab":{"target_id":"ABCDEF0123456789","created":false}}`, false}, + {"json", `{"success":true,"result":42,"tab":{"target_id":"ABCDEF0123456789","created":true}}`, true}, } { t.Run(tc.name, func(t *testing.T) { args := []string{"execute", "session123", "return 1"} @@ -132,10 +128,6 @@ func TestPlaywrightExecuteOutput(t *testing.T) { } stdout, table, err := executePlaywrightCommand(t, func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") - if r.Method == http.MethodGet { - fmt.Fprint(w, `{"session_id":"session123"}`) - return - } fmt.Fprint(w, tc.response) }, args...) require.NoError(t, err) @@ -144,25 +136,57 @@ func TestPlaywrightExecuteOutput(t *testing.T) { assert.Empty(t, table) return } - if tc.wantTab { - assert.Contains(t, table, "Tab Target ID") - assert.Contains(t, table, "ABCDEF0123456789") - assert.Contains(t, table, "Tab Created") - } else { - assert.NotContains(t, table, "Tab Target ID") - assert.Contains(t, table, "boom") + assert.Contains(t, table, "Tab Target ID") + assert.Contains(t, table, "ABCDEF0123456789") + assert.Contains(t, table, "Tab Created") + }) + } +} + +func TestPlaywrightExecuteFailureExitsNonzero(t *testing.T) { + for _, tc := range []struct { + name string + response string + json bool + wantErr string + }{ + {"table", `{"success":false,"error":"boom"}`, false, "boom"}, + {"table without message", `{"success":false}`, false, "playwright execution failed"}, + {"json", `{"success":false,"error":"boom"}`, true, "playwright execution failed"}, + } { + t.Run(tc.name, func(t *testing.T) { + args := []string{"execute", "session123", "return 1"} + if tc.json { + args = append(args, "-o", "json") } + stdout, table, err := executePlaywrightCommand(t, func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + fmt.Fprint(w, tc.response) + }, args...) + require.EqualError(t, err, tc.wantErr) + // The error is printed by the command, so the root handler must not print it again. + var silent interface{ Silent() bool } + require.ErrorAs(t, err, &silent) + assert.True(t, silent.Silent()) + if tc.json { + assert.JSONEq(t, tc.response, stdout) + return + } + assert.Contains(t, table, "false") }) } } +func TestStackFrames(t *testing.T) { + stack := "page.click: Timeout 2000ms exceeded.\nCall log:\n - waiting for locator('#x')\n\n at eval (:5:14)\n at async executeCode (daemon.js:409:20)" + assert.Equal(t, " at eval (:5:14)\n at async executeCode (daemon.js:409:20)", stackFrames(stack)) + assert.Equal(t, "", stackFrames("Error: boom")) + assert.Equal(t, "", stackFrames("")) +} + func TestPlaywrightExecuteExecutorLimit(t *testing.T) { _, _, err := executePlaywrightCommand(t, func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") - if r.Method == http.MethodGet { - fmt.Fprint(w, `{"session_id":"session123"}`) - return - } w.WriteHeader(http.StatusConflict) fmt.Fprint(w, `{"message":"Browser already has 8 named Playwright executors.",`+ `"executors":[{"name":"default","busy":false,"created_at":"2026-01-02T03:04:05Z","last_used_at":"2026-01-02T03:04:05Z"}]}`) @@ -190,10 +214,6 @@ func TestPlaywrightExecuteOtherErrors(t *testing.T) { t.Run(tc.name, func(t *testing.T) { _, _, err := executePlaywrightCommand(t, func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") - if r.Method == http.MethodGet { - fmt.Fprint(w, `{"session_id":"session123"}`) - return - } w.WriteHeader(tc.status) fmt.Fprint(w, tc.body) }, append([]string{"execute", "session123", "return 1"}, tc.flags...)...) diff --git a/cmd/browsers_test.go b/cmd/browsers_test.go index f3f3ef4d..c6b4fa64 100644 --- a/cmd/browsers_test.go +++ b/cmd/browsers_test.go @@ -951,8 +951,13 @@ func TestBrowsersCreate_WithMemory(t *testing.T) { require.NoError(t, err) assert.NotContains(t, string(raw), "memory") + // Headless sizes are accepted. + err = b.Create(context.Background(), BrowsersCreateInput{Headless: BoolFlag{Set: true, Value: true}, Memory: "2GiB"}) + require.NoError(t, err) + assert.Equal(t, kernel.BrowserMemoryRequest2GiB, captured.Memory) + // An unsupported size is rejected before the request is made. - assert.Error(t, b.Create(context.Background(), BrowsersCreateInput{Memory: "4GiB"})) + assert.Error(t, b.Create(context.Background(), BrowsersCreateInput{Memory: "3GiB"})) } func TestBrowsersCreate_WithVideoMemory(t *testing.T) { @@ -1986,6 +1991,26 @@ func TestBrowsersProcessExec_MapsEnv(t *testing.T) { assert.NoError(t, err) } +func TestBrowsersProcessExec_NonzeroExit(t *testing.T) { + for _, output := range []string{"", "json"} { + t.Run("output="+output, func(t *testing.T) { + setupStdoutCapture(t) + fake := &FakeProcessService{ + ExecFunc: func(ctx context.Context, id string, body kernel.BrowserProcessExecParams, opts ...option.RequestOption) (*kernel.BrowserProcessExecResponse, error) { + assert.Equal(t, "my-browser", id) + return &kernel.BrowserProcessExecResponse{ExitCode: 3}, nil + }, + } + b := BrowsersCmd{process: fake} + err := b.ProcessExec(context.Background(), BrowsersProcessExecInput{Identifier: "my-browser", Command: "false", Output: output}) + require.EqualError(t, err, "process exited with code 3") + var silent interface{ Silent() bool } + require.ErrorAs(t, err, &silent) + assert.True(t, silent.Silent()) + }) + } +} + func TestBrowsersProcessSpawn_PrintsInfo(t *testing.T) { setupStdoutCapture(t) fake := &FakeProcessService{}