Skip to content

fix(orbital): Fix event sampling and Safari SSE connection dropping#14

Merged
rahulchhabria merged 6 commits intomasterfrom
fix/sampling-clock-skew
Mar 5, 2026
Merged

fix(orbital): Fix event sampling and Safari SSE connection dropping#14
rahulchhabria merged 6 commits intomasterfrom
fix/sampling-clock-skew

Conversation

@rahulchhabria
Copy link
Contributor

Summary

Two bugs were causing events to be dropped or stop rendering entirely.

Clock skew drops live events

The stale-burst guard used Math.abs() which dropped events in both directions. Source servers with clocks ~5s ahead of the client were causing live events to be incorrectly discarded. Fixed to only drop events that are genuinely old (>10s in the past) — stale SSE bursts always arrive with past timestamps, not future ones.

Safari stops rendering after ~30s

Three compounding issues caused Safari to silently stop receiving events:

  • visibilitychange stuck — Safari iOS sometimes fires pagehide when switching apps but skips the matching visibilitychange on return, leaving windowInFocus = false permanently. Added pageshow/pagehide listeners as reliable Safari fallbacks.
  • SSE reconnect silently fails — Safari fires onerror with readyState=CONNECTING but never actually reconnects. The old handler only retried on CLOSED. Now force-reconnects on both states with a clean teardown.
  • No last-resort recovery — Added a 30s watchdog timer that resets on every received SSE message and force-reconnects if the stream goes silent, catching edge cases where the connection dies without firing an error event.

Test plan

  • Open in Safari — events should continue flowing past 30s without interruption
  • Switch apps on iOS Safari and return — events should resume immediately
  • In production with a source server clock ahead by ~5s — no "Dropping events" warnings in the console
  • Background the tab and foreground it — no stale burst warnings, events resume cleanly

🤖 Generated with Claude Code

rahulchhabria and others added 2 commits March 5, 2026 10:24
…source servers

The stale-burst guard was using Math.abs() which dropped events in both
directions. Source servers with clocks ~5s ahead of the client were
causing live events to be incorrectly discarded.

Only drop events that are genuinely old (>10s in the past) — stale SSE
bursts always arrive with past timestamps. Events with future timestamps
are fine; they just reflect server/client clock drift.

Also loosens the threshold from 5s to 10s to give more margin for slow
SSE queue drains before a tab comes back into focus.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Three issues caused Safari to stop rendering events:

1. visibilitychange unreliable on Safari iOS — added pageshow/pagehide
   listeners as fallbacks since Safari sometimes fires pagehide when
   switching apps but skips the matching visibilitychange on return,
   leaving windowInFocus stuck at false and silently dropping all events.

2. SSE reconnect silently fails on Safari — onerror now force-reconnects
   on both CLOSED and CONNECTING states. Safari fires onerror with
   readyState=CONNECTING but never actually reconnects, leaving the
   connection dead indefinitely.

3. No last-resort recovery — added a 30s watchdog timer that resets on
   every received SSE message and force-closes/reconnects if silent,
   catching any edge case where the connection dies without an error event.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@rahulchhabria rahulchhabria requested a review from BYK March 5, 2026 18:41
…ad lastMessageAt variable

On non-Safari browsers, readyState === CONNECTING during onerror means the browser is
already handling reconnection with native exponential backoff. Forcing a manual reconnect
in that case bypasses backoff and increases server load unnecessarily.

Also remove the lastMessageAt variable which was assigned but never read — the watchdog
relies entirely on setTimeout/clearTimeout.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

…n background tabs

Two fixes:

1. Watchdog/heartbeat: During legitimate low-traffic periods the 30s watchdog
   would fire and force-reconnect in a loop. Add a server-side goroutine that
   sends a JSON ping ({}) every 20s; the client onStreamMessage ignores non-array
   messages so real event processing is unaffected.

2. pageshow background tab bug: pageshow fires for all page loads (not just
   bfcache restorations), so opening a tab in the background would let pageshow
   override the document.hidden initialisation and mark windowInFocus=true. Guard
   the pageshow handler with a document.hidden check to match the Safari bfcache
   fallback intent.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
source = null;
}
connectStream();
}, 30000);
Copy link
Member

Choose a reason for hiding this comment

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

30 seconds is a very long time for Sentry events. I'd make it 5 to keep responsiveness.

main.go Outdated
// Heartbeat: send a ping every 20s so the client watchdog doesn't
// fire during legitimate low-traffic periods.
go func() {
ticker := time.NewTicker(20 * time.Second)
Copy link
Member

Choose a reason for hiding this comment

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

Accordingly, this should be around 5 seconds or less

rahulchhabria and others added 2 commits March 5, 2026 12:37
Per BYK's feedback: 30s is too long for a Sentry events stream.
Watchdog now fires after 5s of silence; server heartbeat ticks every
3s to keep connections alive without triggering the watchdog.

Co-Authored-By: Claude <noreply@anthropic.com>
camera.position.set(0, y, z) was resetting the x-component to 0 on
breakpoint transitions, snapping the globe back to its start angle
when autoRotate had moved the camera away from the Z axis.

Decompose the current camera position into its azimuthal angle before
applying the new distance/y-offset, then recompose at that same angle
so only zoom and vertical offset change.

Co-Authored-By: Claude <noreply@anthropic.com>
@rahulchhabria rahulchhabria merged commit c851eec into master Mar 5, 2026
5 checks passed
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