Fix panic on invalid browser type option (#905)

* Avoid panic from BrowserType launch and connect

Instead this commit changes the API for these methods in order to return
an error that must be parsed from the mapping layer.

* Fix TestBrowserCrashErr

Now that launch method does not panic, we have to verify the returned
error from the Goja execution.

api/browser_type.go

@@ -6,9 +6,9 @@ import (
 
 // BrowserType is the public interface of a CDP browser client.
 type BrowserType interface {
-	Connect(wsEndpoint string) Browser
+	Connect(wsEndpoint string) (Browser, error)
 	ExecutablePath() string
-	Launch() (_ Browser, browserProcessID int)
+	Launch() (_ Browser, browserProcessID int, _ error)
 	LaunchPersistentContext(userDataDir string, opts goja.Value) Browser
 	Name() string
 }

browser/mapping.go

@@ -712,28 +712,39 @@ func mapBrowser(vu moduleVU, b api.Browser) mapping {
 func mapBrowserType(vu moduleVU, bt api.BrowserType, wsURL string, isRemoteBrowser bool) mapping {
 	rt := vu.Runtime()
 	return mapping{
-		"connect": func(wsEndpoint string, opts goja.Value) *goja.Object {
-			b := bt.Connect(wsEndpoint)
+		"connect": func(wsEndpoint string, opts goja.Value) (*goja.Object, error) {
+			b, err := bt.Connect(wsEndpoint)
+			if err != nil {
+				return nil, err //nolint:wrapcheck
+			}
 			m := mapBrowser(vu, b)
-			return rt.ToValue(m).ToObject(rt)
+			return rt.ToValue(m).ToObject(rt), nil
 		},
 		"executablePath":          bt.ExecutablePath,
 		"launchPersistentContext": bt.LaunchPersistentContext,
 		"name":                    bt.Name,
-		"launch": func(opts goja.Value) *goja.Object {
+		"launch": func(opts goja.Value) (*goja.Object, error) {
 			// If browser is remote, transition from launch
 			// to connect and avoid storing the browser pid
 			// as we have no access to it.
 			if isRemoteBrowser {
-				m := mapBrowser(vu, bt.Connect(wsURL))
-				return rt.ToValue(m).ToObject(rt)
+				b, err := bt.Connect(wsURL)
+				if err != nil {
+					return nil, err //nolint:wrapcheck
+				}
+				m := mapBrowser(vu, b)
+				return rt.ToValue(m).ToObject(rt), nil
 			}
 
-			b, pid := bt.Launch()
+			b, pid, err := bt.Launch()
+			if err != nil {
+				return nil, err //nolint:wrapcheck
+			}
 			// store the pid so we can kill it later on panic.
 			vu.registerPid(pid)
 			m := mapBrowser(vu, b)
-			return rt.ToValue(m).ToObject(rt)
+
+			return rt.ToValue(m).ToObject(rt), nil
 		},
 	}
 }

chromium/browser_type.go

@@ -103,10 +103,10 @@ func (b *BrowserType) initContext() context.Context {
 }
 
 // Connect attaches k6 browser to an existing browser instance.
-func (b *BrowserType) Connect(wsEndpoint string) api.Browser {
+func (b *BrowserType) Connect(wsEndpoint string) (api.Browser, error) {
 	ctx, browserOpts, logger, err := b.init(true)
 	if err != nil {
-		k6ext.Panic(ctx, "initializing browser type: %w", err)
+		return nil, fmt.Errorf("initializing browser type: %w", err)
 	}
 
 	bp, err := b.connect(ctx, wsEndpoint, browserOpts, logger)
@@ -115,10 +115,10 @@ func (b *BrowserType) Connect(wsEndpoint string) api.Browser {
 			Err:     err,
 			Timeout: browserOpts.Timeout,
 		}
-		k6ext.Panic(ctx, "%w", err)
+		return nil, fmt.Errorf("%w", err)
 	}
 
-	return bp
+	return bp, nil
 }
 
 func (b *BrowserType) connect(
@@ -158,10 +158,10 @@ func (b *BrowserType) link(
 
 // Launch allocates a new Chrome browser process and returns a new api.Browser value,
 // which can be used for controlling the Chrome browser.
-func (b *BrowserType) Launch() (_ api.Browser, browserProcessID int) {
+func (b *BrowserType) Launch() (_ api.Browser, browserProcessID int, _ error) {
 	ctx, browserOpts, logger, err := b.init(false)
 	if err != nil {
-		k6ext.Panic(ctx, "initializing browser type: %w", err)
+		return nil, 0, fmt.Errorf("initializing browser type: %w", err)
 	}
 
 	bp, pid, err := b.launch(ctx, browserOpts, logger)
@@ -170,10 +170,10 @@ func (b *BrowserType) Launch() (_ api.Browser, browserProcessID int) {
 			Err:     err,
 			Timeout: browserOpts.Timeout,
 		}
-		k6ext.Panic(ctx, "%w", err)
+		return nil, 0, fmt.Errorf("%w", err)
 	}
 
-	return bp, pid
+	return bp, pid, nil
 }
 
 func (b *BrowserType) launch(

tests/browser_test.go

@@ -10,11 +10,12 @@ import (
 	"testing"
 	"time"
 
-	"github.com/dop251/goja"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/grafana/xk6-browser/browser"
 	"github.com/grafana/xk6-browser/common"
+	"github.com/grafana/xk6-browser/k6ext/k6test"
 )
 
 func TestBrowserNewPage(t *testing.T) {
@@ -141,14 +142,21 @@ func TestBrowserUserAgent(t *testing.T) {
 }
 
 func TestBrowserCrashErr(t *testing.T) {
-	t.Parallel()
-
-	assertExceptionContains(t, goja.New(), func() {
-		lopts := defaultBrowserOpts()
-		lopts.Args = []string{"remote-debugging-port=99999"}
-
-		newTestBrowser(t, lopts)
-	}, "launching browser: Invalid devtools server port")
+	vu := k6test.NewVU(t)
+	rt := vu.Runtime()
+	mod := browser.New().NewModuleInstance(vu)
+	jsMod, ok := mod.Exports().Default.(*browser.JSModule)
+	require.Truef(t, ok, "unexpected default mod export type %T", mod.Exports().Default)
+
+	vu.MoveToVUContext()
+	t.Setenv("K6_BROWSER_ARGS", "remote-debugging-port=99999")
+
+	require.NoError(t, rt.Set("chromium", jsMod.Chromium))
+	_, err := rt.RunString(`
+		const b = chromium.launch();
+		b.close();
+	`)
+	assert.ErrorContains(t, err, "launching browser: Invalid devtools server port")
 }
 
 func TestBrowserLogIterationID(t *testing.T) {
@@ -229,13 +237,15 @@ func TestMultiConnectToSingleBrowser(t *testing.T) {
 	tb := newTestBrowser(t, withSkipClose())
 	defer tb.Close()
 
-	b1 := tb.browserType.Connect(tb.wsURL)
+	b1, err := tb.browserType.Connect(tb.wsURL)
+	require.NoError(t, err)
 	bctx1, err := b1.NewContext(nil)
 	require.NoError(t, err)
 	p1, err := bctx1.NewPage()
 	require.NoError(t, err, "failed to create page #1")
 
-	b2 := tb.browserType.Connect(tb.wsURL)
+	b2, err := tb.browserType.Connect(tb.wsURL)
+	require.NoError(t, err)
 	bctx2, err := b2.NewContext(nil)
 	require.NoError(t, err)
 

tests/browser_type_test.go

@@ -18,8 +18,10 @@ func TestBrowserTypeConnect(t *testing.T) {
 	bt := chromium.NewBrowserType(vu)
 	vu.MoveToVUContext()
 
-	b := bt.Connect(tb.wsURL)
-	b.NewPage(nil)
+	b, err := bt.Connect(tb.wsURL)
+	require.NoError(t, err)
+	_, err = b.NewPage(nil)
+	require.NoError(t, err)
 }
 
 func TestBrowserTypeLaunchToConnect(t *testing.T) {

tests/test_browser.go

@@ -114,7 +114,10 @@ func newTestBrowser(tb testing.TB, opts ...any) *testBrowser {
 
 	bt.SetEnvLookupper(setupEnvLookupper(tb, browserOpts))
 
-	b, pid := bt.Launch()
+	b, pid, err := bt.Launch()
+	if err != nil {
+		tb.Fatalf("testBrowser: %v", err)
+	}
 	cb, ok := b.(*common.Browser)
 	if !ok {
 		tb.Fatalf("testBrowser: unexpected browser %T", b)