Skip to content

Commit 7a8016b

Browse files
fix(realtime): prevent telemetry 401s on reconnection (#88)
## Summary - `TelemetryReporter.stop()` now discards buffered data instead of sending a final `keepalive` report - Prevents `401 Unauthorized` errors on `POST /api/v1/telemetry` when reconnecting with a fresh API key while the old reporter's final fetch fires with a stale key ## Changes | File | Change | |------|--------| | `telemetry-reporter.ts` | `stop()` clears buffers instead of calling `sendReport(true)` | | `unit.test.ts` | Updated 2 tests to assert `stop()` does not make network requests | ## Design Decision Telemetry is non-critical — losing one final batch (up to 10s) on disconnect is acceptable. The three `stop()` call sites in `client.ts` remain unchanged; the new contract is strictly simpler (never makes a network call). ## Test plan - [x] All 129 unit tests pass (`pnpm test`) - [ ] Manual: connect → disconnect → reconnect with new key → no 401s in DevTools Network tab <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk: changes are limited to realtime telemetry shutdown behavior and related tests, trading a small amount of final telemetry for avoiding unwanted requests during disconnect/reconnect flows. > > **Overview** > Prevents stale API keys from triggering `401 Unauthorized` telemetry posts during disconnect/reconnect by changing `TelemetryReporter.stop()` to **never send a final report** and instead clear buffered stats/diagnostics. > > Simplifies the reporter send path by removing `keepalive` handling (including from `flush()` calls), and updates unit tests to assert that disconnect/`stop()` discards pre-session and buffered telemetry without making any network requests. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 2a2877c. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Adir Amsalem <adir@decart.ai>
1 parent 70a01a0 commit 7a8016b

2 files changed

Lines changed: 16 additions & 23 deletions

File tree

packages/sdk/src/realtime/telemetry-reporter.ts

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -94,16 +94,17 @@ export class TelemetryReporter implements ITelemetryReporter {
9494

9595
/** Flush buffered data immediately. */
9696
flush(): void {
97-
this.sendReport(false);
97+
this.sendReport();
9898
}
9999

100-
/** Stop the reporter and send a final report with keepalive. */
100+
/** Stop the reporter and discard any buffered data. */
101101
stop(): void {
102102
if (this.intervalId !== null) {
103103
clearInterval(this.intervalId);
104104
this.intervalId = null;
105105
}
106-
this.sendReport(true);
106+
this.statsBuffer = [];
107+
this.diagnosticsBuffer = [];
107108
}
108109

109110
/**
@@ -133,7 +134,7 @@ export class TelemetryReporter implements ITelemetryReporter {
133134
};
134135
}
135136

136-
private sendReport(keepalive: boolean): void {
137+
private sendReport(): void {
137138
if (this.statsBuffer.length === 0 && this.diagnosticsBuffer.length === 0) {
138139
return;
139140
}
@@ -148,14 +149,10 @@ export class TelemetryReporter implements ITelemetryReporter {
148149
// Send as many chunks as needed to drain both buffers.
149150
let chunk = this.createReportChunk();
150151
while (chunk !== null) {
151-
const isLast = this.statsBuffer.length === 0 && this.diagnosticsBuffer.length === 0;
152-
153152
fetch(TELEMETRY_URL, {
154153
method: "POST",
155154
headers: commonHeaders,
156155
body: JSON.stringify(chunk),
157-
// Only set keepalive on the very last chunk (if the caller requested it).
158-
keepalive: keepalive && isLast,
159156
})
160157
.then((response) => {
161158
if (!response.ok) {

packages/sdk/tests/unit.test.ts

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1618,7 +1618,7 @@ describe("Subscribe Client", () => {
16181618
}
16191619
});
16201620

1621-
it("buffers pre-session telemetry diagnostics and flushes them after session_id", async () => {
1621+
it("buffers pre-session telemetry diagnostics and discards them on disconnect", async () => {
16221622
const { createRealTimeClient } = await import("../src/realtime/client.js");
16231623
const { WebRTCManager } = await import("../src/realtime/webrtc-manager.js");
16241624

@@ -1683,17 +1683,11 @@ describe("Subscribe Client", () => {
16831683
});
16841684
}
16851685

1686+
// disconnect() calls stop() which discards buffered data instead of sending it.
1687+
// This avoids 401s when the API key has been invalidated on reconnect.
16861688
client.disconnect();
16871689

1688-
await vi.waitFor(() => {
1689-
expect(fetchMock).toHaveBeenCalledTimes(1);
1690-
});
1691-
1692-
const [, options] = fetchMock.mock.calls[0];
1693-
const body = JSON.parse(options.body);
1694-
expect(body.diagnostics).toHaveLength(1);
1695-
expect(body.diagnostics[0].name).toBe("phaseTiming");
1696-
expect(body.diagnostics[0].data.phase).toBe("websocket");
1690+
expect(fetchMock).not.toHaveBeenCalled();
16971691
} finally {
16981692
connectSpy.mockRestore();
16991693
stateSpy.mockRestore();
@@ -2306,7 +2300,6 @@ describe("TelemetryReporter", () => {
23062300
const [url, options] = fetchMock.mock.calls[0];
23072301
expect(url).toBe("https://platform.decart.ai/api/v1/telemetry");
23082302
expect(options.method).toBe("POST");
2309-
expect(options.keepalive).toBe(false);
23102303

23112304
const body = JSON.parse(options.body);
23122305
expect(body.sessionId).toBe("sess-1");
@@ -2351,7 +2344,7 @@ describe("TelemetryReporter", () => {
23512344
}
23522345
});
23532346

2354-
it("stop sends final report with keepalive", async () => {
2347+
it("stop discards buffered data without sending a request", async () => {
23552348
const { TelemetryReporter } = await import("../src/realtime/telemetry-reporter.js");
23562349

23572350
const fetchMock = vi.fn().mockResolvedValue({ ok: true });
@@ -2375,9 +2368,12 @@ describe("TelemetryReporter", () => {
23752368

23762369
reporter.stop();
23772370

2378-
expect(fetchMock).toHaveBeenCalledTimes(1);
2379-
const [, options] = fetchMock.mock.calls[0];
2380-
expect(options.keepalive).toBe(true);
2371+
// stop() should NOT send any network request
2372+
expect(fetchMock).not.toHaveBeenCalled();
2373+
2374+
// flush() after stop() should be a no-op (buffers were cleared)
2375+
reporter.flush();
2376+
expect(fetchMock).not.toHaveBeenCalled();
23812377
} finally {
23822378
vi.unstubAllGlobals();
23832379
}

0 commit comments

Comments
 (0)