Repository navigation
fix(desktop): skip phx_ keys, keep response bodies out of logs, count duplicate copies - #681
Open
turnipdabeets wants to merge 11 commits into
Open
turnipdabeets wants to merge 11 commits into
turnipdabeets wants to merge 11 commits into
Conversation
Contributor
posthog-flutter Compliance ReportDate: 2026-10-31T02:17:43.415342+00:00 ✅ All Tests Passed!47/47 tests passed Capture Tests✅ 30/30 tests passed View Details
Feature_Flags Tests✅ 17/17 tests passed View Details
|
marandaneto
reviewed
Oct 29, 2026
marandaneto
left a comment
marandaneto
left a comment
Member
There was a problem hiding this comment.
Automated advisory code review.
Base automatically changed from
feat/v6-desktop-capture-ai
to
feat/v6-desktop-capture-v1
October 29, 2026 21:05
… duplicate copies - Posthog.setup skips a projectToken that is a personal API key (phx_), like the native SDKs, so the key never goes on the wire (INV-32). - PostHogFetchHttpError.toString leaves out the response body, which can echo request values (FC-5/FQ-18); the flush error log names the lane. - captureV1Results reads duplicates_dropped and the per-batch log reports the dropped copies (FQ-19).
Co-Authored-By: Claude Opus 5.5 <[email protected]>
…dpoint and reasons - A 2xx whose body has no readable per-event results fails the batch for good: it's dropped, not resent, and reported through the error event, matching posthog-ios and posthog-android. - One aggregate line per batch that lost events, naming the endpoint, the count and capture's `details` tags (never uuids or content). A terminal status logs how many events it lost; a multi-event 413 still halves. - A `limited` result counts as delivered, like any unknown result. - The non-string session/window id warning names the JSON type `boolean`. - Test fakes answer capture requests with per-event results, as capture v1 does. Co-Authored-By: Claude Opus 5.5 <[email protected]>
turnipdabeets
force-pushed
the
fix/v6-desktop-rubric-followups
branch
from
October 30, 2026 15:15
2c15a52 to
9942076
Compare
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 1 · PR risk: 0/10 |
eli-r-ph
reviewed
Oct 30, 2026
| } | ||
|
|
||
| final results = captureV1Results(response.body, uuids); | ||
| final results = captureV1Results(response.body, eventUuids); |
There was a problem hiding this comment.
The Linux and Windows desktop integration tests fail with this change: the example app's test server (example/integration_test/desktop_test.dart, /i/v1/analytics/events) answers with a bare 200 and no body, so this check now drops the batch and throws. Capture v1 always returns {"results": {...}}, and an empty {"results": {}} counts every event as delivered, so can the test server send that instead? The check itself matches the other SDKs.
Contributor
Author
There was a problem hiding this comment.
Fixed in 7dbc213 — the test server now answers {"results": {}}.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
This branch has not been deployed
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.
💡 Motivation and Context
Small follow-ups on the Windows/Linux capture v1 client (#679), to match the native iOS and Android SDKs.
phx_): capture always rejects them, and iOS/Android already disable the SDK for one. Desktop sent the key on every request.Posthog.setupnow skips setup with one log line, the same way it handles a blank token.PostHogFetchHttpError.toString()included the server's response, which can echo values from the request. That string is printed by the flush error log. TheresponseBodyfield is still there for code that needs it.duplicates_dropped: capture now keeps the first copy of a repeated uuid and reports how many later copies it dropped. The per-batch log now counts them. Desktop always generates its own uuids, so this should never fire. The count is there in case an SDK bug causes duplicates.Empty event names are already dropped locally on desktop (
_hasName), so nothing changed there.💚 How did you test it?
phx_tokens, the error string leaves out the response body, andcaptureV1Resultssumsduplicates_dropped.flutter analyzeclean,flutter testall passing (1376).📝 Checklist
If releasing new changes
pnpm changesetto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Claude Code checked the v6 desktop stack against the capture v1 SDK requirements and made these fixes. The
phx_check lives inPosthog.setupnext to the blank-token check, not in the desktop client. That means it also applies on web. That's fine: capture rejects these keys anyway, so nothing that works today stops working.🤖 Generated with Claude Code