fix(subprocess): harden cancellation settlement
This commit is contained in:
1 parent
05b6789425
commit
bc681d7a54
8 files changed
+75
-8
No files matched your search
@@ -285,7 +285,13 @@ export class LocalBashExecutor extends ShellExecutor {
|
||||
}, (error: unknown) => {
|
||||
// Background provider failures settle as killed and surface through the read path.
|
||||
proc.status = 'killed'
|
||||
providerFailureNote = `subprocess failed before reporting an outcome: ${String(error)}`
|
||||
let detail = 'unprintable provider failure'
|
||||
try {
|
||||
detail = String(error)
|
||||
} catch {
|
||||
// Provider-owned rejection values cannot make ShellProcess.done reject.
|
||||
}
|
||||
providerFailureNote = `subprocess failed before reporting an outcome: ${detail}`
|
||||
this.onProcessDone(proc, providerFailureNote, true, error)
|
||||
}),
|
||||
readOutput: (): ShellProcessRead => {
|
||||
|
||||
@@ -322,6 +322,32 @@ describe('LocalBashExecutor.start (background process handles)', () => {
|
||||
expect(proc.readOutput().delta).toBe('')
|
||||
})
|
||||
|
||||
it('settles an unprintable provider rejection instead of rejecting done', async () => {
|
||||
const { ctx, bash } = await setup()
|
||||
const emptyReader: SubprocessOutputReader = {
|
||||
readFrom: () => ({ text: '', nextOffset: 0, lossy: false }),
|
||||
}
|
||||
const providerError = new Error('unprintable provider error')
|
||||
Object.defineProperty(providerError, Symbol.toPrimitive, {
|
||||
value: () => { throw new Error('provider formatting must not escape') },
|
||||
})
|
||||
vi.spyOn(ctx.subprocess, 'spawn').mockReturnValue({
|
||||
stdin: undefined,
|
||||
stdout: undefined,
|
||||
stderr: undefined,
|
||||
collected: { stdout: emptyReader, stderr: emptyReader },
|
||||
done: Promise.reject(providerError),
|
||||
terminate: vi.fn(),
|
||||
waitForExit: async () => true,
|
||||
} satisfies SubprocessHandle)
|
||||
|
||||
const proc = bash.start(bash.resolve({ command: 'true' }))
|
||||
await expect(proc.done).resolves.toBeUndefined()
|
||||
expect(proc.status).toBe('killed')
|
||||
expect(proc.readOutput().delta).toContain('unprintable provider failure')
|
||||
expect(proc.readOutput().delta).toBe('')
|
||||
})
|
||||
|
||||
it('an asynchronous creation failure settles as killed with a stage-neutral note', async () => {
|
||||
const { bash } = await setup()
|
||||
const proc = bash.start(bash.resolve({ command: 'true', workdir: '/nonexistent-dsh' }))
|
||||
|
||||
@@ -314,7 +314,13 @@ export class PwshLocalExecutor extends ShellExecutor {
|
||||
}, (error: unknown) => {
|
||||
// Background provider failures settle as killed and surface through the read path.
|
||||
proc.status = 'killed'
|
||||
providerFailureNote = `subprocess failed before reporting an outcome: ${String(error)}`
|
||||
let detail = 'unprintable provider failure'
|
||||
try {
|
||||
detail = String(error)
|
||||
} catch {
|
||||
// Provider-owned rejection values cannot make ShellProcess.done reject.
|
||||
}
|
||||
providerFailureNote = `subprocess failed before reporting an outcome: ${detail}`
|
||||
this.onProcessDone(proc, providerFailureNote, true, error)
|
||||
}),
|
||||
readOutput: (): ShellProcessRead => {
|
||||
|
||||
@@ -212,6 +212,23 @@ describe('spawn construction (pure, every platform)', () => {
|
||||
expect(output).not.toContain('spawn failed:')
|
||||
expect(proc.readOutput().delta).toBe('')
|
||||
})
|
||||
|
||||
it('settles an unprintable provider rejection instead of rejecting done', async () => {
|
||||
const ctx = new Context()
|
||||
const subprocess = new CapturingSubprocessRuntime(ctx)
|
||||
await ctx.plugin(PwshLocalExecutor)
|
||||
const providerError = new Error('unprintable provider error')
|
||||
Object.defineProperty(providerError, Symbol.toPrimitive, {
|
||||
value: () => { throw new Error('provider formatting must not escape') },
|
||||
})
|
||||
subprocess.done = Promise.reject(providerError)
|
||||
|
||||
const proc = ctx.shell.start(ctx.shell.resolve({ command: 'Write-Output maybe-ran' }))
|
||||
await expect(proc.done).resolves.toBeUndefined()
|
||||
expect(proc.status).toBe('killed')
|
||||
expect(proc.readOutput().delta).toContain('unprintable provider failure')
|
||||
expect(proc.readOutput().delta).toBe('')
|
||||
})
|
||||
})
|
||||
|
||||
describe.skipIf(!hasPwsh)('PwshLocalExecutor.run', () => {
|
||||
|
||||
@@ -156,7 +156,7 @@ interface DirectRange {
|
||||
}
|
||||
|
||||
class SystemdScopeOwner implements BoundProcessOwner {
|
||||
private establishment: 'pending' | 'established' | 'never-created' = 'pending'
|
||||
private establishment: 'pending' | 'established' = 'pending'
|
||||
private stopped = false
|
||||
private observation: Promise<void> | undefined
|
||||
private killFailure: Error | undefined
|
||||
@@ -229,7 +229,6 @@ class SystemdScopeOwner implements BoundProcessOwner {
|
||||
this.observeRequestConsumption()
|
||||
if (this.establishment === 'established') return false
|
||||
if (!this.direct.running() && existsSync(this.files.requestPath)) {
|
||||
this.establishment = 'never-created'
|
||||
return false
|
||||
}
|
||||
if (this.killFailure !== undefined) throw this.killFailure
|
||||
|
||||
@@ -344,7 +344,13 @@ export function validateSubprocessSpec(spec: SubprocessSpawnSpec): void {
|
||||
throw new Error(`subprocess graceMs must be a positive finite number no greater than ${MAX_TIMER_DELAY_MS}`)
|
||||
}
|
||||
if (spec.signal?.aborted) {
|
||||
throw new Error(`aborted before spawn: ${String(spec.signal.reason ?? 'aborted')}`)
|
||||
let reason = 'aborted'
|
||||
try {
|
||||
reason = String(spec.signal.reason ?? reason)
|
||||
} catch {
|
||||
// Arbitrary caller-owned reasons cannot escape the stable Error boundary.
|
||||
}
|
||||
throw new Error(`aborted before spawn: ${reason}`)
|
||||
}
|
||||
const [program] = spec.argv
|
||||
if (program === undefined || program.length === 0) {
|
||||
|
||||
@@ -141,7 +141,6 @@ export function launchWindowsJob(
|
||||
const rangeExit = Promise.withResolvers<void>()
|
||||
let directResultType: WindowsRunnerResult['type'] | undefined
|
||||
let runnerSpawned = false
|
||||
let runnerNeverCreated = false
|
||||
const failInfrastructure = (error: unknown): void => {
|
||||
direct.reject(error)
|
||||
rangeExit.reject(error)
|
||||
@@ -193,7 +192,6 @@ export function launchWindowsJob(
|
||||
})
|
||||
child.once('error', (error) => {
|
||||
if (!runnerSpawned) {
|
||||
runnerNeverCreated = true
|
||||
direct.reject(error)
|
||||
rangeExit.resolve()
|
||||
return
|
||||
@@ -201,7 +199,7 @@ export function launchWindowsJob(
|
||||
failInfrastructure(error)
|
||||
})
|
||||
child.once('close', (exitCode, signal) => {
|
||||
if (runnerNeverCreated) return
|
||||
if (!runnerSpawned) return
|
||||
const clean = exitCode === 0 && signal === null && directResultType !== undefined
|
||||
if (clean) {
|
||||
rangeExit.resolve()
|
||||
|
||||
@@ -323,6 +323,15 @@ describe('spawnSubprocess', () => {
|
||||
expect(() => { validateSubprocessSpec(spec('echo hi', { signal: controller.signal })) })
|
||||
.toThrow(new Error(message))
|
||||
}
|
||||
|
||||
const controller = new AbortController()
|
||||
controller.abort({
|
||||
[Symbol.toPrimitive]() {
|
||||
throw new Error('reason formatting must not escape')
|
||||
},
|
||||
})
|
||||
expect(() => { validateSubprocessSpec(spec('echo hi', { signal: controller.signal })) })
|
||||
.toThrow(new Error('aborted before spawn: aborted'))
|
||||
})
|
||||
|
||||
it('rejects with a spawn error for a nonexistent cwd', async () => {
|
||||
|
||||
Reference in new issue
Block a user