-
Notifications
You must be signed in to change notification settings - Fork 78
feat(cli): graceful termination signals + broken-pipe guard #185
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+254
−0
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,119 @@ | ||
| import { EventEmitter } from 'node:events'; | ||
| import { writeSync } from 'node:fs'; | ||
| import { describe, expect, it, vi } from 'vitest'; | ||
| import { | ||
| SIGINT_EXIT_CODE, | ||
| TERMINATION_EXIT_CODES, | ||
| formatInterruptMessage, | ||
| installBrokenPipeGuard, | ||
| installSignalHandlers, | ||
| } from './interrupt.js'; | ||
|
|
||
| // installSignalHandlers' default stderr writes via fs.writeSync (synchronous, so | ||
| // the hint survives a piped stderr before exit); mock it to assert on that path. | ||
| vi.mock('node:fs', async importOriginal => { | ||
| const actual = (await importOriginal()) as Record<string, unknown>; | ||
| return { ...actual, writeSync: vi.fn() }; | ||
| }); | ||
|
|
||
| describe('formatInterruptMessage', () => { | ||
| it('defaults to SIGINT and explains the run continues server-side', () => { | ||
| const message = formatInterruptMessage(); | ||
| expect(message).toContain('Interrupted (SIGINT)'); | ||
| expect(message).toContain('test wait'); | ||
| expect(message).toContain('test list'); | ||
| }); | ||
|
|
||
| it('names the specific signal when given one', () => { | ||
| expect(formatInterruptMessage('SIGTERM')).toContain('Interrupted (SIGTERM)'); | ||
| expect(formatInterruptMessage('SIGHUP')).toContain('Interrupted (SIGHUP)'); | ||
| }); | ||
| }); | ||
|
|
||
| describe('installSignalHandlers', () => { | ||
| it('registers SIGINT, SIGTERM and SIGHUP with the conventional 128+signum exit codes', () => { | ||
| const handlers = new Map<string, () => void>(); | ||
| const stderr: string[] = []; | ||
| const exit = vi.fn(); | ||
|
|
||
| installSignalHandlers({ | ||
| on: (signal, handler) => handlers.set(signal, handler), | ||
| stderr: line => stderr.push(line), | ||
| exit, | ||
| }); | ||
|
|
||
| expect([...handlers.keys()].sort()).toEqual(['SIGHUP', 'SIGINT', 'SIGTERM']); | ||
|
|
||
| handlers.get('SIGINT')!(); | ||
| expect(exit).toHaveBeenLastCalledWith(130); | ||
| handlers.get('SIGTERM')!(); | ||
| expect(exit).toHaveBeenLastCalledWith(143); | ||
| handlers.get('SIGHUP')!(); | ||
| expect(exit).toHaveBeenLastCalledWith(129); | ||
|
|
||
| // Each handler emits a leading blank line then the explanation. | ||
| expect(stderr[0]).toBe(''); | ||
| expect(stderr.join('\n')).toContain('Interrupted (SIGINT)'); | ||
| expect(stderr.join('\n')).toContain('Interrupted (SIGTERM)'); | ||
| expect(stderr.join('\n')).toContain('Interrupted (SIGHUP)'); | ||
| expect(SIGINT_EXIT_CODE).toBe(130); | ||
| expect(TERMINATION_EXIT_CODES.SIGTERM).toBe(143); | ||
| expect(TERMINATION_EXIT_CODES.SIGHUP).toBe(129); | ||
| }); | ||
|
|
||
| it('writes the hint synchronously via writeSync before exit (survives a piped stderr)', () => { | ||
| vi.mocked(writeSync).mockClear(); | ||
| const handlers = new Map<string, () => void>(); | ||
| const exit = vi.fn(); | ||
| // No stderr dep: exercise the synchronous default path. | ||
| installSignalHandlers({ | ||
| on: (signal, handler) => handlers.set(signal, handler), | ||
| exit, | ||
| }); | ||
| handlers.get('SIGINT')!(); | ||
| expect(exit).toHaveBeenCalledWith(130); | ||
| const written = vi | ||
| .mocked(writeSync) | ||
| .mock.calls.map(call => String(call[1])) | ||
| .join(''); | ||
| expect(written).toContain('Interrupted (SIGINT)'); | ||
| }); | ||
| }); | ||
|
|
||
| describe('installBrokenPipeGuard', () => { | ||
| function makeEpipe(): NodeJS.ErrnoException { | ||
| return Object.assign(new Error('write EPIPE'), { code: 'EPIPE' }); | ||
| } | ||
|
|
||
| it('exits 0 on stdout EPIPE (clean SIGPIPE-equivalent for `| head`)', () => { | ||
| const stdout = new EventEmitter(); | ||
| const stderr = new EventEmitter(); | ||
| const exit = vi.fn(); | ||
| installBrokenPipeGuard({ stdout, stderr, exit }); | ||
|
|
||
| stdout.emit('error', makeEpipe()); | ||
| expect(exit).toHaveBeenCalledWith(0); | ||
| }); | ||
|
|
||
| it('re-throws a non-EPIPE stdout error instead of silently swallowing it', () => { | ||
| const stdout = new EventEmitter(); | ||
| const stderr = new EventEmitter(); | ||
| const exit = vi.fn(); | ||
| installBrokenPipeGuard({ stdout, stderr, exit }); | ||
|
|
||
| expect(() => | ||
| stdout.emit('error', Object.assign(new Error('boom'), { code: 'ENOSPC' })), | ||
| ).toThrow('boom'); | ||
| expect(exit).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('swallows stderr EPIPE without exiting or throwing', () => { | ||
| const stdout = new EventEmitter(); | ||
| const stderr = new EventEmitter(); | ||
| const exit = vi.fn(); | ||
| installBrokenPipeGuard({ stdout, stderr, exit }); | ||
|
|
||
| expect(() => stderr.emit('error', makeEpipe())).not.toThrow(); | ||
| expect(exit).not.toHaveBeenCalled(); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,126 @@ | ||
| /** | ||
| * Process lifecycle hardening: graceful termination signals and broken-pipe. | ||
| * | ||
| * Termination signals: without a handler, Node terminates the process abruptly | ||
| * with no output, so a user (Ctrl+C), a CI runner or `docker stop` (SIGTERM), or | ||
| * a closed terminal/SSH session (SIGHUP) that interrupts a long | ||
| * `test run --wait` is left unsure whether the run was cancelled or is still | ||
| * executing server-side (it is: the CLI only polls; the run lives on the | ||
| * backend). The handler prints a one-line explanation plus how to resume, then | ||
| * exits with the conventional `128 + signal` code. | ||
| * | ||
| * Broken pipe: when output is piped to a reader that closes early | ||
| * (`testsprite ... | head`), the kernel raises `EPIPE` on the next stdout write. | ||
| * Node turns an `'error'` with no listener into an uncaughtException and dumps a | ||
| * raw `write EPIPE` stack (exit 1). The guard swallows it and exits 0, the | ||
| * conventional SIGPIPE-equivalent result for "the reader went away". | ||
| * | ||
| * `process` and the streams are injectable so the wiring is unit-testable | ||
| * without spawning a subprocess or sending a real signal. | ||
| */ | ||
|
|
||
| import { writeSync } from 'node:fs'; | ||
|
|
||
| /** | ||
| * Termination signals handled, mapped to their conventional `128 + signum` | ||
| * exit code. sourceRef: POSIX signal numbers (SIGHUP=1, SIGINT=2, SIGTERM=15). | ||
| */ | ||
| export const TERMINATION_EXIT_CODES = { | ||
| SIGINT: 130, // 128 + 2 | ||
| SIGTERM: 143, // 128 + 15 | ||
| SIGHUP: 129, // 128 + 1 | ||
| } as const; | ||
|
|
||
| export type TerminationSignal = keyof typeof TERMINATION_EXIT_CODES; | ||
|
|
||
| /** Back-compat alias: SIGINT's conventional exit code. */ | ||
| export const SIGINT_EXIT_CODE = TERMINATION_EXIT_CODES.SIGINT; | ||
|
|
||
| export function formatInterruptMessage(signal: TerminationSignal = 'SIGINT'): string { | ||
| return ( | ||
| `Interrupted (${signal}). Any run already started keeps executing on the server; ` + | ||
| 'check it with `testsprite test list` or `testsprite test wait <runId>`.' | ||
| ); | ||
| } | ||
|
|
||
| export interface InterruptDeps { | ||
| /** Signal registrar. Defaults to `process.on`. */ | ||
| on?: (signal: TerminationSignal, handler: () => void) => void; | ||
| /** Line-oriented stderr writer (appends a newline). */ | ||
| stderr?: (line: string) => void; | ||
| /** Process exit. Defaults to `process.exit`. */ | ||
| exit?: (code: number) => void; | ||
| } | ||
|
|
||
| /** | ||
| * Register handlers for SIGINT, SIGTERM and SIGHUP. Idempotent enough for a | ||
| * single top-level call in `index.ts`; not designed to be installed twice. | ||
| */ | ||
| export function installSignalHandlers(deps: InterruptDeps = {}): void { | ||
| const on = | ||
| deps.on ?? | ||
| ((signal: TerminationSignal, handler: () => void) => { | ||
| process.on(signal, handler); | ||
| }); | ||
| const stderr = | ||
| deps.stderr ?? | ||
| ((line: string) => { | ||
| // A signal handler calls process.exit() right after writing, which can | ||
| // truncate an async process.stderr.write() when stderr is a pipe. Write | ||
| // synchronously so the interrupt hint is flushed before the process exits. | ||
| try { | ||
| writeSync(process.stderr.fd, `${line}\n`); | ||
| } catch { | ||
| // Best-effort: if stderr is already gone (EPIPE), still exit cleanly. | ||
| } | ||
| }); | ||
| const exit = deps.exit ?? ((code: number) => process.exit(code)); | ||
|
|
||
| for (const signal of Object.keys(TERMINATION_EXIT_CODES) as TerminationSignal[]) { | ||
| on(signal, () => { | ||
| // Blank line first so the message starts on its own row rather than | ||
| // trailing the progress ticker's in-place line. | ||
| stderr(''); | ||
| stderr(formatInterruptMessage(signal)); | ||
| exit(TERMINATION_EXIT_CODES[signal]); | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| export interface BrokenPipeDeps { | ||
| /** stdout stream. Defaults to `process.stdout`. */ | ||
| stdout?: NodeJS.EventEmitter; | ||
| /** stderr stream. Defaults to `process.stderr`. */ | ||
| stderr?: NodeJS.EventEmitter; | ||
| /** Process exit. Defaults to `process.exit`. */ | ||
| exit?: (code: number) => void; | ||
| } | ||
|
|
||
| /** | ||
| * Guard against `EPIPE` on stdout/stderr so piping to a reader that closes | ||
| * early (`testsprite ... | head`) exits cleanly instead of crashing with an | ||
| * unhandled `write EPIPE` stack. Only `EPIPE` is swallowed; any other stream | ||
| * error is left to surface normally. | ||
| */ | ||
| export function installBrokenPipeGuard(deps: BrokenPipeDeps = {}): void { | ||
| const stdout = deps.stdout ?? process.stdout; | ||
| const stderr = deps.stderr ?? process.stderr; | ||
| const exit = deps.exit ?? ((code: number) => process.exit(code)); | ||
|
|
||
| stdout.on('error', (error: NodeJS.ErrnoException) => { | ||
| // Reader went away (`| head`, `| less` then q): exit cleanly like SIGPIPE | ||
| // rather than dumping an unhandled `write EPIPE` stack. Any other stdout | ||
| // error is a genuine, actionable failure, so re-throw it (Node's default). | ||
| if (error.code === 'EPIPE') { | ||
| exit(0); | ||
| return; | ||
| } | ||
| throw error; | ||
| }); | ||
| stderr.on('error', (error: NodeJS.ErrnoException) => { | ||
| // stderr closed: nothing can be reported over it, so swallow EPIPE. Any | ||
| // other error re-throws so a genuine failure is not silently hidden. | ||
| if (error.code === 'EPIPE') return; | ||
| throw error; | ||
| }); | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.