Skip to content

Commit

Permalink
fix: catch and ignore .kill exceptions and don't force timeout kill i…
Browse files Browse the repository at this point in the history
…n windows
  • Loading branch information
pieh committed Dec 10, 2024
1 parent ef92c48 commit d055d9d
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 5 deletions.
17 changes: 15 additions & 2 deletions packages/build/src/plugins/spawn.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { createRequire } from 'module'
import { platform } from 'os'
import { fileURLToPath, pathToFileURL } from 'url'
import { promisify } from 'util'

import { trace } from '@opentelemetry/api'
import { ExecaChildProcess, execaNode } from 'execa'
import { gte } from 'semver'
import { gte, satisfies } from 'semver'

import { FeatureFlags } from '../core/feature_flags.js'
import { addErrorInfo } from '../error/info.js'
Expand Down Expand Up @@ -217,5 +218,17 @@ const stopPlugin = async function ({
})
childProcess.disconnect()
}
childProcess.kill()

// On Windows with Node 21+, there's a bug where attempting to kill a child process
// results in an EPERM error. Ignore the error in that case.
// See: https://github.com/nodejs/node/issues/51766
// We also disable execa's `forceKillAfterTimeout` in this case
// which can cause unhandled rejection.
try {
childProcess.kill('SIGTERM', {
forceKillAfterTimeout: platform() === 'win32' && satisfies(process.version, '>=21') ? false : undefined,
})
} catch {
// no-op
}
}
19 changes: 16 additions & 3 deletions packages/edge-bundler/node/server/util.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import { platform } from 'os'

import { ExecaChildProcess } from 'execa'
import fetch from 'node-fetch'
import waitFor from 'p-wait-for'
import { satisfies } from 'semver'

// 1 second
const SERVER_KILL_TIMEOUT = 1e3
Expand Down Expand Up @@ -43,9 +46,19 @@ const killProcess = (ps: ExecaChildProcess<string>) => {
ps.on('close', resolve)
ps.on('error', reject)

ps.kill('SIGTERM', {
forceKillAfterTimeout: SERVER_KILL_TIMEOUT,
})
// On Windows with Node 21+, there's a bug where attempting to kill a child process
// results in an EPERM error. Ignore the error in that case.
// See: https://github.com/nodejs/node/issues/51766
// We also disable execa's `forceKillAfterTimeout` in this case
// which can cause unhandled rejection.
try {
ps.kill('SIGTERM', {
forceKillAfterTimeout:
platform() === 'win32' && satisfies(process.version, '>=21') ? false : SERVER_KILL_TIMEOUT,
})
} catch {
// no-op
}
})
}

Expand Down

0 comments on commit d055d9d

Please sign in to comment.