Skip to content

Commit 8fe17c9

Browse files
committed
fix(telemetry): ensure queued events are preserved on dispatcher initialization failure
1 parent fcefbec commit 8fe17c9

4 files changed

Lines changed: 60 additions & 4 deletions

File tree

‎packages/core/lib/remote-telemetry/tracker.cjs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1117,7 +1117,6 @@ module.exports = (function (e) {
11171117
if (signal?.aborted) return;
11181118
// DNS follows the cancellable fetch; a separate preflight could outlive
11191119
// the CLI's flush budget and also bypass an explicitly configured proxy.
1120-
const payloads = [...this._offlineQueue.splice(0), payload];
11211120
// Undici request cancellation alone does not own an unfinished connector.
11221121
// Only telemetry gets this private dispatcher, including its proxy hops.
11231122
let dispatcher;
@@ -1138,6 +1137,7 @@ module.exports = (function (e) {
11381137
})
11391138
: new Agent({ connect });
11401139
}
1140+
const payloads = [...this._offlineQueue.splice(0), payload];
11411141
try {
11421142
await Promise.all(
11431143
payloads.map(async (eventPayload) => {

‎packages/core/tests/telemetry-send.test.ts‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,38 @@ test("an aborted send neither opens a request nor replays queued events", async
5252
expect(queued._offlineQueue).toEqual(["queued"]);
5353
});
5454

55+
test.each(["HTTP_PROXY", "HTTPS_PROXY"])(
56+
"dispatcher initialization failure preserves queued events (%s)",
57+
async (proxyVariable) => {
58+
vi.stubEnv("HTTP_PROXY", "");
59+
vi.stubEnv("HTTPS_PROXY", "");
60+
vi.stubEnv(proxyVariable, "not-a-url");
61+
const tracker = new Tracker({ pid: "fixture-pid" });
62+
const queued = tracker as unknown as { _offlineQueue: string[] };
63+
queued._offlineQueue.push("old event");
64+
const controller = new AbortController();
65+
try {
66+
await expect(tracker.send("new event", { signal: controller.signal })).rejects.toMatchObject({
67+
code: "ERR_INVALID_URL",
68+
});
69+
expect(fetchSpy).not.toHaveBeenCalled();
70+
expect(queued._offlineQueue).toEqual(["old event"]);
71+
72+
vi.stubEnv(proxyVariable, "");
73+
await tracker.send("new event", { signal: controller.signal });
74+
expect(fetchSpy.mock.calls.map(([, options]) => JSON.parse(options?.body as string))).toEqual(
75+
[
76+
{ gokey: "old%20event", gmkey: "EXP" },
77+
{ gokey: "new%20event", gmkey: "EXP" },
78+
],
79+
);
80+
expect(queued._offlineQueue).toEqual([]);
81+
} finally {
82+
vi.unstubAllEnvs();
83+
}
84+
},
85+
);
86+
5587
test("queued retries share the current cancellation signal and retain event encoding", async () => {
5688
const tracker = new Tracker({ pid: "fixture-pid" });
5789
(tracker as unknown as { _offlineQueue: string[] })._offlineQueue.push("old event");

‎packages/runtime/tests/harness/telemetry-exit-main.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import dns from "node:dns";
22
import { createRequire } from "node:module";
3+
import { performance } from "node:perf_hooks";
34
import { httpTransport } from "../../../core/src/client/transport.ts";
45

56
const [mode, origin, dnsServer, tlsOrigin] = process.argv.slice(2);
@@ -36,13 +37,15 @@ if (process.versions.bun) {
3637
}
3738

3839
const { createCli } = await import("../../src/create-cli.ts");
40+
let outputCompletedAt = 0;
3941
await createCli(
4042
{
4143
probe: {
4244
description: "Telemetry exit fixture",
4345
auth: "none",
4446
async run(context) {
4547
context.output.result({ ok: true });
48+
outputCompletedAt = performance.now();
4649
},
4750
},
4851
},
@@ -53,7 +56,8 @@ await createCli(
5356
npmPackage: "telemetry-fixture",
5457
},
5558
).run(["probe", "--output", "json"]);
59+
const outputToRunMs = performance.now() - outputCompletedAt;
5660

5761
// Inspect closure while this process is still alive: exit must not hide a leak.
5862
const status = await directFetch(new URL("/status", origin));
59-
process.stderr.write(JSON.stringify(await status.json()) + "\n");
63+
process.stderr.write(JSON.stringify({ ...(await status.json()), outputToRunMs }) + "\n");

‎packages/runtime/tests/telemetry-exit.e2e.test.ts‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { createServer as createTcpServer, type Socket } from "node:net";
66
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
77
import { tmpdir } from "node:os";
88
import { dirname, join } from "node:path";
9+
import { performance } from "node:perf_hooks";
910
import { fileURLToPath } from "node:url";
1011
import { promisify } from "node:util";
1112
import { buildIsolatedCliEnv } from "e2e/isolation";
@@ -128,7 +129,7 @@ describe.each(["node", ...(hasBun ? ["bun"] : [])])(
128129
});
129130
try {
130131
// runNodeMain forces DO_NOT_TRACK=1; this test intentionally exercises telemetry.
131-
const { stdout, stderr } = await execFileAsync(
132+
const execution = execFileAsync(
132133
runtime === "node" ? process.execPath : bun,
133134
[
134135
...(runtime === "node" ? ["--import", import.meta.resolve("tsx")] : []),
@@ -140,8 +141,27 @@ describe.each(["node", ...(hasBun ? ["bun"] : [])])(
140141
],
141142
{ cwd: runtimeRoot, env: isolated.env, timeout: 8_000, maxBuffer: 1024 * 1024 },
142143
);
144+
let outputBuffer = "";
145+
let outputCompletedAt: number | undefined;
146+
execution.child.stdout?.on("data", (chunk: string) => {
147+
outputBuffer += chunk;
148+
if (outputCompletedAt === undefined && outputBuffer.trimEnd().endsWith("}")) {
149+
outputCompletedAt = performance.now();
150+
}
151+
});
152+
const { stdout, stderr } = await execution;
153+
const exitedAt = performance.now();
143154
expect(JSON.parse(stdout)).toEqual({ ok: true });
144-
expect(JSON.parse(stderr)).toEqual({ requests: disabled ? 0 : 1, closed: !disabled });
155+
const { outputToRunMs, ...connectionStatus } = JSON.parse(stderr);
156+
expect(connectionStatus).toEqual({ requests: disabled ? 0 : 1, closed: !disabled });
157+
if (["fast", "headers", "body"].includes(mode)) {
158+
expect(outputToRunMs).toBeGreaterThanOrEqual(0);
159+
expect(outputToRunMs, "output completion to run() return").toBeLessThan(3_000);
160+
expect(outputCompletedAt).toBeDefined();
161+
expect(exitedAt - outputCompletedAt!, "output completion to process exit").toBeLessThan(
162+
3_000,
163+
);
164+
}
145165
expect(dnsQueries).toBe(0);
146166
if (!disabled && !mode.startsWith("tls")) {
147167
expect(requests[0]?.method).toBe("POST");

0 commit comments

Comments
 (0)