fix(realtime): prevent telemetry 401s on reconnection#88
Draft
tomershlasky wants to merge 1 commit intomainfrom
Draft
fix(realtime): prevent telemetry 401s on reconnection#88tomershlasky wants to merge 1 commit intomainfrom
tomershlasky wants to merge 1 commit intomainfrom
Conversation
commit: |
| } | ||
| this.sendReport(true); | ||
| this.statsBuffer = []; | ||
| this.diagnosticsBuffer = []; |
There was a problem hiding this comment.
sendReport keepalive parameter is now dead code
Low Severity
The keepalive parameter of sendReport is now always false since stop() no longer calls sendReport(true) and flush() always passes false. This makes the keepalive parameter, the isLast computation on line 152, and the keepalive: keepalive && isLast expression dead code that could confuse future readers into thinking there's still a keepalive code path.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Summary
TelemetryReporter.stop()now discards buffered data instead of sending a finalkeepalivereport401 Unauthorizederrors onPOST /api/v1/telemetrywhen reconnecting with a fresh API key while the old reporter's final fetch fires with a stale keyChanges
telemetry-reporter.tsstop()clears buffers instead of callingsendReport(true)unit.test.tsstop()does not make network requestsDesign Decision
Telemetry is non-critical — losing one final batch (up to 10s) on disconnect is acceptable. The three
stop()call sites inclient.tsremain unchanged; the new contract is strictly simpler (never makes a network call).Test plan
pnpm test)Note
Low Risk
Behavior change is isolated to telemetry shutdown semantics; main risk is losing the last buffered telemetry batch on disconnect, which is non-critical.
Overview
Prevents realtime disconnect/reconnect flows from emitting a final telemetry
fetchwith a potentially stale API key by changingTelemetryReporter.stop()to clear buffers and never send a final keepalive report.Updates unit tests to reflect the new contract: pre-session diagnostics are now discarded on disconnect, and
stop()(and subsequentflush()) should not produce any network requests.Written by Cursor Bugbot for commit 97ad940. This will update automatically on new commits. Configure here.