Fix launching a browser results in closed file already closed error

[inancgumus]

Brief summary

TestBrowserCrashErr ends with the following error when trying to launch a browser:

--- FAIL: TestBrowserCrashErr (0.03s)
    browser_test.go:151: 
            Error Trace:    /home/runner/work/xk6-browser/xk6-browser/tests/page_test.go:705
                                        /home/runner/work/xk6-browser/xk6-browser/tests/browser_test.go:151
            Error:          Error "GoError: launching browser: read |0: file already closed" does not contain "launching browser: Invalid devtools server port"
            Test:           TestBrowserCrashErr

Although we only observe the error on CI for now. It can be problematic in some instances when users launch a browser too. We need to fix it before it happens again.

xk6-browser version

79b7f087

OS

Github CI

Chrome version

107.0.5304.110

Docker version and image (if applicable)

No response

Steps to reproduce the problem

Last detected in: TestBrowserCrashErr.

https://github.com/grafana/xk6-browser/actions/runs/3438417446/jobs/5734442790

Expected behaviour

No error.

Actual behaviour

Error.

Related issues

415, #363.

[ankur22]

I was able to reproduce this issue with the help of https://stackoverflow.com/questions/70422836/error-in-reading-the-byte-read-0-file-already-closed-in-go. I wasn't able to reproduce this just by stress testing the test unfortunately. It had to be done in a manual way:

1. Checkout 79b7f08.

2. Use the following diff:

     diff --git a/common/browser_process.go b/common/browser_process.go
     index 78de55d..8fc5a7c 100644
     --- a/common/browser_process.go
     +++ b/common/browser_process.go
     @@ -159,6 +159,10 @@ func execute(
           return command{}, fmt.Errorf("%w", ctx.Err())
       }
   
     + if cmd.Process != nil && cmd.Process.Pid != 0 {
     +     fmt.Println(cmd.Process.Pid)
     + }
     +
       done := make(chan struct{})
       go func() {
           // TODO: How to handle these errors?
     diff --git a/tests/test_browser.go b/tests/test_browser.go
     index 26bfa7e..710eee7 100644
     --- a/tests/test_browser.go
     +++ b/tests/test_browser.go
     @@ -355,7 +355,7 @@ type withLaunchOptions = launchOptions
      // defaultLaunchOptions returns defaults for browser type launch options.
      // TestBrowser uses this for launching a browser type by default.
      func defaultLaunchOpts() launchOptions {
     - headless := true
     + headless := false
       if v, found := os.LookupEnv("XK6_BROWSER_TEST_HEADLESS"); found {
           headless, _ = strconv.ParseBool(v)
       }
   

3. Place a breakpoint at line 183 in browser_process.go (after applying the diff above).
4. Debug run TestBrowserCrashErr.
5. It should stop on line 183, and the PID of the newly launched browser should be in the debug console.
6. Use kill -9 {PID_NUM} (and replace {PID_NUM} with the actual pid num.
7. Now hit Continue.
8. You should end up with the same error.

Obviously this isn't a great way to replicate the issue. What it does show is that it's somewhat out of our control if the browser process die. Happy to discuss solutions, maybe we retry the launch?

[ankur22]

After a quick discussion, here are the next steps:

1. Improve the error message.
2. See if we can set a minimum VM machine on Github actions so that the browser instance doesn't crash due to resource limitations.

[ankur22]

I believe the Github Action Runners provide enough computer power to run the tests: https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners#supported-runners-and-hardware-resources.

WDYT @inancgumus @ka3de?

[ankur22]

@ka3de has been hitting this issue frequently today e.g. https://github.com/grafana/xk6-browser/actions/runs/4025838004/jobs/6919871984. I'll try to investigate within the CI job (lots of logging).

[inancgumus]

@grafana/k6-browser I've removed my assignment as I have yet to work on it.