Repository navigation
fix: normalize non-finite desktop event properties - #656
Conversation
There was a problem hiding this comment.
PostHog Review · Standard · reviewed 15290dc: 1 Should fix.
This is the only automatic review while this pull request is a draft. Comment @posthog review to review new changes sooner. Otherwise, a review of the new changes runs when you mark it ready for review.
posthog-flutter Compliance ReportDate: 2026-10-22 14:44:39 UTC ✅ All Tests Passed!45/45 tests passed Capture Tests✅ 29/29 tests passed View Details
Feature_Flags Tests✅ 16/16 tests passed View Details
|
There was a problem hiding this comment.
toJsonValue checks for null before converting, so a NaN member passes the dropNullMembers filter and is then emitted as null. Event properties end up sending "ratio": null, while the same field is omitted from the buffered exception step, and the step's byte budget counts members that are stripped before sending.
Should we filter on the converted value in toJsonValue's map branch (convert first, then drop if null when dropNullMembers)? That keeps buffered steps byte-identical to what's sent and makes NaN members follow the same omission rule as explicit nulls.
💡 Motivation and Context
On opt-in Windows/Linux, numeric NaN and infinities in event properties were sent as the strings "NaN", "Infinity" and "-Infinity". Omit these numeric values from object members and use JSON null for array slots, without dropping events or changing array positions. Caller-supplied literal strings remain unchanged.
Normalize object members before deciding whether to omit them. This keeps buffered exception steps identical to their emitted form, so byte budgets do not falsely reject steps or evict valid older steps. Native Android/Apple and web wrappers are unchanged. No public API shape changes.
Related cross-SDK context: PostHog/posthog-python#1081. The shared exception-step contract requires identical budgeted, buffered and emitted forms. Shared specs do not currently prescribe a non-finite-number policy.
💚 How did you test it?
Reproduction-first Flutter tests cover all three numeric values directly and in nested maps, arrays and objects inside arrays, plus finite/bool/literal controls, input preservation and a healthy event in the same batch. Review regressions reproduced a 171-byte wire step rejected as 210 buffered bytes, premature eviction of fitting steps, and a buffered/wire mismatch. Those exact regressions pass with the fix.
Affected tests passed 170 cases. The full VM suite passed 1260 tests with one existing macOS platform skip. Desktop core passed 251 tests and compliance adapter tests passed 25. Dart formatting, analysis, public API snapshot and Kotlin/Swift formatting passed. Authoring audit and exact-HEAD branch autoreview found no blockers. No live telemetry or native executable build was used.
The earlier iOS CocoaPods job failed only the unchanged native decodable-snapshot XCTest in the available log. Its assertion diagnostics were not included. This desktop Dart change does not claim to fix that separate native failure; CI should rerun after the update.
📝 Checklist
If releasing new changes
pnpm changesetto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
A Pi implementation agent used local shell/file tools, Flutter tests, read-only GitHub feedback APIs and isolated autoreview under maintainer direction. The follow-up reproduced the review claim and chose consistent object omission rather than counting null members that capture later deletes. Array slots and existing unrelated serialization behavior remain unchanged. Human review is required.
Human DRI: @marandaneto.