Skip to content

Commit f0c8a10

Browse files
authored
fix: preserve the crash error in diagnostic logs on unexpected exit (#1757)
The TUI crash path (uncaughtException / unhandledRejection) logged the error into an asynchronously-drained sink and then called process.exit() on the same tick, so the one line explaining the crash was never written to disk. Flush the diagnostic logs synchronously before exiting.
1 parent df75a0f commit f0c8a10

9 files changed

Lines changed: 150 additions & 1 deletion

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@moonshot-ai/kimi-code": patch
3+
---
4+
5+
Fix the diagnostic log missing the actual error when the CLI exits unexpectedly.

apps/kimi-code/src/cli/run-shell.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { join } from 'node:path';
44

55
import {
66
createKimiHarness,
7+
flushDiagnosticLogsSync,
78
log,
89
type KimiHarness,
910
type TelemetryClient,
@@ -159,6 +160,14 @@ export async function runShell(
159160
// raw mode with a hidden cursor and XON/XOFF flow control disabled. Restore
160161
// both before exiting so the user's shell is usable afterwards.
161162
const emergencyExit = (exitCode: number): void => {
163+
// The crash log above is only enqueued into the async sink; flush it
164+
// synchronously or the `process.exit()` below would drop the one line that
165+
// explains why we crashed. Best-effort: an exit path must never throw.
166+
try {
167+
flushDiagnosticLogsSync();
168+
} catch {
169+
/* ignore */
170+
}
162171
restoreTerminalModes();
163172
restoreStty();
164173
process.exit(exitCode);

apps/kimi-code/test/cli/run-shell.test.ts

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ const mocks = vi.hoisted(() => {
5858
track: lifecycleTrack,
5959
})),
6060
resolveKimiHome: vi.fn((homeDir?: string) => homeDir ?? '/tmp/kimi-code-test-home'),
61+
flushDiagnosticLogsSync: vi.fn(),
6162
harnessCreatesDeviceIdOnConstruction: false,
6263
execSync: vi.fn(),
6364
TuiConfigParseError,
@@ -69,6 +70,7 @@ vi.mock('@moonshot-ai/kimi-code-sdk', async (importOriginal) => {
6970
return {
7071
...actual,
7172
resolveKimiHome: mocks.resolveKimiHome,
73+
flushDiagnosticLogsSync: mocks.flushDiagnosticLogsSync,
7274
createKimiHarness: (...args: unknown[]) => {
7375
const options = args[0] as { readonly homeDir?: string } | undefined;
7476
const homeDir = options?.homeDir ?? '/tmp/kimi-code-test-home';
@@ -508,6 +510,101 @@ describe('runShell', () => {
508510
});
509511
});
510512

513+
it('flushes diagnostic logs synchronously before exiting on a runtime crash', async () => {
514+
mocks.loadTuiConfig.mockResolvedValue({
515+
theme: 'dark',
516+
editorCommand: null,
517+
notifications: { enabled: true, condition: 'unfocused' },
518+
});
519+
mocks.tuiStart.mockResolvedValue(undefined);
520+
521+
const processOnSpy = vi.spyOn(process, 'on');
522+
const stdout = captureProcessWrite('stdout');
523+
const exitSpy = mockProcessExit();
524+
525+
try {
526+
await runShell(
527+
{
528+
session: undefined,
529+
continue: false,
530+
yolo: false,
531+
auto: false,
532+
plan: false,
533+
model: undefined,
534+
outputFormat: undefined,
535+
prompt: undefined,
536+
skillsDirs: [],
537+
},
538+
'1.2.3-test',
539+
);
540+
541+
const handler = processOnSpy.mock.calls.find(
542+
([event]) => event === 'uncaughtException',
543+
)?.[1] as ((error: unknown) => void) | undefined;
544+
expect(handler).toBeDefined();
545+
546+
// The async log sink cannot flush before process.exit() runs, so the
547+
// crash handler must force a synchronous flush or the crash reason is
548+
// lost (regression: uncaughtException logs never reached disk).
549+
expect(() => handler?.(new Error('boom'))).toThrow(ExitCalled);
550+
expect(mocks.flushDiagnosticLogsSync).toHaveBeenCalledOnce();
551+
expect(exitSpy).toHaveBeenCalledWith(1);
552+
expect(mocks.flushDiagnosticLogsSync.mock.invocationCallOrder[0]!).toBeLessThan(
553+
exitSpy.mock.invocationCallOrder[0]!,
554+
);
555+
} finally {
556+
processOnSpy.mockRestore();
557+
exitSpy.mockRestore();
558+
stdout.restore();
559+
}
560+
});
561+
562+
it('flushes diagnostic logs synchronously before exiting on an unhandled rejection', async () => {
563+
mocks.loadTuiConfig.mockResolvedValue({
564+
theme: 'dark',
565+
editorCommand: null,
566+
notifications: { enabled: true, condition: 'unfocused' },
567+
});
568+
mocks.tuiStart.mockResolvedValue(undefined);
569+
570+
const processOnSpy = vi.spyOn(process, 'on');
571+
const stdout = captureProcessWrite('stdout');
572+
const exitSpy = mockProcessExit();
573+
574+
try {
575+
await runShell(
576+
{
577+
session: undefined,
578+
continue: false,
579+
yolo: false,
580+
auto: false,
581+
plan: false,
582+
model: undefined,
583+
outputFormat: undefined,
584+
prompt: undefined,
585+
skillsDirs: [],
586+
},
587+
'1.2.3-test',
588+
);
589+
590+
const handler = processOnSpy.mock.calls.find(
591+
([event]) => event === 'unhandledRejection',
592+
)?.[1] as ((reason: unknown) => void) | undefined;
593+
expect(handler).toBeDefined();
594+
595+
expect(() => handler?.(new Error('boom'))).toThrow(ExitCalled);
596+
expect(mocks.flushDiagnosticLogsSync).toHaveBeenCalledOnce();
597+
expect(exitSpy).toHaveBeenCalledWith(1);
598+
expect(mocks.flushDiagnosticLogsSync.mock.invocationCallOrder[0]!).toBeLessThan(
599+
exitSpy.mock.invocationCallOrder[0]!,
600+
);
601+
} finally {
602+
processOnSpy.mockRestore();
603+
exitSpy.mockRestore();
604+
stdout.restore();
605+
}
606+
});
607+
511608
it('closes the harness when TUI startup fails', async () => {
512609
mocks.loadTuiConfig.mockResolvedValue({
513610
theme: 'dark',

apps/kimi-code/test/helpers/process.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ export class ExitCalled extends Error {
66
}
77
}
88

9-
export function mockProcessExit(): { mockRestore(): void } {
9+
export function mockProcessExit() {
1010
return vi.spyOn(process, 'exit').mockImplementation(((code?: string | number | null) => {
1111
throw new ExitCalled(Number(code ?? 0));
1212
}) as never);

packages/agent-core/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ export * from './plugin';
1010
export { buildReplay } from './agent/replay/build';
1111
export {
1212
flushDiagnosticLogs,
13+
flushDiagnosticLogsSync,
1314
getRootLogger,
1415
log,
1516
redact,

packages/agent-core/src/logging/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ export { LOG_LEVEL_RANK, levelEnabled } from './types';
1414
export {
1515
__resetRootLoggerForTest,
1616
flushDiagnosticLogs,
17+
flushDiagnosticLogsSync,
1718
getRootLogger,
1819
log,
1920
redact,

packages/agent-core/src/logging/logger.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,16 @@ export function flushDiagnosticLogs(): Promise<boolean> {
276276
return getRootInternal().flush();
277277
}
278278

279+
/**
280+
* Synchronous variant for crash / emergency-exit paths that call
281+
* `process.exit()` on the same tick: pending entries are appended with
282+
* `appendFileSync`, so they survive the immediate exit that would otherwise
283+
* drop everything still sitting in the async queue.
284+
*/
285+
export function flushDiagnosticLogsSync(): void {
286+
getRootInternal().flushSync();
287+
}
288+
279289
class LoggerImpl implements Logger {
280290
constructor(private readonly boundCtx: LogContext) {}
281291

packages/agent-core/test/logging/logger.test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
import { mkdtemp, readFile, rm } from 'node:fs/promises';
2+
import { readFileSync } from 'node:fs';
23
import { tmpdir } from 'node:os';
34
import { join } from 'pathe';
45

56
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
67

78
import {
89
__resetRootLoggerForTest,
10+
flushDiagnosticLogsSync,
911
getRootLogger,
1012
log,
1113
redact,
@@ -429,6 +431,29 @@ describe('session routing', () => {
429431
});
430432
});
431433

434+
describe('flushDiagnosticLogsSync', () => {
435+
it('persists enqueued entries synchronously, before the async drain could run', async () => {
436+
await getRootLogger().configure(defaultConfig());
437+
log.error('crash marker', { error: new Error('boom') });
438+
439+
// No `await` between enqueue and flush: the async drain (microtask + async
440+
// fs) cannot have written yet, so only the synchronous append can produce
441+
// the file content below. This mirrors crash paths that call
442+
// process.exit() on the same tick.
443+
flushDiagnosticLogsSync();
444+
445+
const content = readFileSync(resolveGlobalLogPath(homeDir), 'utf-8');
446+
expect(content).toContain('crash marker');
447+
expect(content).toContain('boom');
448+
});
449+
450+
it('is a silent no-op before configure', () => {
451+
expect(() => {
452+
flushDiagnosticLogsSync();
453+
}).not.toThrow();
454+
});
455+
});
456+
432457
describe('redact helper', () => {
433458
it('returns same shape with sensitive fields replaced', () => {
434459
const out = redact({ user: 'x', token: 'abc', nested: { apiKey: '1' } });

packages/node-sdk/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ export {
5252
// RootLogger / getRootLogger / LoggingConfig stay inside agent-core.
5353
export {
5454
flushDiagnosticLogs,
55+
flushDiagnosticLogsSync,
5556
log,
5657
redact,
5758
resolveGlobalLogPath,

0 commit comments

Comments
 (0)