Skip to content

fix: lift JSON-string session ids and enforce the event uuid rule - #367

Open
eli-r-ph wants to merge 2 commits into
v1-options-parityfrom
v1-session-id-lift
Open

eli-r-ph wants to merge 2 commits into
v1-options-parityfrom
v1-session-id-lift

Conversation

@eli-r-ph

@eli-r-ph eli-r-ph commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

💡 Motivation and Context

Capture v1 requires session_id and window_id to be strings and rejects the whole request otherwise, so every v1 SDK lifts only a string. Go's lift had two gaps against the agreed rule and against Python and Rust:

  • It used v.(string), so it dropped values whose JSON form is a string: a uuid.UUID, a named string type, or any json.Marshaler / encoding.TextMarshaler that encodes to a string. Go 1.x sent all of these as strings.
  • It returned "" for an absent key too, and the omitempty field then dropped a real empty string. Python and Rust send "", as Go 1.x sent the property.

Changes:

  • liftStringProperty keeps a string as is. For any other non-nil value, it marshals once and keeps the result if it is a JSON string. The payload fields are now *string, so "" is sent and an absent key is still omitted.
  • Every other value is dropped with a warning on every drop: dropping $session_id: a number value is not a string. The warning names the key and the JSON type (or the Go type when the value cannot be marshaled), never the value. nil, including a typed nil pointer, counts as unset and drops silently, the same rule options use.
  • buildEvent and prepareForSend take the client Logger. The five Enqueue call sites pass c.Logger; tests pass nil (rewritten with gofmt -r).
  • The migration guide gets a "Session and window IDs" section, plus a changeset.

A second commit applies the event uuid rule that every v1 SDK follows: an invalid caller uuid is replaced with a generated one and the SDK warns, and an empty or missing uuid is generated silently.

  • resolveEventUUID normalizes a valid Uuid, generates one for an empty Uuid without logging, and replaces an invalid one with one warning: event Uuid is not a valid UUID; sending the event with a generated UUID. The warning omits the value, because callers can put their own data in the field. Before, the replacement was silent.
  • Enqueue applies the rule before BeforeSend as before, and now again to the message the hook returns. Before, a hook that set an invalid or empty Uuid sent it as is, and capture rejected the whole batch. Re-checking a valid uuid costs one parse.
  • A changeset covers it. The migration guide text for uuids lives in docs: complete the v2 migration guide, changesets and godoc #368, which is rebased onto this branch and updated there.

Stacked on #359.

💚 How did you test it?

  • TestSessionAndWindowLiftOnlyJSONStrings covers a string, "", nil, a typed nil pointer, a uuid.UUID, a named string type, a number, a bool, a slice, a map and a channel. It checks the top-level field (sent or omitted), that both keys leave properties, and the exact warnings, which also proves no value text is logged. The typed nil pointer case caught that unmarshaling null into a string yields "".
  • TestEnqueue_UUID_InvalidPreSupplied now also checks for exactly one warning, without the value, for all five message types. TestEnqueue_UUID_RuleAppliesAfterBeforeSend sets an invalid, an empty and an uppercase Uuid in BeforeSend for all five types, and checks the sent uuid and the warnings. These 20 cases fail on the previous commit and pass on this one.
  • gofmt -l, go vet ./..., go test -race ./... and make api-diff pass locally. TestOpenMachOUniversal fails on my macOS machine with and without this change; CI runs it on Linux.

📝 Checklist

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

If releasing new changes

  • Ran pnpm changeset to generate a changeset file

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Written with Cursor (Claude Opus 5.5) under the direction of the assignee, to apply the session and window ID rules and the event uuid rule agreed for every v1 SDK. The assignee also decided that, where a hook can change the uuid, the SDK applies the uuid rule again after the hook. Companion PR for posthog-rs: PostHog/posthog-rs#285.

@eli-r-ph eli-r-ph self-assigned this Oct 10, 2026
@eli-r-ph
eli-r-ph marked this pull request as ready for review October 10, 2026 04:03
@eli-r-ph
eli-r-ph requested a review from a team as a code owner October 10, 2026 04:03
@eli-r-ph eli-r-ph changed the title fix: lift JSON-string session ids and warn on every drop fix: lift JSON-string session ids and enforce the event uuid rule Oct 10, 2026
Keeps a uuid.UUID, a named string type or an empty string as
$session_id / $window_id, drops nil silently, and logs a warning that
names the key and the value's type for any other dropped value.

This branch has not been deployed

No deployments
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