Skip to content

fix(core): guard timeline method calls for non-conformant objects#1098

Merged
miguel-heygen merged 2 commits into
mainfrom
fix/player-timeline-safety-guards
May 28, 2026
Merged

fix(core): guard timeline method calls for non-conformant objects#1098
miguel-heygen merged 2 commits into
mainfrom
fix/player-timeline-safety-guards

Conversation

@miguel-heygen
Copy link
Copy Markdown
Collaborator

Summary

  • User compositions can register timeline-like objects on window.__timeline where .duration is a number property (not a function) and .pause/.play may be missing entirely. The runtime player called these unconditionally, producing ~166 t.getTimeline(...).duration is not a function and ~38 t.pause is not a function errors per day.
  • Added safeNum() and safeVoid() helpers in player.ts that check typeof before calling — reading numbers as direct property values when not functions, and silently skipping missing void methods. Applied across all timeline call sites.
  • Added 4 test cases for non-conformant timeline objects: duration-as-number, missing pause, missing play, time-as-number.

Test plan

  • bun test packages/core/src/runtime/player.test.ts — 39 tests pass (35 existing + 4 new)
  • bun run build — clean
  • bunx oxlint + bunx oxfmt --check — clean
  • All pre-commit hooks pass (lint, format, typecheck, fallow audit, commitlint)

User compositions can register timeline-like objects on window.__timeline
where .duration is a number property (not a function) and .pause/.play
may be missing entirely. The runtime player called these unconditionally,
causing ~166 "duration is not a function" and ~38 "pause is not a function"
errors per day.

Add safeNum() and safeVoid() helpers that check typeof before calling,
falling back to reading numbers as properties and silently skipping
missing void methods. Applied consistently across all timeline method
call sites in player.ts.
Copy link
Copy Markdown
Collaborator

@vanceingalls vanceingalls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: fix(core): guard timeline method calls for non-conformant objects

Design is sound. The two helpers are well-scoped, the call sites are exhaustive, and the test coverage closes the cases that caused the PostHog noise. Approving — this ships the P0. Two items to carry forward:


[important] safeNum / safeVoid produce zero observability for non-conformant timelines

After this lands, the ~204 errors/day vanish from PostHog. That's the goal. But the team will now have no signal on:

  • Which user compositions are non-conformant and which properties are missing
  • Whether that population is growing or regressing
  • If rate doubles tomorrow

The diagnostics.ts swallow() helper exists exactly for this — its own JSDoc says "emitting nothing makes silent failures invisible to anyone debugging a genuinely broken composition." It's already used two lines down for sibling errors (runtime.player.site1, runtime.player.site2), and it's already imported in this file.

A minimal fix inside each helper:

// safeNum — when the property is not a function and not a number
swallow("runtime.player.nonConformantNum", { prop, actual: typeof val });
return fallback;

// safeVoid — when the method is not callable
swallow("runtime.player.nonConformantVoid", { method, actual: typeof fn });

This costs nothing in production (the swallow path is silent unless __hfDebug is set or __hf.onSwallowed is installed), but gives the studio and observability pipeline a hook to surface which compositions are hitting this. The error-rate chart on the AI Studio Health dashboard would then show a real "non-conformant calls silenced" counter instead of a gap. Can be a follow-up PR.


[nit] safeNum: number branch doesn't guard NaN

if (typeof val === "number") return val;

typeof NaN === "number" is true. So duration = NaN flows straight through, while the function branch does Number(val.call(obj)) || fallback which correctly maps NaN to fallback. Current call sites are all wrapped in Math.max(0, ...) || 0 so NaN can't escape, but the helper itself is inconsistent. Suggest:

if (typeof val === "number" && Number.isFinite(val)) return val;

Everything else checks out — sibling propagation is preserved, all 35 existing tests still pass, the 4 new ones cover every non-conformant shape observed in the PostHog errors, and all required CI is green. Ship it.

— Vai

@miguel-heygen miguel-heygen merged commit 2d7b9e5 into main May 28, 2026
45 checks passed
@miguel-heygen miguel-heygen deleted the fix/player-timeline-safety-guards branch May 28, 2026 00:16
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.

2 participants