Skip to content

Commit 1d7c205

Browse files
authored
fix: close two silent-exit vectors around unhandled rejections (#1758)
* fix: record crash telemetry for unhandled promise rejections The crash handler only listened on uncaughtExceptionMonitor, which never fires for a rejection that has a listener — and the TUI always registers one, converting rejections into a silent exit(1) with no telemetry at all. Observe unhandledRejection directly so those crashes still leave a trace. Since registering a real listener suppresses Node's default crash-on-rejection, rethrow when we are the only listener (print / server modes), deduped so the monitor does not double-report. * fix: fall back to text paste when the clipboard image handler rejects The Ctrl+V image-paste dispatch chained off the async handler without a rejection branch, so any failure inside it became an unhandled rejection — which the CLI's crash path turns into a silent exit(1). Treat a rejection like "no image available" and paste as text. * fix: dedupe all rethrown rejection reasons at the crash monitor Non-Error rejection reasons (plain objects, strings, null) were recorded by the rejection handler but not added to the dedupe set, so the uncaughtExceptionMonitor pass after the rethrow reported a second crash for the same failure; null/undefined reasons also crashed the monitor's own error-type extraction. Use a Set so primitives dedupe by value, add every rethrown reason, and classify monitor crashes null-safely.
1 parent 918c135 commit 1d7c205

6 files changed

Lines changed: 271 additions & 10 deletions

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 CLI exiting unexpectedly when reading an image from the clipboard fails; it now falls back to pasting text.
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+
Report crash telemetry for unhandled promise rejections, so exits they cause are no longer invisible.

apps/kimi-code/src/tui/components/editor/custom-editor.ts

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -397,12 +397,21 @@ export class CustomEditor extends Editor {
397397
}
398398
if (this.onPasteImage !== undefined) {
399399
const handler = this.onPasteImage;
400-
void handler().then((handled) => {
401-
if (!handled) {
402-
this.onTextPaste?.();
403-
super.handleInput.call(this, normalized);
404-
}
405-
});
400+
const pasteAsText = (): void => {
401+
this.onTextPaste?.();
402+
super.handleInput.call(this, normalized);
403+
};
404+
void handler().then(
405+
(handled) => {
406+
if (!handled) pasteAsText();
407+
},
408+
() => {
409+
// A rejecting image-paste handler must not leak an unhandled
410+
// rejection (the CLI turns those into a silent exit) — treat it
411+
// the same as "no image available" and fall back to text paste.
412+
pasteAsText();
413+
},
414+
);
406415
return;
407416
}
408417
}

apps/kimi-code/test/tui/components/editor/custom-editor.test.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -587,6 +587,34 @@ describe('CustomEditor paste marker expansion', () => {
587587
editor.handleInput('x');
588588
expect(editor.getText()).toContain('x');
589589
});
590+
591+
it('falls back to the text paste path when the image paste handler rejects', async () => {
592+
const editor = makeEditor();
593+
const onTextPaste = vi.fn();
594+
editor.onTextPaste = onTextPaste;
595+
editor.onPasteImage = vi.fn(async () => {
596+
throw new Error('clipboard backend broken');
597+
});
598+
599+
// Regression: a rejecting onPasteImage must not leak an unhandled
600+
// rejection — the CLI's crash path turns those into a silent exit.
601+
const rejections: unknown[] = [];
602+
const onRejection = (reason: unknown): void => {
603+
rejections.push(reason);
604+
};
605+
process.on('unhandledRejection', onRejection);
606+
try {
607+
editor.handleInput(process.platform === 'win32' ? '\u001Bv' : '\u0016');
608+
await new Promise((resolve) => {
609+
setImmediate(resolve);
610+
});
611+
612+
expect(onTextPaste).toHaveBeenCalledOnce();
613+
expect(rejections).toHaveLength(0);
614+
} finally {
615+
process.off('unhandledRejection', onRejection);
616+
}
617+
});
590618
});
591619

592620
describe('CustomEditor shortcut telemetry hooks', () => {

packages/telemetry/src/crash.ts

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ let installed = false;
77
let installedUncaughtHandler:
88
| ((error: Error, origin: NodeJS.UncaughtExceptionOrigin) => void)
99
| null = null;
10+
let installedRejectionHandler: ((reason: unknown) => void) | null = null;
1011

1112
export function setCrashPhase(nextPhase: CrashPhase): void {
1213
phase = nextPhase;
@@ -17,11 +18,15 @@ export function installCrashHandlers(): () => void {
1718
}
1819

1920
export function installCrashHandlersForClient(client: TelemetryClient): () => void {
20-
if (installed && installedUncaughtHandler !== null) {
21+
if (installed && installedUncaughtHandler !== null && installedRejectionHandler !== null) {
2122
return () => {
2223
uninstallCrashHandlers();
2324
};
2425
}
26+
// Rejections recorded by the rejection handler below and then rethrown to
27+
// preserve Node's default crash; the monitor must not report them twice.
28+
// A Set (not WeakSet) so primitive reasons dedupe by value too.
29+
const recordedRejections = new Set<unknown>();
2530
const trackCrash = (errorType: string, source: string) => {
2631
try {
2732
client.track('crash', {
@@ -36,9 +41,30 @@ export function installCrashHandlersForClient(client: TelemetryClient): () => vo
3641
};
3742
installedUncaughtHandler = (error, origin) => {
3843
if (isAbortError(error)) return;
39-
trackCrash(error.name || error.constructor.name, origin);
44+
if (recordedRejections.has(error)) return;
45+
// `error` is typed Error but can be anything a rejection rethrow sent
46+
// through (primitives, null) — classify it null-safely.
47+
trackCrash(crashErrorType(error), origin);
4048
};
4149
process.on('uncaughtExceptionMonitor', installedUncaughtHandler);
50+
// `uncaughtExceptionMonitor` never fires for a rejection that has a
51+
// listener — and the TUI always registers one, converting the rejection
52+
// into a silent exit(1) with no telemetry at all. Observe rejections
53+
// directly so those crashes still leave a trace. Registering a real
54+
// listener suppresses Node's default crash-on-rejection, so when we are
55+
// the only listener (print / server modes) rethrow to preserve it; the
56+
// dedupe set above keeps the monitor from double-reporting that path.
57+
installedRejectionHandler = (reason: unknown) => {
58+
const soleListener = process.listenerCount('unhandledRejection') === 1;
59+
if (!isAbortError(reason)) {
60+
trackCrash(crashErrorType(reason), 'unhandledRejection');
61+
recordedRejections.add(reason);
62+
}
63+
if (soleListener) {
64+
throw reason;
65+
}
66+
};
67+
process.on('unhandledRejection', installedRejectionHandler);
4268
installed = true;
4369
return () => {
4470
uninstallCrashHandlers();
@@ -50,10 +76,19 @@ export function uninstallCrashHandlers(): void {
5076
if (installedUncaughtHandler !== null) {
5177
process.off('uncaughtExceptionMonitor', installedUncaughtHandler);
5278
}
79+
if (installedRejectionHandler !== null) {
80+
process.off('unhandledRejection', installedRejectionHandler);
81+
}
5382
installedUncaughtHandler = null;
83+
installedRejectionHandler = null;
5484
installed = false;
5585
}
5686

87+
function crashErrorType(reason: unknown): string {
88+
if (reason instanceof Error) return reason.name || reason.constructor.name;
89+
return typeof reason;
90+
}
91+
5792
function isAbortError(reason: unknown): boolean {
5893
return (
5994
typeof reason === 'object' &&

packages/telemetry/test/telemetry.test.ts

Lines changed: 181 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1000,7 +1000,7 @@ describe('crash handler', () => {
10001000
]);
10011001
});
10021002

1003-
it('does not register duplicate monitor listeners when installed twice', () => {
1003+
it('does not register duplicate listeners when installed twice', () => {
10041004
const client = new TelemetryClient();
10051005
const beforeUncaught = process.listenerCount('uncaughtExceptionMonitor');
10061006
const beforeRejection = process.listenerCount('unhandledRejection');
@@ -1009,13 +1009,192 @@ describe('crash handler', () => {
10091009
const uninstallSecond = installCrashHandlersForClient(client);
10101010

10111011
expect(process.listenerCount('uncaughtExceptionMonitor')).toBe(beforeUncaught + 1);
1012-
expect(process.listenerCount('unhandledRejection')).toBe(beforeRejection);
1012+
expect(process.listenerCount('unhandledRejection')).toBe(beforeRejection + 1);
10131013
uninstallSecond();
10141014
uninstallFirst();
10151015
expect(process.listenerCount('uncaughtExceptionMonitor')).toBe(beforeUncaught);
10161016
expect(process.listenerCount('unhandledRejection')).toBe(beforeRejection);
10171017
});
10181018

1019+
it('observes and records unhandled rejections when another handler owns the lifecycle', () => {
1020+
const client = new TelemetryClient();
1021+
const transport = new RecordingTransport();
1022+
client.attachSink(makeSink(transport));
1023+
setCrashPhase('runtime');
1024+
installCrashHandlersForClient(client);
1025+
// The TUI registers its own rejection handler; while one exists the
1026+
// crash handler must observe (not rethrow) so the lifecycle is untouched.
1027+
const owner = (): void => {};
1028+
process.on('unhandledRejection', owner);
1029+
try {
1030+
(process.emit as (event: string, ...args: unknown[]) => boolean)(
1031+
'unhandledRejection',
1032+
new TypeError('promise failed'),
1033+
Promise.resolve(),
1034+
);
1035+
} finally {
1036+
process.off('unhandledRejection', owner);
1037+
}
1038+
1039+
expect(transport.saved[0]?.[0]).toMatchObject({
1040+
event: 'crash',
1041+
properties: {
1042+
error_type: 'TypeError',
1043+
where: 'runtime',
1044+
source: 'unhandledRejection',
1045+
},
1046+
});
1047+
});
1048+
1049+
it('ignores aborted-operation rejections while observing', () => {
1050+
const client = new TelemetryClient();
1051+
const transport = new RecordingTransport();
1052+
client.attachSink(makeSink(transport));
1053+
installCrashHandlersForClient(client);
1054+
const owner = (): void => {};
1055+
process.on('unhandledRejection', owner);
1056+
try {
1057+
(process.emit as (event: string, ...args: unknown[]) => boolean)(
1058+
'unhandledRejection',
1059+
new DOMException('The operation was aborted.', 'AbortError'),
1060+
Promise.resolve(),
1061+
);
1062+
} finally {
1063+
process.off('unhandledRejection', owner);
1064+
}
1065+
1066+
expect(transport.saved).toHaveLength(0);
1067+
});
1068+
1069+
it('rethrows when it is the only rejection listener, recording the crash exactly once', () => {
1070+
const client = new TelemetryClient();
1071+
const transport = new RecordingTransport();
1072+
client.attachSink(makeSink(transport));
1073+
setCrashPhase('runtime');
1074+
// Vitest keeps its own rejection listeners; temporarily drop every
1075+
// listener so the crash handler is the sole one, as in print/server mode.
1076+
const others = process.listeners('unhandledRejection');
1077+
process.removeAllListeners('unhandledRejection');
1078+
installCrashHandlersForClient(client);
1079+
const reason = new TypeError('promise failed');
1080+
try {
1081+
expect(() =>
1082+
(process.emit as (event: string, ...args: unknown[]) => boolean)(
1083+
'unhandledRejection',
1084+
reason,
1085+
Promise.resolve(),
1086+
),
1087+
).toThrow(reason);
1088+
1089+
// Tracked once as a rejection; when the rethrow later surfaces at the
1090+
// uncaughtException monitor it must not be reported a second time.
1091+
expect(transport.saved[0]?.[0]).toMatchObject({
1092+
event: 'crash',
1093+
properties: {
1094+
error_type: 'TypeError',
1095+
where: 'runtime',
1096+
source: 'unhandledRejection',
1097+
},
1098+
});
1099+
emitCrash(reason);
1100+
expect(transport.saved).toHaveLength(1);
1101+
} finally {
1102+
uninstallCrashHandlers();
1103+
for (const listener of others) {
1104+
process.on('unhandledRejection', listener as (...args: unknown[]) => void);
1105+
}
1106+
}
1107+
});
1108+
1109+
it('dedupes rethrown non-Error rejection reasons at the uncaught monitor', () => {
1110+
const client = new TelemetryClient();
1111+
const transport = new RecordingTransport();
1112+
client.attachSink(makeSink(transport));
1113+
setCrashPhase('runtime');
1114+
const others = process.listeners('unhandledRejection');
1115+
process.removeAllListeners('unhandledRejection');
1116+
installCrashHandlersForClient(client);
1117+
const reason = { code: 'E' };
1118+
try {
1119+
let caught: unknown;
1120+
try {
1121+
(process.emit as (event: string, ...args: unknown[]) => boolean)(
1122+
'unhandledRejection',
1123+
reason,
1124+
Promise.resolve(),
1125+
);
1126+
} catch (error) {
1127+
caught = error;
1128+
}
1129+
expect(caught).toBe(reason);
1130+
1131+
// The plain-object reason is rethrown through the monitor; it must be
1132+
// deduped there, not reported as a second crash.
1133+
(process.emit as (event: string, ...args: unknown[]) => boolean)(
1134+
'uncaughtExceptionMonitor',
1135+
reason,
1136+
'uncaughtException',
1137+
);
1138+
expect(transport.saved).toHaveLength(1);
1139+
expect(transport.saved[0]?.[0]).toMatchObject({
1140+
event: 'crash',
1141+
properties: {
1142+
error_type: 'object',
1143+
where: 'runtime',
1144+
source: 'unhandledRejection',
1145+
},
1146+
});
1147+
} finally {
1148+
uninstallCrashHandlers();
1149+
for (const listener of others) {
1150+
process.on('unhandledRejection', listener as (...args: unknown[]) => void);
1151+
}
1152+
}
1153+
});
1154+
1155+
it('dedupes null rejection reasons and classifies monitor crashes null-safely', () => {
1156+
const client = new TelemetryClient();
1157+
const transport = new RecordingTransport();
1158+
client.attachSink(makeSink(transport));
1159+
setCrashPhase('runtime');
1160+
const others = process.listeners('unhandledRejection');
1161+
process.removeAllListeners('unhandledRejection');
1162+
installCrashHandlersForClient(client);
1163+
try {
1164+
let caught: unknown = 'not-thrown';
1165+
try {
1166+
(process.emit as (event: string, ...args: unknown[]) => boolean)(
1167+
'unhandledRejection',
1168+
null,
1169+
Promise.resolve(),
1170+
);
1171+
} catch (error) {
1172+
caught = error;
1173+
}
1174+
expect(caught).toBe(null);
1175+
1176+
// The rethrown null reaches the monitor: deduped, and the error-type
1177+
// classification must not itself throw on null/undefined.
1178+
expect(() => {
1179+
emitCrash(null as unknown as Error);
1180+
}).not.toThrow();
1181+
expect(transport.saved).toHaveLength(1);
1182+
expect(transport.saved[0]?.[0]).toMatchObject({
1183+
event: 'crash',
1184+
properties: {
1185+
error_type: 'object',
1186+
where: 'runtime',
1187+
source: 'unhandledRejection',
1188+
},
1189+
});
1190+
} finally {
1191+
uninstallCrashHandlers();
1192+
for (const listener of others) {
1193+
process.on('unhandledRejection', listener as (...args: unknown[]) => void);
1194+
}
1195+
}
1196+
});
1197+
10191198
it('ignores aborted-operation errors', () => {
10201199
const client = new TelemetryClient();
10211200
const transport = new RecordingTransport();

0 commit comments

Comments
 (0)