Skip to content

Commit e020a2b

Browse files
committed
fix(execution): describe a signal's exit code without claiming a kill
1 parent 03f7752 commit e020a2b

3 files changed

Lines changed: 32 additions & 31 deletions

File tree

‎apps/sim/lib/execution/remote-sandbox/code-failure.test.ts‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -199,24 +199,29 @@ describe('processCodeFailure', () => {
199199
expect(error).toEqual({ name, value, traceback: stderr })
200200
})
201201

202-
it('names the signal for a process killed after printing only a warning', () => {
202+
it('keeps the printed headline and adds the signal exit code beside it', () => {
203203
const warning = 'Warning: You are sending unauthenticated requests to the HF Hub.'
204204
const killed = processCodeFailure({ stdout: '', stderr: warning, exitCode: 137 })
205205
const exited = processCodeFailure({ stdout: '', stderr: warning, exitCode: 1 })
206206

207207
expect(killed.error).toEqual({
208-
name: 'Error',
209-
value: 'Process exited with code 137 (killed by SIGKILL, usually out of memory)',
208+
name: 'Warning',
209+
value:
210+
'You are sending unauthenticated requests to the HF Hub. [Process exited with code 137 (the exit code of SIGKILL, usually out of memory)]',
211+
traceback: warning,
212+
})
213+
expect(exited.error).toEqual({
214+
name: 'Warning',
215+
value: 'You are sending unauthenticated requests to the HF Hub.',
210216
traceback: warning,
211217
})
212-
expect(exited.error?.name).toBe('Warning')
213218
})
214219

215220
it.each([
216221
[1, 'Process exited with code 1'],
217222
[128, 'Process exited with code 128'],
218-
[129, 'Process exited with code 129 (killed by signal 1)'],
219-
[143, 'Process exited with code 143 (killed by SIGTERM)'],
223+
[129, 'Process exited with code 129 (the exit code of signal 1)'],
224+
[143, 'Process exited with code 143 (the exit code of SIGTERM)'],
220225
[193, 'Process exited with code 193'],
221226
])('describes exit code %i', (exitCode, description) => {
222227
expect(describeProcessExit(exitCode)).toBe(description)

‎apps/sim/lib/execution/remote-sandbox/code-failure.ts‎

Lines changed: 19 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -22,39 +22,32 @@ const SIGNAL_NAMES: Readonly<Record<number, string>> = {
2222
}
2323

2424
/**
25-
* `Process exited with code N`, naming the signal behind a code above 128. A killed process often
26-
* printed only a warning first, and that warning must not read as the reason it failed; SIGKILL
27-
* (137) is usually the out-of-memory killer.
25+
* `Process exited with code N`, naming the signal a code above 128 stands for. A process a signal
26+
* killed often printed only a warning first, so the code must be visible beside whatever it
27+
* printed. The code alone cannot prove a kill (a program may exit 137 itself), so it is described
28+
* as the signal's code; SIGKILL's (137) usually means the out-of-memory killer.
2829
*/
2930
export function describeProcessExit(exitCode: number): string {
30-
const described = `Process exited with code ${exitCode}`
31-
const signal = killingSignal(exitCode)
32-
if (signal === undefined) return described
33-
const name = SIGNAL_NAMES[signal] ?? `signal ${signal}`
34-
return `${described} (killed by ${name}${signal === 9 ? ', usually out of memory' : ''})`
31+
const note = signalCodeNote(exitCode)
32+
return `Process exited with code ${exitCode}${note ? ` (${note})` : ''}`
3533
}
3634

37-
/** The signal a shell's `128 + n` exit code reports, if the code is one. */
38-
function killingSignal(exitCode: number): number | undefined {
39-
return exitCode > 128 && exitCode <= 128 + 64 ? exitCode - 128 : undefined
35+
/** What a shell's `128 + n` exit code stands for, if the code is one. */
36+
function signalCodeNote(exitCode: number): string | undefined {
37+
if (exitCode <= 128 || exitCode > 128 + 64) return undefined
38+
const signal = exitCode - 128
39+
const name = SIGNAL_NAMES[signal] ?? `signal ${signal}`
40+
return `the exit code of ${name}${signal === 9 ? ', usually out of memory' : ''}`
4041
}
4142

4243
/**
4344
* Converts a failed code command into a code result, naming the error from the
44-
* last error headline of the traceback (stderr, else stdout). A process a signal
45-
* killed raised nothing: anything it printed (a warning) is not the cause, so the
46-
* signal names the error and the output stays as the traceback.
45+
* last error headline of the traceback (stderr, else stdout). A signal's exit code
46+
* is appended to the value, since a killed process's last printed line (often a
47+
* warning) is not necessarily why it stopped.
4748
*/
4849
export function processCodeFailure(result: SandboxCommandResult): SandboxCodeResult {
4950
const traceback = result.stderr || result.stdout
50-
if (killingSignal(result.exitCode) !== undefined) {
51-
return {
52-
text: '',
53-
stdout: result.stdout,
54-
stderr: result.stderr,
55-
error: { name: 'Error', value: describeProcessExit(result.exitCode), traceback },
56-
}
57-
}
5851
const headline = traceback
5952
.split('\n')
6053
.reverse()
@@ -66,10 +59,13 @@ export function processCodeFailure(result: SandboxCommandResult): SandboxCodeRes
6659
const separator = errorLine?.indexOf(':') ?? -1
6760
const parsedErrorLine = errorLine ?? ''
6861
const name = separator > 0 ? parsedErrorLine.slice(0, separator) : 'Error'
69-
const value =
62+
const headlineValue =
7063
separator > 0
7164
? parsedErrorLine.slice(separator + 1).trim()
7265
: parsedErrorLine || 'Execution failed'
66+
const value = signalCodeNote(result.exitCode)
67+
? `${headlineValue} [${describeProcessExit(result.exitCode)}]`
68+
: headlineValue
7369
return {
7470
text: '',
7571
stdout: result.stdout,

‎apps/sim/lib/execution/remote-sandbox/conformance.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1261,14 +1261,14 @@ describe.each(PROVIDERS)('sandbox conformance [%s]', (provider) => {
12611261
})
12621262
})
12631263

1264-
it("leads a killed command's error with its signal, not the warning it printed first", async () => {
1264+
it("leads a failed command's error with its exit code and the signal it stands for", async () => {
12651265
const warning = 'Warning: You are sending unauthenticated requests to the HF Hub.'
12661266
stubShellCommand(provider, provider === 'daytona' ? warning : '', warning, 137)
12671267

12681268
const res = await executeShellInSandbox({ code: 'python train.py', envs: {}, timeoutMs: 1000 })
12691269

12701270
expect(res.error).toBe(
1271-
`Process exited with code 137 (killed by SIGKILL, usually out of memory): ${warning}`
1271+
`Process exited with code 137 (the exit code of SIGKILL, usually out of memory): ${warning}`
12721272
)
12731273
expect(res.stdout).toContain(warning)
12741274
})

0 commit comments

Comments
 (0)