Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix broken BrowserStackLocal binary after first test #242

Merged
merged 1 commit into from
Apr 19, 2021

Conversation

Krinkle
Copy link
Contributor

@Krinkle Krinkle commented Apr 11, 2021

Currently after installing browserstack-runner you can run one test.
After that, the command is broken. There are numerous symptoms
from this:

  • BrowserStackLocal gets re-downloaded every single time.
  • an ETXTBSY error appears because the previous file is still busy.

Here is what happens on later runs:

  • fs.exists(localBinary) finds the binary.
  • runTunnelCmd(['-version']) gets the following output:
$ node_modules/browserstack-runner/lib/BrowserStackLocal -version
Sun Apr 11 2021 20:59:41 GMT+0000 (UTC) -- BrowserStackLocal v8.1

Sun Apr 11 2021 20:59:41 GMT+0000 (UTC) -- Container runtime environment detected
Sun Apr 11 2021 20:59:41 GMT+0000 (UTC) -- Attaching services to public interface
BrowserStack Local version 8.1
  • Only the first line of data is checked by subProcess.stdout.on('data').
  • The line does not match /version\s+(\d)/. It reports: Tunnel binary: found version null.
  • The script invokes callbackOnce() and will try to kill the process. This may succeed, but it is not immediate as there are additional output lines still in the data buffer which must be processed first, which is not possible since we are still indirectly in the call stack of the first data stdout callback.
  • The re-download begins, but the operating system does not permit writing to a currently executing binary, since it will not shutdown until a few milliseconds later.
  • We fail with Error: ETXTBSY: text file is busy.

All of this happens only because -version is interpreted as both -v for verbose and --version at the same time, the verbose output comes first and does not match the regex. Changing it to --version solves the problem.

This code should also be changed so that it actually waits for this subprocess to exit. Perhaps falling back to unlinking the file if we can't close it so that at least the redownload will work. But, that is another issue and will not be a problem in most cases.

Fixes #224.

Currently after installing browserstack-runner you can run one test.
After that, the command is broken. There are numerous symptoms
from this:

* BrowserStackLocal gets re-downloaded every single time.
* an ETXTBSY error appears because the previous file is still busy.

Here is what happens on later runs:

* `fs.exists(localBinary)` finds the binary.
* `runTunnelCmd(['-version'])` gets the following output:

```
$ node_modules/browserstack-runner/lib/BrowserStackLocal -version
Sun Apr 11 2021 20:59:41 GMT+0000 (UTC) -- BrowserStackLocal v8.1

Sun Apr 11 2021 20:59:41 GMT+0000 (UTC) -- Container runtime environment detected
Sun Apr 11 2021 20:59:41 GMT+0000 (UTC) -- Attaching services to public interface
BrowserStack Local version 8.1
```

* Only the first line of data is checked by `subProcess.stdout.on('data')`.
* The line does not match `/version\s+(\d)/`. It reports:  `Tunnel binary: found version null`.
* The script invokes `callbackOnce()` and will try to kill the process. This may succeed, but it is not immediate as there are additional output lines still in the data buffer which must be processed first, which is not possible since we are still indirectly in the call stack of the first `data` stdout callback.
* The re-download begins, but the operating system does not permit writing to a currently executing binary, since it will not shutdown until a few milliseconds later.
* We fail with `Error: ETXTBSY: text file is busy`.

All of this happens only because `-version` is interpreted as both `-v` for verbose and `--version` at the same time, the verbose output comes first and does not match the regex. Changing it to `--version` solves the problem.

This code should also be changed so that it actually waits for this subprocess to exit. Perhaps falling back to unlinking the file if we can't close it so that at least the redownload will work. But, that is another issue and will not be a problem in most cases.

Fixes browserstack#224.
Krinkle added a commit to qunitjs/browserstack-runner that referenced this pull request Apr 11, 2021
Currently after installing browserstack-runner you can run one test.
After that, the command is broken. There are numerous symptoms
from this:

* BrowserStackLocal gets re-downloaded every single time.
* an ETXTBSY error appears because the previous file is still busy.

Here is what happens on later runs:

* `fs.exists(localBinary)` finds the binary.
* `runTunnelCmd(['-version'])` gets the following output:

```
$ node_modules/browserstack-runner/lib/BrowserStackLocal -version
Sun Apr 11 2021 20:59:41 GMT+0000 (UTC) -- BrowserStackLocal v8.1

Sun Apr 11 2021 20:59:41 GMT+0000 (UTC) -- Container runtime environment detected
Sun Apr 11 2021 20:59:41 GMT+0000 (UTC) -- Attaching services to public interface
BrowserStack Local version 8.1
```

* Only the first line of data is checked by `subProcess.stdout.on('data')`.
* The line does not match `/version\s+(\d)/`. It reports:  `Tunnel binary: found version null`.
* The script invokes `callbackOnce()` and will try to kill the process. This may succeed, but it is not immediate as there are additional output lines still in the data buffer which must be processed first, which is not possible since we are still indirectly in the call stack of the first `data` stdout callback.
* The re-download begins, but the operating system does not permit writing to a currently executing binary, since it will not shutdown until a few milliseconds later.
* We fail with `Error: ETXTBSY: text file is busy`.

All of this happens only because `-version` is interpreted as both `-v` for verbose and `--version` at the same time, the verbose output comes first and does not match the regex. Changing it to `--version` solves the problem.

This code should also be changed so that it actually waits for this subprocess to exit. Perhaps falling back to unlinking the file if we can't close it so that at least the redownload will work. But, that is another issue and will not be a problem in most cases.

Fixes browserstack#224.
Ref browserstack#242.
@francisf francisf merged commit d75cbb4 into browserstack:master Apr 19, 2021
@Krinkle Krinkle deleted the patch-2 branch April 19, 2021 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

spawn EXTBSY error
3 participants