Repository navigation
fix: preserve events with non-finite numeric properties - #382
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
PostHog Review · Standard · reviewed db9698c: 1 Must fix, 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.
Contributor
posthog-go Compliance ReportDate: 2026-10-22 18:31:22 UTC ✅ All Tests Passed!111/111 tests passed Capture_V1 Tests✅ 94/94 tests passed View Details
Feature_Flags Tests✅ 17/17 tests passed View Details
|
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: 2 · PR risk: 0/10 |
dustinbyrne
approved these changes
Oct 22, 2026
This was referenced Oct 25, 2026
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
Numeric NaN and infinities can discard an event before queue admission in legacy and analytics-v1 capture. Non-finite float32 values can also produce invalid JSON without an encoder error.
Normalize supported property containers once into a copied graph, then encode the event. Replace numeric non-finites with null while preserving array positions, healthy properties/events, literal strings and caller-owned values. Arbitrary structs containing a non-finite field become null as a whole property.
Preserve native custom-marshaler boundaries, including go-json context marshalers and pointer-receiver redaction. Malformed custom output stays an error and is never replaced by exported fields. By-value array elements use addressable copies to preserve native redaction without enabling pointer methods on unrelated array containers. Unrelated serialization errors still propagate.
Related source Python PR: PostHog/posthog-python#1081. This is context, not a claim that it is merged.
💚 How did you test it?
Verified RED/GREEN for numeric event loss, invalid float32 JSON, malformed context-marshaler privacy, repeated subtree encoding and pointer-receiver redaction in slices and by-value arrays. Exact main/native controls verify array-element and named-container method semantics independently.
There are 95 authored regression/control cases through real SDK preparation/capture with fake HTTP transport, healthy neighboring events, strict JSON, error identity and input ownership checks. Full root make build passes on the exact SDK source (568 top-level PASS records), including vet and race tests. After merging the authorized upstream action-pin-only update, focused race tests, API/formatting checks and exact-final-HEAD branch autoreview pass again on Go1.26.4.
The 2,000-level probe improved from 375–390ms to 1.47–1.83ms after one-pass normalization. Upfront copying costs more for finite input: representative full preparation is about 4.06µs/38 allocations versus 1.88µs/15 on main and 2.61µs/15 on the earlier PR version. These are local microbenchmarks, not end-to-end throughput claims.
Hosted checks are clear on published
103af4a. The new array correction and workflow-only merge still need hosted checks after publication. Earlier local minimum-Go runtime checks were blocked by macOS; Linux cross-compilation was compile-only. Hosted Go 1.21 and nested-adapter checks passed on103af4a. No production shutdown/retry behavior or timing fixtures were changed.📝 Checklist
If releasing new changes
pnpm changesetto generate a changeset fileThe patch changeset was authored directly in the documented format. No API, runtime dependency or lane version bump is added.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Human DRI: @marandaneto. An OpenAI implementation agent used local shell/file/gh tools under human direction. Review claims were independently reproduced before narrow corrections. Struct field/tag handling remains delegated to the native encoder rather than a replacement serializer.
The array correction is signed in
b406ce477cebb6ec1ea9955352fdaf8d94336d4e. Final signed merge466a2517009811ea8f0538f67e60e8a4a963abbaincorporates canonical action pins without rewriting history. Exact branch autoreview is clean against6ffac5a198fa246c7596376f27fad0f73466ca7a. Human review is required.