From afa48341fbd636c64c741a5e439c0b70231843da Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 30 Sep 2023 17:15:14 +0200 Subject: [PATCH 1/2] backend-dev-utils: fix ipc response race Signed-off-by: Patrik Oldsberg --- .changeset/itchy-monkeys-reply.md | 5 ++ packages/backend-dev-utils/src/ipcClient.ts | 60 +++++++++++---------- 2 files changed, 36 insertions(+), 29 deletions(-) create mode 100644 .changeset/itchy-monkeys-reply.md diff --git a/.changeset/itchy-monkeys-reply.md b/.changeset/itchy-monkeys-reply.md new file mode 100644 index 0000000000..3773fe3ff8 --- /dev/null +++ b/.changeset/itchy-monkeys-reply.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-dev-utils': patch +--- + +Fix an issue where early IPC responses would be lost. diff --git a/packages/backend-dev-utils/src/ipcClient.ts b/packages/backend-dev-utils/src/ipcClient.ts index 87c1596a39..d9ec7821de 100644 --- a/packages/backend-dev-utils/src/ipcClient.ts +++ b/packages/backend-dev-utils/src/ipcClient.ts @@ -38,6 +38,8 @@ type Response = const requestType = '@backstage/cli/channel/request'; const responseType = '@backstage/cli/channel/response'; +const IPC_TIMEOUT_MS = 5000; + /** * The client side of an IPC communication channel. * @@ -78,43 +80,43 @@ export class BackstageIpcClient { body, }; - this.#sendMessage(request, (e: Error) => { - if (e) { - reject(e); + let timeout: NodeJS.Timeout | undefined = undefined; + + const messageHandler = (response: Response) => { + if (response?.type !== responseType) { + return; + } + if (response.id !== id) { return; } - let timeout: NodeJS.Timeout | undefined = undefined; + clearTimeout(timeout); + timeout = undefined; + process.removeListener('message', messageHandler); - const messageHandler = (response: Response) => { - if (response?.type !== responseType) { - return; - } - if (response.id !== id) { - return; + if ('error' in response) { + const error = new Error(response.error.message); + if (response.error.name) { + error.name = response.error.name; } + reject(error); + } else { + resolve(response.body as TResponseBody); + } + }; - if ('error' in response) { - const error = new Error(response.error.message); - if (response.error.name) { - error.name = response.error.name; - } - reject(error); - } else { - resolve(response.body as TResponseBody); - } + timeout = setTimeout(() => { + reject(new Error(`IPC request '${method}' with ID ${id} timed out`)); + process.removeListener('message', messageHandler); + }, IPC_TIMEOUT_MS); + timeout.unref(); - clearTimeout(timeout); - process.removeListener('message', messageHandler); - }; + process.addListener('message', messageHandler as () => void); - timeout = setTimeout(() => { - reject(new Error(`IPC request '${method}' with ID ${id} timed out`)); - process.removeListener('message', messageHandler); - }, 5000); - timeout.unref(); - - process.addListener('message', messageHandler as () => void); + this.#sendMessage(request, (e: Error) => { + if (e) { + reject(e); + } }); }); } From d0f26cfa4fc82fecf1ec58f3898c6bc6e5d7675a Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 1 Oct 2023 15:35:04 +0200 Subject: [PATCH 2/2] cli: gracefully shut down child process on windows Signed-off-by: Patrik Oldsberg --- .changeset/red-bees-try.md | 5 +++++ packages/cli/package.json | 1 + .../cli/src/lib/experimental/startBackendExperimental.ts | 7 ++++++- yarn.lock | 8 ++++++++ 4 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 .changeset/red-bees-try.md diff --git a/.changeset/red-bees-try.md b/.changeset/red-bees-try.md new file mode 100644 index 0000000000..de7eacfe14 --- /dev/null +++ b/.changeset/red-bees-try.md @@ -0,0 +1,5 @@ +--- +'@backstage/cli': patch +--- + +Fixed an issue where the new backend start command would not gracefully shut down the backend process on Windows. diff --git a/packages/cli/package.json b/packages/cli/package.json index e49c649e8c..2f9f6109f0 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -78,6 +78,7 @@ "cross-fetch": "^3.1.5", "cross-spawn": "^7.0.3", "css-loader": "^6.5.1", + "ctrlc-windows": "^2.1.0", "diff": "^5.0.0", "esbuild": "^0.19.0", "esbuild-loader": "^2.18.0", diff --git a/packages/cli/src/lib/experimental/startBackendExperimental.ts b/packages/cli/src/lib/experimental/startBackendExperimental.ts index 1272ba9e66..578fd1c747 100644 --- a/packages/cli/src/lib/experimental/startBackendExperimental.ts +++ b/packages/cli/src/lib/experimental/startBackendExperimental.ts @@ -18,6 +18,7 @@ import { FSWatcher, watch } from 'chokidar'; import { BackendServeOptions } from '../bundler/types'; import type { ChildProcess } from 'child_process'; +import { ctrlc } from 'ctrlc-windows'; import { IpcServer } from './IpcServer'; import { ServerDataStore } from './ServerDataStore'; import debounce from 'lodash/debounce'; @@ -57,7 +58,11 @@ export async function startBackendExperimental(options: BackendServeOptions) { if (child && !child.killed && child.exitCode === null) { // We always wait for the existing process to exit, to make sure we don't get IPC conflicts shutdownPromise = new Promise(resolve => child!.once('exit', resolve)); - child.kill(); + if (process.platform === 'win32' && child.pid) { + ctrlc(child.pid); + } else { + child.kill(); + } await shutdownPromise; shutdownPromise = undefined; } diff --git a/yarn.lock b/yarn.lock index 16d1e39149..12bc38b450 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3795,6 +3795,7 @@ __metadata: cross-fetch: ^3.1.5 cross-spawn: ^7.0.3 css-loader: ^6.5.1 + ctrlc-windows: ^2.1.0 del: ^7.0.0 diff: ^5.0.0 esbuild: ^0.19.0 @@ -23048,6 +23049,13 @@ __metadata: languageName: node linkType: hard +"ctrlc-windows@npm:^2.1.0": + version: 2.1.0 + resolution: "ctrlc-windows@npm:2.1.0" + checksum: 0f0582ba9516290d3e90ea7b91710f8b9b110e1ed29b7c84ebd44c16368b2553722b86a17226120ca3ea0ef679ac3596f48104cc113cfb7c3d07260f6c92e38b + languageName: node + linkType: hard + "d3-array@npm:2 - 3, d3-array@npm:2.10.0 - 3, d3-array@npm:^3.1.6": version: 3.2.3 resolution: "d3-array@npm:3.2.3"