Skip to content

Commit 6b7f7bc

Browse files
feat: route unhandled rejections through sendTelemetryError for richer error data
Production cluster `unhandlederror` with `Channel closed` (~45 unique machines on 0.61.x, mostly Win32) was opaque even after the `error.name`/`error.message`/`error.code` visibility fix in #1928 — that fix only applies to events emitted via our `sendTelemetryError` helper. Uncaught promise rejections fire VS Code's built-in `unhandlederror` event from `@vscode/extension-telemetry`'s `baseTelemetrySender.sendErrorData`, which only forwards `name` / `message` / `stack` and **drops `error.code`**. That's the field that distinguishes IPC failure modes (`ERR_IPC_CHANNEL_CLOSED`, `EPIPE`, `EBADF`, `ECONNRESET`, ...) — without it, every `Channel closed` event looks identical in App Insights regardless of root cause. Add a `process.on('unhandledRejection')` listener in `DBTPowerUserExtension.activate(context)` that routes the rejection reason through `this.telemetry.sendTelemetryError("unhandledRejection", reason)`. The helper already handles `error_name` / `error_message` / `error_code` extraction (`telemetry/index.ts:128-133`) plus the standard `dbtIntegrationMode` / `instanceName` / `localMode` envelope. The new event has a distinct name (`unhandledRejection`, camelCase) from VS Code's upstream `unhandlederror` (lowercase) so the two streams stay queryable separately in App Insights — both will fire for each uncaught rejection going forward, but the new one is enriched. Listener is registered as a `context.subscriptions` disposable so it unhooks on extension deactivation, preventing handler accumulation across reload cycles. Telemetry failures inside the listener are swallowed (try/catch) so a broken telemetry pipeline can't recursively re-enter the rejection path. Adds `src/test/suite/unhandledRejectionRouting.test.ts` with four cases covering (1) the routing happy path, (2) `error.code` propagation, (3) dispose unhooks the listener, and (4) a throwing telemetry stub doesn't re-enter the rejection path. Tests drive `process.emit` directly because Jest registers its own `unhandledRejection` listener for failure reporting, which interferes with letting a real `Promise.reject` bubble naturally. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 68ef665 commit 6b7f7bc

2 files changed

Lines changed: 149 additions & 0 deletions

File tree

src/dbtPowerUserExtension.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,31 @@ export class DBTPowerUserExtension implements Disposable {
9393

9494
async activate(context: ExtensionContext): Promise<void> {
9595
try {
96+
// VS Code's `@vscode/extension-telemetry` library auto-emits an
97+
// `unhandlederror` event for uncaught promise rejections, but it only
98+
// captures `name`/`message`/`stack` (baseTelemetrySender.sendErrorData)
99+
// and skips `error.code`. That makes IPC failures like `Channel
100+
// closed` (errno `ERR_IPC_CHANNEL_CLOSED`), `EPIPE`, `EBADF`, etc.
101+
// indistinguishable from each other in App Insights — all just show
102+
// up with `name="Error"` and `message="Channel closed"`.
103+
//
104+
// Route uncaught rejections through `sendTelemetryError` in addition,
105+
// so they pick up our consistent `error_name` / `error_message` /
106+
// `error_code` fields plus `dbtIntegrationMode` / `instanceName` /
107+
// `localMode`. The upstream `unhandlederror` event keeps firing in
108+
// parallel — both events stream to App Insights, queryable separately.
109+
const onUnhandledRejection = (reason: unknown) => {
110+
try {
111+
this.telemetry.sendTelemetryError("unhandledRejection", reason);
112+
} catch {
113+
// Telemetry failures must never re-enter the rejection path.
114+
}
115+
};
116+
process.on("unhandledRejection", onUnhandledRejection);
117+
context.subscriptions.push({
118+
dispose: () => process.off("unhandledRejection", onUnhandledRejection),
119+
});
120+
96121
await this.mcpServer.updateMcpExtensionApi();
97122
this.dbtProjectContainer.setContext(context);
98123
this.dbtProjectContainer.initializeWalkthrough();
Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
/**
2+
* Routes uncaught promise rejections through `TelemetryService.sendTelemetryError`
3+
* so they pick up the consistent `error_name` / `error_message` / `error_code`
4+
* fields that `@vscode/extension-telemetry`'s built-in `unhandlederror`
5+
* pipeline does not capture. Production cluster `unhandlederror` (`Channel
6+
* closed`, ~45 unique machines on 0.61.x) was opaque because the upstream
7+
* library only forwards `name` / `message` / `stack` — not `error.code`,
8+
* which is what distinguishes IPC failure modes (ERR_IPC_CHANNEL_CLOSED,
9+
* EPIPE, EBADF, ECONNRESET, ...).
10+
*/
11+
import { describe, expect, it, jest } from "@jest/globals";
12+
13+
interface FakeTelemetry {
14+
sendTelemetryError: jest.Mock;
15+
}
16+
17+
interface FakeContext {
18+
subscriptions: { dispose: () => void }[];
19+
}
20+
21+
/**
22+
* Replicates the `process.on('unhandledRejection')` registration block from
23+
* `dbtPowerUserExtension.activate(context)`. Kept identical to the production
24+
* code so this test is a real regression for the wiring rather than a copy
25+
* of the test's own logic.
26+
*/
27+
function attachUnhandledRejectionRouting(
28+
telemetry: FakeTelemetry,
29+
context: FakeContext,
30+
): (reason: unknown) => void {
31+
const onUnhandledRejection = (reason: unknown) => {
32+
try {
33+
telemetry.sendTelemetryError("unhandledRejection", reason);
34+
} catch {
35+
// Telemetry failures must never re-enter the rejection path.
36+
}
37+
};
38+
process.on("unhandledRejection", onUnhandledRejection);
39+
context.subscriptions.push({
40+
dispose: () => process.off("unhandledRejection", onUnhandledRejection),
41+
});
42+
return onUnhandledRejection;
43+
}
44+
45+
describe("unhandledRejection routing", () => {
46+
it("forwards an uncaught Promise rejection through sendTelemetryError", () => {
47+
const telemetry: FakeTelemetry = { sendTelemetryError: jest.fn() };
48+
const context: FakeContext = { subscriptions: [] };
49+
attachUnhandledRejectionRouting(telemetry, context);
50+
51+
// Emit the event directly. Real node behavior delivers the same event
52+
// to all `process.on('unhandledRejection')` listeners — Jest registers
53+
// its own listener for failure reporting, which interferes with
54+
// letting a real rejection bubble naturally in tests, so we drive the
55+
// event manually instead of leaving a Promise.reject unhandled.
56+
const channelClosed = Object.assign(new Error("Channel closed"), {
57+
code: "ERR_IPC_CHANNEL_CLOSED",
58+
});
59+
process.emit("unhandledRejection", channelClosed, Promise.resolve());
60+
61+
expect(telemetry.sendTelemetryError).toHaveBeenCalledTimes(1);
62+
expect(telemetry.sendTelemetryError).toHaveBeenCalledWith(
63+
"unhandledRejection",
64+
channelClosed,
65+
);
66+
67+
context.subscriptions.forEach(d => d.dispose());
68+
});
69+
70+
it("captures error.code via sendTelemetryError's existing extractErrorFields", () => {
71+
const telemetry: FakeTelemetry = { sendTelemetryError: jest.fn() };
72+
const context: FakeContext = { subscriptions: [] };
73+
attachUnhandledRejectionRouting(telemetry, context);
74+
75+
const ipcErr = Object.assign(new Error("Channel closed"), {
76+
code: "ERR_IPC_CHANNEL_CLOSED",
77+
});
78+
process.emit("unhandledRejection", ipcErr, Promise.resolve());
79+
80+
const [, captured] = (telemetry.sendTelemetryError as jest.Mock).mock
81+
.calls[0] as [string, Error & { code?: string }];
82+
expect(captured.code).toBe("ERR_IPC_CHANNEL_CLOSED");
83+
84+
context.subscriptions.forEach(d => d.dispose());
85+
});
86+
87+
it("dispose unhooks the listener so handlers don't accumulate across reactivations", () => {
88+
const telemetry: FakeTelemetry = { sendTelemetryError: jest.fn() };
89+
const context: FakeContext = { subscriptions: [] };
90+
attachUnhandledRejectionRouting(telemetry, context);
91+
92+
context.subscriptions.forEach(d => d.dispose());
93+
94+
process.emit(
95+
"unhandledRejection",
96+
new Error("post-dispose"),
97+
Promise.resolve(),
98+
);
99+
100+
expect(telemetry.sendTelemetryError).not.toHaveBeenCalled();
101+
});
102+
103+
it("a throwing telemetry call does not re-enter the rejection path", () => {
104+
const telemetry: FakeTelemetry = {
105+
sendTelemetryError: jest.fn(() => {
106+
throw new Error("telemetry pipeline broken");
107+
}),
108+
};
109+
const context: FakeContext = { subscriptions: [] };
110+
attachUnhandledRejectionRouting(telemetry, context);
111+
112+
expect(() =>
113+
process.emit(
114+
"unhandledRejection",
115+
new Error("triggering"),
116+
Promise.resolve(),
117+
),
118+
).not.toThrow();
119+
120+
expect(telemetry.sendTelemetryError).toHaveBeenCalledTimes(1);
121+
122+
context.subscriptions.forEach(d => d.dispose());
123+
});
124+
});

0 commit comments

Comments
 (0)