fix(errortracking): stamp native crashes on the SDK's clock - #813
Conversation
Exit records carry wall-clock time, but on API 33+ the batch sent_at comes from the network-corrected date provider. Ingestion shifts each event by timestamp - sent_at, so a crash moved by however far the two clocks disagreed. Shift the exit timestamp into the date provider's clock; the watermark keeps the raw exit timestamp.
|
The PR should not merge until recovered crashes remain correctly timed across clock-source changes and changes between crash and recovery. Reviews (1) · Last reviewed commit: "fix(errortracking): stamp native crashes..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…shes Record each run's date-provider-to-wall-clock offset and apply the offset of the run a crash happened in, so a clock change between the crash and its recovery no longer shifts the event.
marandaneto
left a comment
marandaneto
left a comment
There was a problem hiding this comment.
Two reproduced timestamp-correction regressions from the review of d40a8ae.
|
suggestion: Non-blocking follow-up on the clock-offset approach — This PR addresses the mismatch between wall-clock crash timestamps and the SDK’s network-time clock, but an offset saved when the app starts may be wrong when it crashes if the device time changes during that run. Libraries such as TrueTime Android may be useful references for persisting clock information and maintaining a network-based time estimate across device-time changes. That is a reference to investigate, not a recommendation to add a dependency. I’m not sure the extra complexity is worth it here: wall clock and network time generally agree, and when they differ the gap may be milliseconds, seconds, or minutes. We should weigh the practical impact against the implementation and maintenance cost before expanding this fix. This comment is non-blocking; documenting the limitation may be sufficient. curious what @ioannisj say? |
… them on time changes Runs were matched to crashes by wall-clock start time, which breaks when the clock moves backwards. The exit record carries the pid, so key runs by pid. A run's startup offset also went stale when the wall clock changed during the run, so the offset is now re-recorded on ACTION_TIME_CHANGED. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Yeah not sure if the complexity is needed here, documenting should be enough until we have a stronger signal? |
|
Agreed on keeping this small. Keying by pid and refreshing on time changes fixes both regressions with no new dependency. The case Greptile raised is still open: a run that never got network time records 0. That applies to every event captured before the date provider gets network time, so I've left it out of this PR. |
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
3 participants
💡 Motivation and Context
Native (NDK) crashes recovered from
ApplicationExitInfocan land at the wrong time in PostHog, by however far the device's wall clock and network time disagree.PostHogNativeCrashIntegrationstamps the event withexitInfo.timestamp, which is wall-clock time.PostHogAndroidDateProviderfills the batchsent_atfrom network time.received time + (timestamp - sent_at). With the two values on different clocks, the gap between the clocks goes straight into the stored time.Seen on an emulator whose network clock lagged the wall clock by about 36 minutes after the host slept: every recovered NDK crash was stored 36 minutes in the future. A real device with a manually set or drifted clock would be off by its clock error in the same way. Found while testing React Native NDK crash capture (PostHog/posthog-js#5062).
The fix records each run's offset between
dateProvider.currentTimeMillis()and the wall clock (NativeCrashClockOffsetStore, last 16 runs, keyed by pid). The offset is recorded at startup and again on everyACTION_TIME_CHANGED, so it stays right if the wall clock is set while the app runs. Each recovered crash's exit timestamp is then shifted by the offset of the run it crashed in, found by the exit record's pid, so the event is on the same clock assent_ateven if the clock changed between the crash and its recovery. Runs are keyed by pid, not start time, because the wall clock can move backwards. The current offset is only used when no run was recorded yet, for example on the first launch after upgrading. The watermark still stores the rawexitInfo.timestamp, so deduplication is unchanged. Events captured in-process (JVM crashes and everything else) already use the date provider and are unaffected.💚 How did you test it?
NativeCrashClockOffsetStoreTest. Each fails without its part of the fix. All:posthog-android:testDebugUnitTesttests pass, andspotlessApplymade no changes.posthog-jsexample-expo-57) with this build forced in place ofposthog-android3.66.0, release build, and a real JNI SIGSEGV. The crash time is the OS exit-record time; the stored time comes from PostHog.kill -SEGV(posthog-android sample app)The seconds that remain are network delay between building the batch and the server receiving it, which the skew correction adds to every event.
Regression checks on the same build:
📝 Checklist
If releasing new changes
pnpm changesetto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Found and fixed with Claude Code while testing posthog-js#5062 on device. The root cause was traced by reading the queued event file on a rooted emulator, which showed a correct crash timestamp with a network-time
sent_at. The emulator's NTP state confirmed the clock gap.Rejected alternatives:
PostHogAndroidDateProvider: it uses network time on purpose.🤖 Generated with Claude Code