fix: 🐛 Fix windows being unable to search client daemon (#2245)

When using the client daemon to search on windows, we use
the CLI instead of the unix socket for our requests.
Unfortunately that means we spawn a separate process to
retrive the results and one idiosyncracy of this is that
`spawnSync` buffers its outputs which means we were hitting
the default max buffer size. `spawn` doesn't have this
issue because it streams its outputs and we didn't previously
use `spawnSync` for any command that produced large amounts
of output.

✅ Closes: https://hashicorp.atlassian.net/browse/ICU-13466
This commit is contained in:
Zhihe Li authored and GitHub committed 2024-04-17 07:40:09 -07:00
1 parent 0870a8f623
commit 6bbddc56b1
2 files changed
+22 -11

No files matched your search

@@ -78,6 +78,12 @@ module.exports = {
* @returns {{stdout: string | undefined, stderr: string | undefined}} */
spawnSync(args, envVars = {}) {
const childProcess = spawnSync(path(), args, {
// Some of our outputs (namely client daemon searching) can be very large.
// This an undocumented hack to allow for an unlimited buffer size which
// could change at any time. If it does, we should just set an arbitrarily
// large buffer size or switch to spawn and stream the output and handle
// killing the process ourselves.
maxBuffer: undefined,
env: {
...process.env,
...envVars,
@@ -103,7 +109,7 @@ module.exports = {
childProcess.stderr.on('data', (data) => {
resolve({ childProcess, stderr: data.toString() });
});
childProcess.on('error', (err) => reject(e));
childProcess.on('error', (err) => reject(err));
});
},
};
@@ -3,20 +3,18 @@
* SPDX-License-Identifier: MPL-2.0
*/
const {
spawnSync,
spawn,
spawnAsyncJSONPromise,
} = require('../helpers/spawn-promise');
const { spawnSync, spawn } = require('../helpers/spawn-promise');
const jsonify = require('../utils/jsonify.js');
const { unixSocketRequest } = require('../helpers/request-promise');
const runtimeSettings = require('./runtime-settings');
const sanitizer = require('../utils/sanitizer.js');
const { isWindows } = require('../helpers/platform.js');
const treeKill = require('tree-kill');
class ClientDaemonManager {
#socketPath;
#isClientDaemonAlreadyRunning = true;
#clientDaemonProcess;
get socketPath() {
return this.#socketPath;
@@ -42,7 +40,8 @@ class ClientDaemonManager {
async start() {
const startDaemonCommand = ['daemon', 'start'];
// We use spawn here because we want to check the stderr for specific logs
const { stderr } = await spawn(startDaemonCommand);
const { childProcess, stderr } = await spawn(startDaemonCommand);
this.#clientDaemonProcess = childProcess;
// If we get a null/undefined, err on safe side and don't stop daemon when
// we close the desktop client
@@ -56,12 +55,18 @@ class ClientDaemonManager {
* Stops the daemon.
*/
stop() {
if (this.#isClientDaemonAlreadyRunning) {
return;
// We started up the daemon so we should stop it
if (!this.#isClientDaemonAlreadyRunning) {
const stopDaemonCommand = ['daemon', 'stop'];
spawnSync(stopDaemonCommand);
}
const stopDaemonCommand = ['daemon', 'stop'];
spawnSync(stopDaemonCommand);
// Kill the process if it's still running
if (this.#clientDaemonProcess && !this.#clientDaemonProcess.killed) {
isWindows()
? treeKill(this.#clientDaemonProcess.pid)
: this.#clientDaemonProcess.kill();
}
}
/**