Skip to content

Event properties are silently overwritten by provider context keys #35

Description

@multiplehats

Summary

Every provider builds its outgoing payload by spreading the event's own properties first and then writing trakoo's context and metadata over the top. An event that declares a property whose name collides with one of those injected keys loses its value on the way out: no error, no warning, no type complaint. The registry says the property is part of the event; the provider drops it anyway.

src/providers/openpanel/shared.ts:

return {
	...properties,                                          // event's own properties
	...(metadata.category && { category: metadata.category }),
	...(context?.page && { page: context.page }),           // wins
	...(context?.device && { device: context.device }),
	...(context?.utm && { utm: context.utm }),
	// ...
};

How it showed up

An event declared page: "landing" | "tool" to record where a copy button was pressed. It typechecked, validated, and arrived in OpenPanel carrying trakoo's page context object instead of "landing". Types, lint, unit tests and the build were all green — the only way to catch it was to read a real POST body off the wire. The property was renamed to work around it.

Two things make it worse than a normal footgun:

  • The registry is the contract. defineEvents is where a consumer declares what an event carries, and it accepts a name the provider will then quietly discard.
  • Most collisions are conditional. page, device, utm and friends are only written when that context is present, so the same event can survive one call site and be clobbered at the next. category and timestamp collide unconditionally.

Scope

Not OpenPanel-specific — the pattern is repo-wide, with a different reserved set per provider and per method. Keys marked * are written unconditionally; the rest only when the matching context is present.

Provider / method Keys written over the event's properties
OpenPanel track (client + server) category, __timestamp, profileId, sessionId, __path, __title, __referrer, page, device, utm, user_email, user_traits
OpenPanel pageView page, device, utm, __path, __title, __referrer, user_email, user_traits (+ category, profileId on the server)
PostHog track (client) category*, timestamp*, userId, sessionId, $current_url, device, utm, user_email, user_traits
PostHog track (server) category*, timestamp*, sessionId, $current_url, $page_title, $referrer, device, utm, user_email, user_traits
PostHog pageView / pageLeave path, title, referrer
Bento track (client) category*, timestamp*, userId, sessionId, page, device, utm, user_email, user_traits, visitor
Bento track (server) category*, timestamp*, sessionId, page, device, utm, site*, visitor
Bento pageView / pageLeave date*, page, visitor, site
Pirsch track (server) category*, timestamp*, userId, sessionId, user_email, timezone, browser
Pirsch track (client) category, page_path
Visitors track category, page_path, page_title

category collides everywhere. EmitKit is a variation on the same coupling in the other direction: it reads __emitkit_channel and description out of the event's properties as routing metadata.

Possible directions

Not proposing one yet — they trade off differently and the choice is a breaking-change question.

  1. Fail loudly on collision. Detect a declared property that matches the target provider's reserved set and route it through the existing validation failure policy (onError, onFailure: "drop" | "throw"). Precise, reuses machinery consumers already configure, and it is runtime-only — a wrong name still ships.
  2. Make it a compile error. Exclude reserved names in defineEvents so page on an event is a type error. Catches it at the only moment it is cheap to fix, but the reserved set is provider-specific and the registry is deliberately provider-agnostic, so this would have to reserve the union across all providers.
  3. Namespace the injected keys. Give trakoo's context a prefix (__page, __device) or nest it under a single key so consumer properties can never be hit. Cleanest boundary, but it changes the property names every existing consumer's dashboards and queries are built on — a major.

My inclination is 1 + 2 together: the type error is what actually prevents the bug, and the runtime check covers dynamic and proxied events where types were never involved. 3 is the correct end state if a major is on the table anyway.

Whichever way it goes, the regression test should assert the exact key set on the payload a provider hands to its SDK, not just that the declared property is present — the original bug passed every test that only checked for presence.

Notes

  • Found while wiring trakoo into a production project, alongside the silent-401 problem fixed in Report OpenPanel delivery failures instead of dropping them #34. Unrelated cause, same root symptom: an event that looks delivered and is not.
  • The provider docs and the agent skill do not currently list any reserved names, so a consumer has no way to know which names are unsafe.

https://claude.ai/code/session_01MmqjXsp5iX3Upj27YiGiw1

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions