Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/windows-hide-child-process-console.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@effect/platform-node-shared": patch
---

Pass Node's `windowsHide` flag for spawned Windows children by default (except detached processes), with an independent
`windowsHide` option for callers that need visible GUI windows. Process-group cleanup now invokes `taskkill` without a
`cmd.exe` wrapper and hides its window.
8 changes: 8 additions & 0 deletions packages/effect/src/unstable/process/ChildProcess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,14 @@ export interface CommandOptions extends KillOptions {
* Defaults to `true` on non-Windows platforms and `false` on Windows platforms.
*/
readonly detached?: boolean | undefined
/**
* If set to `true`, prevents the child process's console or GUI window from
* becoming visible on Windows.
*
* Defaults to `true` unless `detached` is set to `true`. This option has no
* effect on non-Windows platforms.
*/
readonly windowsHide?: boolean | undefined
Comment thread
tim-smart marked this conversation as resolved.
/**
* Configuration options for the standard input stream for the child process.
*/
Expand Down
27 changes: 16 additions & 11 deletions packages/platform-node-shared/src/NodeChildProcessSpawner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import {
} from "effect/unstable/process/ChildProcessSpawner"
import * as NodeChildProcess from "node:child_process"
import { PassThrough } from "node:stream"
import { buildSpawnOptions } from "./internal/nodeChildProcessSpawner.ts"
import { handleErrnoException } from "./internal/utils.ts"
import * as NodeSink from "./NodeSink.ts"
import * as NodeStream from "./NodeStream.ts"
Expand All @@ -65,6 +66,17 @@ const toPlatformError = (
type ExitCodeWithSignal = readonly [code: number | null, signal: NodeJS.Signals | null]
type ExitSignal = Deferred.Deferred<ExitCodeWithSignal>

const taskkill = (
childProcess: NodeChildProcess.ChildProcess,
onExit: (error: NodeChildProcess.ExecException | null) => void = () => {}
) =>
NodeChildProcess.execFile(
"taskkill",
["/pid", String(childProcess.pid!), "/T", "/F"],
{ windowsHide: true },
onExit
)

const make = Effect.gen(function*() {
const fs = yield* FileSystem.FileSystem
const path = yield* Path.Path
Expand Down Expand Up @@ -354,7 +366,7 @@ const make = Effect.gen(function*() {
) => {
if (globalThis.process.platform === "win32") {
return Effect.callback<void, PlatformError.PlatformError>((resume) => {
NodeChildProcess.exec(`taskkill /pid ${childProcess.pid} /T /F`, (error) => {
taskkill(childProcess, (error) => {
if (error) {
resume(Effect.fail(toPlatformError("kill", toError(error), command)))
} else {
Expand All @@ -376,9 +388,8 @@ const make = Effect.gen(function*() {
signal: NodeJS.Signals
): void => {
if (globalThis.process.platform === "win32") {
NodeChildProcess.exec(`taskkill /pid ${childProcess.pid} /T /F`, () => {
// ignore errors during best-effort cleanup
})
// ignore errors during best-effort cleanup
taskkill(childProcess)
return
}
try {
Expand Down Expand Up @@ -471,13 +482,7 @@ const make = Effect.gen(function*() {
const stdio = buildStdioArray(stdinConfig, stdoutConfig, stderrConfig, resolvedAdditionalFds)

const [childProcess, exitSignal] = yield* Effect.acquireRelease(
spawn(cmd, {
cwd,
env,
stdio,
detached: cmd.options.detached ?? process.platform !== "win32",
shell: cmd.options.shell
}),
spawn(cmd, buildSpawnOptions(cmd.options, { cwd, env, stdio }, process.platform)),
Effect.fnUntraced(function*([childProcess, exitSignal]) {
const exited = yield* Deferred.isDone(exitSignal)
const killWithTimeout = withTimeout(childProcess, cmd, cmd.options)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import type * as ChildProcess from "effect/unstable/process/ChildProcess"
import type * as NodeChildProcess from "node:child_process"

export const buildSpawnOptions = (
options: ChildProcess.CommandOptions,
base: Pick<NodeChildProcess.SpawnOptions, "cwd" | "env" | "stdio">,
platform: NodeJS.Platform
): NodeChildProcess.SpawnOptions => {
const detached = options.detached ?? platform !== "win32"
return {
...base,
detached,
shell: options.shell,
windowsHide: options.windowsHide ?? !detached
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { buildSpawnOptions } from "@effect/platform-node-shared/internal/nodeChildProcessSpawner"
import * as NodeChildProcessSpawner from "@effect/platform-node-shared/NodeChildProcessSpawner"
import * as NodeFileSystem from "@effect/platform-node-shared/NodeFileSystem"
import * as NodePath from "@effect/platform-node-shared/NodePath"
import { assert, it } from "@effect/vitest"
import { assert, describe, it } from "@effect/vitest"
import * as ChildProcessSpawnerTest from "effect-test/unstable/process/ChildProcessSpawnerTest"
import * as Effect from "effect/Effect"
import * as FileSystem from "effect/FileSystem"
Expand All @@ -19,6 +20,52 @@ ChildProcessSpawnerTest.suite("NodeChildProcessSpawner", NodeServices, {
processGroups: true
})

describe("buildSpawnOptions", () => {
const base = { stdio: "pipe" } as const

it("defaults to hiding non-detached Windows children", () => {
assert.deepStrictEqual(buildSpawnOptions({}, base, "win32"), {
stdio: "pipe",
detached: false,
shell: undefined,
windowsHide: true
})
assert.deepStrictEqual(buildSpawnOptions({ detached: true }, base, "win32"), {
stdio: "pipe",
detached: true,
shell: undefined,
windowsHide: false
})
assert.deepStrictEqual(buildSpawnOptions({ detached: false }, base, "win32"), {
stdio: "pipe",
detached: false,
shell: undefined,
windowsHide: true
})
})

it("allows windowsHide to be configured independently of detached", () => {
assert.deepStrictEqual(
buildSpawnOptions({ detached: false, windowsHide: false }, base, "win32"),
{
stdio: "pipe",
detached: false,
shell: undefined,
windowsHide: false
}
)
assert.deepStrictEqual(
buildSpawnOptions({ detached: true, windowsHide: true }, base, "win32"),
{
stdio: "pipe",
detached: true,
shell: undefined,
windowsHide: true
}
)
})
})

it.live("kills every process in a pipeline", () =>
Effect.gen(function*() {
const fs = yield* FileSystem.FileSystem
Expand Down
Loading