diff --git a/src/relay/fs-handler-git-fallback.ts b/src/relay/fs-handler-git-fallback.ts index 2d8cf384633..246199a8dfd 100644 --- a/src/relay/fs-handler-git-fallback.ts +++ b/src/relay/fs-handler-git-fallback.ts @@ -38,6 +38,11 @@ export function listFilesWithGit( ): Promise { const files = new Set() const { primary, ignoredPass } = buildGitLsFilesArgsForQuickOpen(excludePathPrefixes) + const children: { + child: ReturnType + isDone: () => boolean + reject: (error: Error) => void + }[] = [] const runGitLsFiles = (args: string[]): Promise => { return new Promise((resolve, reject) => { @@ -64,8 +69,41 @@ export function listFilesWithGit( env: buildRelayCommandEnv(), stdio: ['ignore', 'pipe', 'pipe'] }) - child.stdout!.setEncoding('utf-8') - child.stdout!.on('data', (chunk: string) => { + let timer: ReturnType | null = null + const cleanup = (): void => { + if (timer) { + clearTimeout(timer) + timer = null + } + child.stdout!.off('data', handleStdoutData) + child.stderr!.off('data', handleStderrData) + child.off('error', handleError) + child.off('close', handleClose) + } + const rejectPass = (error: Error): void => { + if (done) { + return + } + done = true + buf = '' + cleanup() + reject(error) + } + const resolvePass = (): void => { + if (done) { + return + } + done = true + cleanup() + resolve() + } + children.push({ + child, + isDone: () => done, + reject: rejectPass + }) + + function handleStdoutData(chunk: string): void { buf += chunk let start = 0 let idx = buf.indexOf('\n', start) @@ -75,53 +113,62 @@ export function listFilesWithGit( idx = buf.indexOf('\n', start) } buf = start < buf.length ? buf.substring(start) : '' - }) - child.stderr!.on('data', () => { + } + function handleStderrData(): void { /* drain */ - }) - child.once('error', (err) => { + } + function handleError(err: Error): void { + rejectPass(err) + } + function handleClose(_code: number | null, signal: NodeJS.Signals | null): void { if (done) { return } - done = true - clearTimeout(timer) - buf = '' - reject(err) - }) - child.once('close', (_code, signal) => { - if (done) { - return - } - done = true - clearTimeout(timer) if (signal) { // Why: a signal exit means the child was killed (timeout or // external). Treat that as a load failure rather than silently // resolving with whatever git had managed to print. - buf = '' - reject(new Error(`git ls-files killed by ${signal}`)) + rejectPass(new Error(`git ls-files killed by ${signal}`)) return } if (buf) { processLine(buf) } - resolve() - }) - const timer = setTimeout(() => { - if (done) { - return - } - done = true - buf = '' + resolvePass() + } + + child.stdout!.setEncoding('utf-8') + child.stdout!.on('data', handleStdoutData) + child.stderr!.on('data', handleStderrData) + child.once('error', handleError) + child.once('close', handleClose) + timer = setTimeout(() => { child.kill() - reject(new Error('git ls-files timed out')) + rejectPass(new Error('git ls-files timed out')) }, 10_000) }) } - return Promise.all([runGitLsFiles(primary), runGitLsFiles(ignoredPass)]).then(() => - Array.from(files) - ) + const killSurvivors = (): void => { + // Why: Promise.all returns after the first failed pass, but the sibling + // git process can keep streaming on SSH unless we cancel it explicitly. + for (const entry of children) { + if (entry.isDone()) { + continue + } + if (entry.child.exitCode === null && entry.child.signalCode === null) { + entry.child.kill() + } + entry.reject(new Error('git ls-files canceled after sibling failure')) + } + } + + return Promise.all([runGitLsFiles(primary), runGitLsFiles(ignoredPass)]) + .then(() => Array.from(files)) + .catch((err) => { + killSurvivors() + throw err + }) } /** diff --git a/src/relay/fs-handler-list-files-ignored.test.ts b/src/relay/fs-handler-list-files-ignored.test.ts index fea487a4913..0a859f8c928 100644 --- a/src/relay/fs-handler-list-files-ignored.test.ts +++ b/src/relay/fs-handler-list-files-ignored.test.ts @@ -154,6 +154,14 @@ describe('relay quick open ignored file listing', () => { expect(outcome).toContain('git ls-files timed out') expect(primaryProc.kill).toHaveBeenCalled() expect(ignoredProc.kill).toHaveBeenCalled() + expect((primaryProc.stdout as unknown as EventEmitter).listenerCount('data')).toBe(0) + expect((primaryProc.stderr as unknown as EventEmitter).listenerCount('data')).toBe(0) + expect(primaryProc.listenerCount('error')).toBe(0) + expect(primaryProc.listenerCount('close')).toBe(0) + expect((ignoredProc.stdout as unknown as EventEmitter).listenerCount('data')).toBe(0) + expect((ignoredProc.stderr as unknown as EventEmitter).listenerCount('data')).toBe(0) + expect(ignoredProc.listenerCount('error')).toBe(0) + expect(ignoredProc.listenerCount('close')).toBe(0) } finally { vi.useRealTimers() }