Skip to content

fix: preserve null array positions in event serialization - #770

Draft
marandaneto wants to merge 3 commits into
mainfrom
fix/null-property-serialization-sqq3ojkm
Draft

fix: preserve null array positions in event serialization#770
marandaneto wants to merge 3 commits into
mainfrom
fix/null-property-serialization-sqq3ojkm

Conversation

@marandaneto

@marandaneto marandaneto commented Sep 9, 2026

Copy link
Copy Markdown
Member

💡 Motivation and Context

Event serialization currently removes null array entries, shifting the remaining values. Preserve those positions while continuing to omit null-valued object members recursively, as required by PostHog/sdk-specs#60.

Only PostHogEvent encoding opts into this behavior in the shared core Gson serializer. Event disk persistence and batch delivery use the same fix, including restored events and properties added by before-send hooks. Java and Kotlin public inputs, caller-owned data, empty objects, other scalar values, and existing envelope metadata rules stay unchanged. General maps, log disk records, and server feature-flag caches retain their previous behavior. No wrapper serializer or dependency update is added. The server wire snapshot now expects the preserved null positions in its ordered list and array. Unsupported NaN values remain omitted.

💚 How did you test it?

  • Compiled the actual core and server sources and ran 18 focused JUnit tests, including all four unchanged server wire snapshots. Tests inspect serialized bytes, physical event files, fresh-queue restore, guarded loopback HTTP delivery, exception capture, hook enrichment/drop, and log/cache compatibility controls. All passed. The snapshot runner verifies its owned localhost resolver and denies non-loopback DNS, TCP, UDP, and multicast before loading SDK or test classes. No forbidden network requests occurred during tests.
  • Compared javap -public output for all 191 core and 27 server classes listed in the committed API surfaces. No signature changes.
  • Checked the five changed Kotlin files with the matching ktlint 1.1.1 formatter. No findings. git diff --check passed.
  • Ran isolated autoreview against the exact committed head 47bdf22084f2c5e0ef17858beaef1f2a4357d683, based on main d4a3a0387ab232414de887152aa27e4f5cb13b54. No actionable findings.

Local validation limits: These are Kotlin CLI 2.4.10 checks with language/API compatibility 2.0 and JVM target 1.8 on Corretto 21, not the pinned Kotlin 2.1.21 Gradle release toolchain. Full Makefile/Gradle suites and Kotlin metadata/API-validator checks were not run locally. Android device/instrumentation, replay runtime, real process restart/crash, old QueueFile migration, and exhaustive custom-converter/cycle cases remain unvalidated. A fresh queue object is not a process restart. Dedicated AI/generated flag-event wire cases were not run. This draft is for human review, not merge or release clearance.

The previous Build & Test run failed because the server wire snapshot still expected null array slots to be removed. Reproduced that exact failure locally, updated only the two expected null slots, and reran all 18 focused tests successfully. Hosted CI for 47bdf22084f2c5e0ef17858beaef1f2a4357d683 passed all 16 checks, including Build & Test, SDK Compliance Tests, CodeQL, Semgrep, Wiz, wrapper validation, changeset hygiene, and PR title validation. Build & Test completed make compile and the release-task/dependency-lock check successfully. The existing compliance harness does not establish this null-property contract.

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed. The existing changeset documents the behavior.
  • No breaking change or entry added to the changelog.

If releasing new changes

  • Ran pnpm changeset to generate a changeset file

The previously reviewed patch changeset for core, Android, and server is retained. No package-manager setup or release command was run for this PR.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Implemented and validated with Pi coding agents, Git, Kotlin/JUnit, ktlint, and an isolated Pi autoreview helper. The human-directed scope keeps the fix in shared event serialization and preserves unrelated log/cache behavior. The private agent session is not publicly shareable. Human review is required before merge.

@marandaneto marandaneto self-assigned this Sep 9, 2026
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

posthog-android Compliance Report

Date: 2026-09-09 10:03:46 UTC
Duration: 118402ms

✅ All Tests Passed!

46/46 tests passed


Capture Tests

29/29 tests passed

View Details
Test Status Duration
Format Validation.Event Has Required Fields 340ms
Format Validation.Event Has Uuid 27ms
Format Validation.Event Has Lib Properties 30ms
Format Validation.Distinct Id Is String 26ms
Format Validation.Token Is Present 26ms
Format Validation.Custom Properties Preserved 30ms
Format Validation.Event Has Timestamp 27ms
Retry Behavior.Retries On 503 7029ms
Retry Behavior.Does Not Retry On 400 4026ms
Retry Behavior.Does Not Retry On 401 4028ms
Retry Behavior.Respects Retry After Header 7029ms
Retry Behavior.Implements Backoff 17040ms
Retry Behavior.Retries On 500 7020ms
Retry Behavior.Retries On 502 7016ms
Retry Behavior.Retries On 504 7018ms
Retry Behavior.Max Retries Respected 17023ms
Deduplication.Generates Unique Uuids 38ms
Deduplication.Preserves Uuid On Retry 7016ms
Deduplication.Preserves Uuid And Timestamp On Retry 12029ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry 7020ms
Deduplication.No Duplicate Events In Batch 39ms
Deduplication.Different Events Have Different Uuids 25ms
Compression.Sends Gzip When Enabled 20ms
Batch Format.Uses Proper Batch Structure 19ms
Batch Format.Flush With No Events Sends Nothing 14ms
Batch Format.Multiple Events Batched Together 33ms
Error Handling.Does Not Retry On 403 4023ms
Error Handling.Does Not Retry On 413 4021ms
Error Handling.Retries On 408 5026ms

Feature_Flags Tests

17/17 tests passed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id 41ms
Request Payload.Flags Request Uses V2 Query Param 25ms
Request Payload.Flags Request Hits Flags Path Not Decide 25ms
Request Payload.Flags Request Omits Authorization Header 24ms
Request Payload.Token In Flags Body Matches Init 29ms
Request Payload.Groups Round Trip 29ms
Request Payload.Groups Default To Empty Object 31ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False 28ms
Request Payload.Disable Geoip Omitted Defaults To False 36ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key 26ms
Request Lifecycle.No Flags Request On Init Alone 17ms
Request Lifecycle.No Flags Request On Normal Capture 24ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests 44ms
Request Lifecycle.Mock Response Value Is Returned To Caller 28ms
Retry Behavior.Retries Flags On 502 325ms
Retry Behavior.Retries Flags On 504 325ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event 22ms

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant