Skip to content

Commit 369f8fb

Browse files
committed
Discard queued telemetry events when the user opts out
Dropping the singleton on opt-out left the outgoing instance's batch timer armed, so events captured before the toggle would still reach the network afterwards. Opting out speaks to activity already generated, not only to activity still to come.
1 parent fa1e3f5 commit 369f8fb

5 files changed

Lines changed: 77 additions & 7 deletions

File tree

docs/TELEMETRY.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,12 @@ Any of the following disables telemetry entirely:
4646
- `CORBITS_TELEMETRY` set to any falsy value: `0`, `false`, `off`, `no`, or empty
4747
- `DO_NOT_TRACK=1` (the standard [Console Do Not Track](https://consoledonottrack.com/) convention)
4848

49+
Turning telemetry off also discards whatever is still queued and unsent.
50+
Events captured earlier in the session but not yet transmitted are thrown
51+
away at the moment you opt out, not sent on the way out — opting out covers
52+
the activity you have already generated, not just the activity still to
53+
come.
54+
4955
Re-enable from the same Telemetry tab or by removing the env var / settings
5056
override. While an env kill is active the Telemetry tab cannot re-enable —
5157
the env override always wins, and the attempt is refused rather than

src/telemetry/index.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,12 @@ export type Telemetry = {
146146
// exit. Callers use this to bound exit against dropped fire-and-forget
147147
// requests without ever making capture() itself blocking.
148148
flush(): Promise<void>;
149+
// Throws away everything queued and disarms the batch timer, so nothing
150+
// captured before this call can ever reach the network. Opting out uses
151+
// this: a user who says stop mid-session is saying they do not want the
152+
// activity they have already generated sent, which makes discarding the
153+
// queue the honest reading of that intent and flushing it a betrayal.
154+
discard(): void;
149155
};
150156

151157
// Fire-and-forget PostHog batch client. Never throws, never blocks the
@@ -260,5 +266,12 @@ export function createTelemetry(options: CreateTelemetryOptions): Telemetry {
260266
]);
261267
}
262268

263-
return { enabled, capture, flush };
269+
// A request already on the wire cannot be unsent, but nothing still held
270+
// in memory follows it.
271+
function discard(): void {
272+
cancelTimer();
273+
queue.length = 0;
274+
}
275+
276+
return { enabled, capture, flush, discard };
264277
}

src/telemetry/singleton.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,12 @@ import type { Telemetry } from "./index.js";
55
// than threading it through every intermediate call site. Defaults to a
66
// disabled no-op so any code path that runs before index.ts sets it (or in
77
// tests) never throws.
8-
let instance: Telemetry = { enabled: false, capture: () => {}, flush: async () => {} };
8+
let instance: Telemetry = {
9+
enabled: false,
10+
capture: () => {},
11+
flush: async () => {},
12+
discard: () => {},
13+
};
914

1015
export function setTelemetry(telemetry: Telemetry): void {
1116
instance = telemetry;

src/telemetry/toggle.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,14 @@ export function createTelemetryToggleHandler(
5757
return;
5858
}
5959
if (!enabled) {
60-
// Opt-out must be immediate and absolute: swap the in-memory singleton
61-
// synchronously, before any await, so no capture in flight during the
62-
// persistence step below can land on a still-enabled instance, and so
63-
// an unhandled rejection from disk I/O can never leave telemetry on.
60+
// Opt-out must be immediate and absolute: discard whatever the outgoing
61+
// instance has queued (dropping the singleton alone would leave its
62+
// batch timer armed to send it anyway), then swap the in-memory
63+
// singleton synchronously, before any await, so no capture in flight
64+
// during the persistence step below can land on a still-enabled
65+
// instance, and so an unhandled rejection from disk I/O can never
66+
// leave telemetry on.
67+
deps.getTelemetry().discard();
6468
deps.setTelemetry(
6569
deps.createTelemetry({ settings: { providers: {}, telemetry: { enabled: false } } }),
6670
);

tests/unit/telemetry-toggle.test.ts

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,43 @@ test("capture called immediately after toggle-off makes zero fetch calls", () =>
5656
expect(fetchCalls()).toBe(0);
5757
});
5858

59+
// Opting out is a statement about activity already generated, not only about
60+
// activity to come: events captured before the toggle must never be sent
61+
// afterwards. Dropping the singleton is not enough on its own — the outgoing
62+
// instance's batch timer would still fire and post its queue — so this test
63+
// guards the explicit discard. If a future change makes opt-out flush what it
64+
// was holding, this fails, and that is the point.
65+
test("opting out discards events captured before the toggle instead of sending them", async () => {
66+
let sends = 0;
67+
const fetchFn = (() => {
68+
sends++;
69+
return Promise.resolve(new Response("1", { status: 200 }));
70+
}) as unknown as typeof fetch;
71+
const { deps, getInstance } = fakeDeps({
72+
createTelemetry: (opts) =>
73+
createTelemetry({
74+
...opts,
75+
env: opts.env ?? {},
76+
apiKey: opts.apiKey ?? "test-key",
77+
fetchFn,
78+
// Short enough that an undiscarded queue would reach the network well
79+
// inside this test's wait, rather than passing by outrunning a timer.
80+
batch: { intervalMs: 20 },
81+
}),
82+
});
83+
const handler = createTelemetryToggleHandler("/fake/path", deps);
84+
85+
handler(true);
86+
await new Promise((resolve) => setTimeout(resolve, 10));
87+
getInstance().capture("cli_start");
88+
expect(sends).toBe(0);
89+
90+
handler(false);
91+
await new Promise((resolve) => setTimeout(resolve, 150));
92+
expect(getInstance().enabled).toBe(false);
93+
expect(sends).toBe(0);
94+
});
95+
5996
test("save rejection leaves the singleton disabled with no unhandled rejection", async () => {
6097
const { deps, getInstance } = fakeDeps({
6198
saveGlobalSettings: async () => {
@@ -90,7 +127,12 @@ test("load failure skips persistence entirely and stays disabled in memory", asy
90127
test("toggle on while env-killed writes nothing and swaps no instance", async () => {
91128
let ensureCalled = false;
92129
let saveCalled = false;
93-
const initial: Telemetry = { enabled: false, capture: () => {}, flush: async () => {} };
130+
const initial: Telemetry = {
131+
enabled: false,
132+
capture: () => {},
133+
flush: async () => {},
134+
discard: () => {},
135+
};
94136
let setInstance: Telemetry | undefined;
95137
const { deps } = fakeDeps({
96138
getTelemetry: () => initial,

0 commit comments

Comments
 (0)