Skip to content

Commit 47ab106

Browse files
rgarciargarcia
committed
Address review: single-line executor limit hint, move playwright group to its own file
1 parent 99617b9 commit 47ab106

3 files changed

Lines changed: 154 additions & 168 deletions

File tree

‎cmd/browsers.go‎

Lines changed: 11 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -90,11 +90,6 @@ type BrowserLogService interface {
9090
StreamStreaming(ctx context.Context, idOrName string, query kernel.BrowserLogStreamParams, opts ...option.RequestOption) (stream *ssestream.Stream[shared.LogEvent])
9191
}
9292

93-
// BrowserPlaywrightService defines the subset we use for Playwright execution.
94-
type BrowserPlaywrightService interface {
95-
Execute(ctx context.Context, idOrName string, body kernel.BrowserPlaywrightExecuteParams, opts ...option.RequestOption) (res *kernel.BrowserPlaywrightExecuteResponse, err error)
96-
}
97-
9893
// BrowserComputerService defines the subset we use for OS-level mouse & screen.
9994
type BrowserComputerService interface {
10095
Batch(ctx context.Context, idOrName string, body kernel.BrowserComputerBatchParams, opts ...option.RequestOption) (err error)
@@ -517,17 +512,17 @@ type BrowsersUpdateInput struct {
517512

518513
// BrowsersCmd is a cobra-independent command handler for browsers operations.
519514
type BrowsersCmd struct {
520-
browsers BrowsersService
521-
replays BrowserReplaysService
522-
fs BrowserFSService
523-
fsWatch BrowserFWatchService
524-
process BrowserProcessService
525-
logs BrowserLogService
526-
computer BrowserComputerService
527-
playwright BrowserPlaywrightService
528-
executors BrowserPlaywrightExecutorService
529-
telemetry BrowserTelemetryService
530-
webmcp BrowserWebMCPService
515+
browsers BrowsersService
516+
replays BrowserReplaysService
517+
fs BrowserFSService
518+
fsWatch BrowserFWatchService
519+
process BrowserProcessService
520+
logs BrowserLogService
521+
computer BrowserComputerService
522+
playwright BrowserPlaywrightService
523+
playwrightExecutors BrowserPlaywrightExecutorService
524+
telemetry BrowserTelemetryService
525+
webmcp BrowserWebMCPService
531526
}
532527

533528
type BrowsersListInput struct {
@@ -1837,71 +1832,6 @@ type BrowsersFSWatchEventsInput struct {
18371832
WatchID string
18381833
}
18391834

1840-
// Playwright
1841-
type BrowsersPlaywrightExecuteInput struct {
1842-
Identifier string
1843-
Code string
1844-
Executor string
1845-
Timeout int64
1846-
Output string
1847-
}
1848-
1849-
func (b BrowsersCmd) PlaywrightExecute(ctx context.Context, in BrowsersPlaywrightExecuteInput) error {
1850-
if err := validateJSONOutput(in.Output); err != nil {
1851-
return err
1852-
}
1853-
1854-
if b.playwright == nil {
1855-
pterm.Error.Println("playwright service not available")
1856-
return nil
1857-
}
1858-
br, err := b.browsers.Get(ctx, in.Identifier, kernel.BrowserGetParams{})
1859-
if err != nil {
1860-
return util.CleanedUpSdkError{Err: err}
1861-
}
1862-
params := kernel.BrowserPlaywrightExecuteParams{Code: in.Code}
1863-
if in.Executor != "" {
1864-
params.Executor = kernel.Opt(in.Executor)
1865-
}
1866-
if in.Timeout > 0 {
1867-
params.TimeoutSec = kernel.Opt(in.Timeout)
1868-
}
1869-
res, err := b.playwright.Execute(ctx, br.SessionID, params)
1870-
if err != nil {
1871-
return playwrightExecuteError(err)
1872-
}
1873-
1874-
if in.Output == "json" {
1875-
return util.PrintPrettyJSON(res)
1876-
}
1877-
1878-
rows := pterm.TableData{{"Property", "Value"}, {"Success", fmt.Sprintf("%t", res.Success)}}
1879-
if res.JSON.Tab.Valid() {
1880-
rows = append(rows, []string{"Tab Target ID", res.Tab.TargetID}, []string{"Tab Created", fmt.Sprintf("%t", res.Tab.Created)})
1881-
}
1882-
PrintTableNoPad(rows, true)
1883-
1884-
if res.Stdout != "" {
1885-
pterm.Info.Println("stdout:")
1886-
fmt.Println(res.Stdout)
1887-
}
1888-
if res.Stderr != "" {
1889-
pterm.Info.Println("stderr:")
1890-
fmt.Fprintln(os.Stderr, res.Stderr)
1891-
}
1892-
if res.Result != nil {
1893-
bs, err := json.MarshalIndent(res.Result, "", " ")
1894-
if err == nil {
1895-
pterm.Info.Println("result:")
1896-
fmt.Println(string(bs))
1897-
}
1898-
}
1899-
if !res.Success && res.Error != "" {
1900-
pterm.Error.Printf("error: %s\n", res.Error)
1901-
}
1902-
return nil
1903-
}
1904-
19051835
// REPL
19061836
type BrowsersReplInput struct {
19071837
Identifier string
@@ -3830,34 +3760,6 @@ func runBrowsersFSWatchEvents(cmd *cobra.Command, args []string) error {
38303760
return b.FSWatchEvents(cmd.Context(), BrowsersFSWatchEventsInput{Identifier: args[0], WatchID: args[1]})
38313761
}
38323762

3833-
func runBrowsersPlaywrightExecute(cmd *cobra.Command, args []string) error {
3834-
client := getKernelClient(cmd)
3835-
svc := client.Browsers
3836-
3837-
var code string
3838-
if len(args) >= 2 {
3839-
code = strings.Join(args[1:], " ")
3840-
} else {
3841-
// Read code from stdin
3842-
stat, _ := os.Stdin.Stat()
3843-
if (stat.Mode() & os.ModeCharDevice) != 0 {
3844-
pterm.Error.Println("no code provided. Provide code as an argument or pipe via stdin")
3845-
return nil
3846-
}
3847-
data, err := io.ReadAll(os.Stdin)
3848-
if err != nil {
3849-
pterm.Error.Printf("failed to read stdin: %v\n", err)
3850-
return nil
3851-
}
3852-
code = string(data)
3853-
}
3854-
executor, _ := cmd.Flags().GetString("executor")
3855-
timeout, _ := cmd.Flags().GetInt64("timeout")
3856-
output, _ := cmd.Flags().GetString("output")
3857-
b := BrowsersCmd{browsers: &svc, playwright: &svc.Playwright}
3858-
return b.PlaywrightExecute(cmd.Context(), BrowsersPlaywrightExecuteInput{Identifier: args[0], Code: strings.TrimSpace(code), Executor: executor, Timeout: timeout, Output: output})
3859-
}
3860-
38613763
func runBrowsersRepl(cmd *cobra.Command, args []string) error {
38623764
client := getKernelClient(cmd)
38633765
svc := client.Browsers

cmd/browsers_playwright_executors.go renamed to cmd/browsers_playwright.go

Lines changed: 122 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@ import (
55
"encoding/json"
66
"errors"
77
"fmt"
8+
"io"
89
"net/http"
10+
"os"
911
"strconv"
1012
"strings"
1113

@@ -17,6 +19,11 @@ import (
1719
"github.com/spf13/cobra"
1820
)
1921

22+
// BrowserPlaywrightService defines the subset we use for Playwright execution.
23+
type BrowserPlaywrightService interface {
24+
Execute(ctx context.Context, idOrName string, body kernel.BrowserPlaywrightExecuteParams, opts ...option.RequestOption) (res *kernel.BrowserPlaywrightExecuteResponse, err error)
25+
}
26+
2027
// BrowserPlaywrightExecutorService defines the subset we use for Playwright executors.
2128
type BrowserPlaywrightExecutorService interface {
2229
List(ctx context.Context, idOrName string, opts ...option.RequestOption) (*kernel.ExecutorList, error)
@@ -40,6 +47,14 @@ count); a call that would create another fails with HTTP 409. Named executors
4047
are not removed automatically while the browser runs, so delete the ones you no
4148
longer need.`
4249

50+
type BrowsersPlaywrightExecuteInput struct {
51+
Identifier string
52+
Code string
53+
Executor string
54+
Timeout int64
55+
Output string
56+
}
57+
4358
type BrowsersPlaywrightExecutorsListInput struct {
4459
Identifier string
4560
Output string
@@ -51,11 +66,85 @@ type BrowsersPlaywrightExecutorsDeleteInput struct {
5166
CloseTab param.Opt[bool]
5267
}
5368

69+
func (b BrowsersCmd) PlaywrightExecute(ctx context.Context, in BrowsersPlaywrightExecuteInput) error {
70+
if err := validateJSONOutput(in.Output); err != nil {
71+
return err
72+
}
73+
74+
if b.playwright == nil {
75+
pterm.Error.Println("playwright service not available")
76+
return nil
77+
}
78+
br, err := b.browsers.Get(ctx, in.Identifier, kernel.BrowserGetParams{})
79+
if err != nil {
80+
return util.CleanedUpSdkError{Err: err}
81+
}
82+
params := kernel.BrowserPlaywrightExecuteParams{Code: in.Code}
83+
if in.Executor != "" {
84+
params.Executor = kernel.Opt(in.Executor)
85+
}
86+
if in.Timeout > 0 {
87+
params.TimeoutSec = kernel.Opt(in.Timeout)
88+
}
89+
res, err := b.playwright.Execute(ctx, br.SessionID, params)
90+
if err != nil {
91+
return playwrightExecuteError(err, in)
92+
}
93+
94+
if in.Output == "json" {
95+
return util.PrintPrettyJSON(res)
96+
}
97+
98+
rows := pterm.TableData{{"Property", "Value"}, {"Success", fmt.Sprintf("%t", res.Success)}}
99+
if res.JSON.Tab.Valid() {
100+
rows = append(rows, []string{"Tab Target ID", res.Tab.TargetID}, []string{"Tab Created", fmt.Sprintf("%t", res.Tab.Created)})
101+
}
102+
PrintTableNoPad(rows, true)
103+
104+
if res.Stdout != "" {
105+
pterm.Info.Println("stdout:")
106+
fmt.Println(res.Stdout)
107+
}
108+
if res.Stderr != "" {
109+
pterm.Info.Println("stderr:")
110+
fmt.Fprintln(os.Stderr, res.Stderr)
111+
}
112+
if res.Result != nil {
113+
bs, err := json.MarshalIndent(res.Result, "", " ")
114+
if err == nil {
115+
pterm.Info.Println("result:")
116+
fmt.Println(string(bs))
117+
}
118+
}
119+
if !res.Success && res.Error != "" {
120+
pterm.Error.Printf("error: %s\n", res.Error)
121+
}
122+
return nil
123+
}
124+
125+
// playwrightExecuteError points at the executors commands when a named
126+
// executor call is rejected with 409 because the browser is at its executor limit.
127+
func playwrightExecuteError(err error, in BrowsersPlaywrightExecuteInput) error {
128+
var apiErr *kernel.Error
129+
if in.Executor == "" || !errors.As(err, &apiErr) || apiErr.StatusCode != http.StatusConflict {
130+
return util.CleanedUpSdkError{Err: err}
131+
}
132+
var body struct {
133+
Message string `json:"message"`
134+
}
135+
if json.Unmarshal([]byte(apiErr.RawJSON()), &body) != nil || body.Message == "" {
136+
return util.CleanedUpSdkError{Err: err}
137+
}
138+
// Not %w: the root handler re-renders any wrapped *kernel.Error as "code: message", dropping the hint.
139+
return fmt.Errorf("%s. See them with 'kernel browsers playwright executors list %s' and free one with 'kernel browsers playwright executors delete %s <executor>'",
140+
strings.TrimSuffix(body.Message, "."), in.Identifier, in.Identifier)
141+
}
142+
54143
func (b BrowsersCmd) PlaywrightExecutorsList(ctx context.Context, in BrowsersPlaywrightExecutorsListInput) error {
55144
if err := validateJSONOutput(in.Output); err != nil {
56145
return err
57146
}
58-
res, err := b.executors.List(ctx, in.Identifier)
147+
res, err := b.playwrightExecutors.List(ctx, in.Identifier)
59148
if err != nil {
60149
return util.CleanedUpSdkError{Err: err}
61150
}
@@ -76,7 +165,7 @@ func (b BrowsersCmd) PlaywrightExecutorsList(ctx context.Context, in BrowsersPla
76165

77166
func (b BrowsersCmd) PlaywrightExecutorsDelete(ctx context.Context, in BrowsersPlaywrightExecutorsDeleteInput) error {
78167
params := kernel.BrowserPlaywrightExecutorDeleteParams{IDOrName: in.Identifier, CloseTab: in.CloseTab}
79-
if err := b.executors.Delete(ctx, in.Name, params); err != nil {
168+
if err := b.playwrightExecutors.Delete(ctx, in.Name, params); err != nil {
80169
return util.CleanedUpSdkError{Err: err}
81170
}
82171
if in.Name == "default" {
@@ -87,38 +176,6 @@ func (b BrowsersCmd) PlaywrightExecutorsDelete(ctx context.Context, in BrowsersP
87176
return nil
88177
}
89178

90-
// playwrightExecuteError turns the 409 returned when a call would exceed the
91-
// named executor limit into an error that names the current executors.
92-
func playwrightExecuteError(err error) error {
93-
var apiErr *kernel.Error
94-
if !errors.As(err, &apiErr) || apiErr.StatusCode != http.StatusConflict {
95-
return util.CleanedUpSdkError{Err: err}
96-
}
97-
var body struct {
98-
Message string `json:"message"`
99-
Executors []kernel.Executor `json:"executors"`
100-
}
101-
if json.Unmarshal([]byte(apiErr.RawJSON()), &body) != nil || body.Message == "" {
102-
return util.CleanedUpSdkError{Err: err}
103-
}
104-
var sb strings.Builder
105-
sb.WriteString(body.Message)
106-
if len(body.Executors) > 0 {
107-
sb.WriteString("\nCurrent executors:")
108-
for _, e := range body.Executors {
109-
fmt.Fprintf(&sb, "\n %s", e.Name)
110-
if e.Busy {
111-
sb.WriteString(" (busy)")
112-
}
113-
if e.URL != "" {
114-
fmt.Fprintf(&sb, " %s", e.URL)
115-
}
116-
}
117-
}
118-
sb.WriteString("\nDelete one with 'kernel browsers playwright executors delete <id-or-name> <executor>'")
119-
return errors.New(sb.String())
120-
}
121-
122179
func newBrowsersPlaywrightCommand() *cobra.Command {
123180
root := &cobra.Command{Use: "playwright", Short: "Playwright operations"}
124181
execute := &cobra.Command{
@@ -130,7 +187,7 @@ func newBrowsersPlaywrightCommand() *cobra.Command {
130187
Args: cobra.MinimumNArgs(1),
131188
RunE: runBrowsersPlaywrightExecute,
132189
}
133-
execute.Flags().String("executor", "", "Executor to run the call in. Calls on different executors run concurrently in separate tabs of the same browser; calls on one executor run one at a time. Omit to use the always-present 'default' executor bound to the active tab. Any other name creates a named executor on first use that owns a background tab 'page' is bound to. At most 8 named executors per browser (409 when exceeded)")
190+
execute.Flags().String("executor", "", "Named executor to run the call in; each owns its own tab (default: the active tab)")
134191
execute.Flags().Int64("timeout", 0, "Maximum execution time in seconds (default per server)")
135192
addJSONOutputFlag(execute)
136193
root.AddCommand(execute, newBrowsersPlaywrightExecutorsCommand())
@@ -163,10 +220,38 @@ func newBrowsersPlaywrightExecutorsCommand() *cobra.Command {
163220
return root
164221
}
165222

223+
func runBrowsersPlaywrightExecute(cmd *cobra.Command, args []string) error {
224+
client := getKernelClient(cmd)
225+
svc := client.Browsers
226+
227+
var code string
228+
if len(args) >= 2 {
229+
code = strings.Join(args[1:], " ")
230+
} else {
231+
// Read code from stdin
232+
stat, _ := os.Stdin.Stat()
233+
if (stat.Mode() & os.ModeCharDevice) != 0 {
234+
pterm.Error.Println("no code provided. Provide code as an argument or pipe via stdin")
235+
return nil
236+
}
237+
data, err := io.ReadAll(os.Stdin)
238+
if err != nil {
239+
pterm.Error.Printf("failed to read stdin: %v\n", err)
240+
return nil
241+
}
242+
code = string(data)
243+
}
244+
executor, _ := cmd.Flags().GetString("executor")
245+
timeout, _ := cmd.Flags().GetInt64("timeout")
246+
output, _ := cmd.Flags().GetString("output")
247+
b := BrowsersCmd{browsers: &svc, playwright: &svc.Playwright}
248+
return b.PlaywrightExecute(cmd.Context(), BrowsersPlaywrightExecuteInput{Identifier: args[0], Code: strings.TrimSpace(code), Executor: executor, Timeout: timeout, Output: output})
249+
}
250+
166251
func runBrowsersPlaywrightExecutorsList(cmd *cobra.Command, args []string) error {
167252
output, _ := cmd.Flags().GetString("output")
168253
client := getKernelClient(cmd)
169-
b := BrowsersCmd{executors: &client.Browsers.Playwright.Executors}
254+
b := BrowsersCmd{playwrightExecutors: &client.Browsers.Playwright.Executors}
170255
return b.PlaywrightExecutorsList(cmd.Context(), BrowsersPlaywrightExecutorsListInput{Identifier: args[0], Output: output})
171256
}
172257

@@ -177,6 +262,6 @@ func runBrowsersPlaywrightExecutorsDelete(cmd *cobra.Command, args []string) err
177262
closeTab = kernel.Opt(value)
178263
}
179264
client := getKernelClient(cmd)
180-
b := BrowsersCmd{executors: &client.Browsers.Playwright.Executors}
265+
b := BrowsersCmd{playwrightExecutors: &client.Browsers.Playwright.Executors}
181266
return b.PlaywrightExecutorsDelete(cmd.Context(), BrowsersPlaywrightExecutorsDeleteInput{Identifier: args[0], Name: args[1], CloseTab: closeTab})
182267
}

0 commit comments

Comments
 (0)