fix: omit null custom event properties during serialization - #314
Draft
marandaneto wants to merge 2 commits into
Draft
fix: omit null custom event properties during serialization#314marandaneto wants to merge 2 commits into
marandaneto wants to merge 2 commits into
Conversation
Contributor
posthog-go Compliance ReportDate: 2026-09-09 10:11:54 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
|
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
Custom event properties currently send explicit null object members. This implements the serialization rule in PostHog/sdk-specs#60 for the Go SDK's legacy and v1 event paths.
The event serializer now omits null object members recursively, including objects inside arrays. It preserves null array positions, empty objects and arrays, false, zero, strings, supported numbers, and ordered duplicate JSON members. Existing nullable public inputs and caller-owned values remain unchanged. Cleanup happens after enrichment and before-send processing without restoring privacy-filtered fields.
The shared event serialization boundary covers capture, AI-named ordinary capture, exception custom properties, and identify/group custom properties. It does not change generic
PropertiesJSON or introduce a separate AI route. Existing typed flag response nulls and exception metadata keep their field-specific behavior. There is no blanket exemption for$or AI properties and no disk queue added.The correction also preserves the original concrete serialization callback errors. Only the extra wrapper from the private event-property adapter is removed. User marshaler error layers and the existing exception wrapper remain intact. The zero-values fixture now expects null omission, with its nil input and all other assertions preserved.
💚 How did you test it?
Current draft status: The signed and reviewed HEAD is
28ea011afc52a967565102d20c3576671e919007. This includes the correction for the stale null fixture and callback error wrapper that caused four test failures at the previous HEAD. All 28 hosted checks passed for this exact SHA, including Test and Race jobs on Go 1.21 and Go 1.x, Public API, builds, vet, formatting, OTel, security scans, and v0/v1 compliance. Unit Tests: https://github.com/PostHog/posthog-go/actions/runs/34338572018. The bounded watch completed successfully on 2026-09-09. These results supersede the four failures on the previous SHA.-race -count=1on Go 1.26.4, Darwin/arm64. These include 84 exact-byte ordered-member cases, 8 malformed/out-of-range rejection controls, and 24 concrete error-chain and user-error identity controls across both protocols and capture/identify/group/exception paths.go vet -mod=readonly ., changed-filegofmtchecks, and branch whitespace checks. Reused existing offline caches with module downloads disabled. No dependencies were installed.28ea011afc52a967565102d20c3576671e919007against main53f54712427369388565577777d13970689395e1, with mandatory TruffleHog clean and no findings. Main is included without a merge because it has not advanced.Draft validation limits: The full local API snapshot check remains blocked by existing uncached CLI/example dependencies (
godotenv,urfave/cli, andgo-md2man/v2), as recorded during the previous attempt. It was not retried or repaired here. The full suite, minimum-Go matrix, local-poller/transport-failure flags, retry/compression-specific null payloads, and performance checks were not run locally. Existing goccy rejection of1e400remains unchanged. Disk persistence is not applicable. Hosted results are separate from these local limits. Existing compliance harness passes do not establish the missing null-policy fixture coverage.📝 Checklist
If releasing new changes
pnpm changesetto generate a changeset fileAn equivalent
posthog-go: patchchangeset was added manually. The same changeset covers the correction. No package-manager setup or release was run.🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Implemented and validated with Pi coding assistance, Git, Go, GitHub CLI, and the installed isolated autoreview helper. The human-directed scope was recursive event-property null omission without public API narrowing, array compaction, or duplicate-key loss. Existing goccy token traversal was used instead of a map round-trip so supported duplicate members and numeric values survive. The callback correction removes only the private adapter's added error layer. Private session logs are not published.
This remains a draft for human review, assigned to marandaneto. No release, auto-merge, or approval is requested by the agent.