feat(errortracking): capture native ndk crashes from tombstones - #659
feat(errortracking): capture native ndk crashes from tombstones#659cat-ph wants to merge 5 commits into
Conversation
posthog-android Compliance ReportDate: 2026-08-04 20:00:55 UTC ✅ All Tests Passed!46/46 tests passed Capture Tests✅ 29/29 tests passed View Details
Feature_Flags Tests✅ 17/17 tests passed View Details
|
…ture # Conflicts: # posthog-android/src/main/java/com/posthog/android/PostHogAndroid.kt # posthog/api/posthog.api # posthog/src/main/java/com/posthog/internal/PostHogRemoteConfig.kt
| val activityManager = | ||
| context.getSystemService(Context.ACTIVITY_SERVICE) as? ActivityManager ?: return | ||
| val preferences = config.cachePreferences ?: return |
There was a problem hiding this comment.
we can move this to PostHogAndroidUtils.kt i think
| } | ||
| integrationInstalled = true | ||
|
|
||
| Thread({ scanSafely(postHog) }, "PostHogNativeCrashScanner") |
There was a problem hiding this comment.
we can probably use a single thread executor instead, and we'd like to be able to pass the executor as ctor param so we can unit test
| } | ||
|
|
||
| properties?.let { | ||
| postHog.capture( |
There was a problem hiding this comment.
we should attach the cached properties as we do for error tracking hard crashes (iOS)
not a blocker for this PR, but an improvement we'd need to make at some point to avoid issues with error investigation
eg sdk version 1.0.0, app version 2.0.0, but the app got upgraded after sending this error, customers would be investigating the wrong version eg 2.0.1 instead
this should be written down in the docs/config since its an important caveat that can waste lots of hours of investigation
| * | ||
| * Disabled by default | ||
| */ | ||
| public var captureNativeCrashes: Boolean = false, |
There was a problem hiding this comment.
blocking: Adding this defaulted parameter to the public Kotlin primary constructor changes/removes the synthetic default-argument constructor. @JvmOverloads preserves the ordinary overloads, but not that synthetic descriptor, so existing Kotlin callers that omit any constructor argument can fail at runtime with NoSuchMethodError after upgrading. I reproduced this by compiling PostHogErrorTrackingConfig(inAppIncludes = mutableListOf("com.example")) against main and running it against this branch. Could we keep the primary constructor unchanged and declare captureNativeCrashes as a property in the class body instead?
| private var postHog: PostHogInterface? = null | ||
|
|
||
| private companion object { | ||
| private const val LAST_CAPTURED_TIMESTAMP_KEY = "nativeCrashLastCapturedTimestamp" |
There was a problem hiding this comment.
blocking: This watermark needs to be treated as SDK-internal, project-level state. Because the key is not included in PostHogPreferences.ALL_INTERNAL_KEYS, PostHogSharedPreferences.getAll() returns it as a registered property and buildProperties() attaches nativeCrashLastCapturedTimestamp to subsequent customer events. It is also absent from PostHog.reset()'s preserved keys, so reset/logout clears the deduplication cursor and the same retained tombstones are captured again after restart.
There is also a durability issue: setValue() ultimately uses SharedPreferences.Editor.apply(), which updates memory synchronously but writes to disk asynchronously without reporting failures. An abrupt process death before that write completes can lose the cursor and recapture an already queued crash on the next launch. Could we add the key to ALL_INTERNAL_KEYS, preserve it across reset(), and persist this marker synchronously in project-scoped storage before treating the record as acknowledged?
| val watermark = preferences.getValue(LAST_CAPTURED_TIMESTAMP_KEY) as? Long ?: 0L | ||
| val crashes = | ||
| activityManager | ||
| .getHistoricalProcessExitReasons(context.packageName, 0, MAX_EXIT_RECORDS) |
There was a problem hiding this comment.
blocking: Android applies maxNum before this code filters by exit reason. If a native crash is older than 20 newer non-native exits, it is excluded from every scan and will never be reported. Since the OS history is already a bounded ring buffer and maxNum = 0 means all retained matching records, could we request all records here and filter by reason and watermark afterward?
|
|
||
| // Advance per record — unparsable ones too, retrying can't succeed — | ||
| // so dying mid-scan never re-captures already-reported crashes. | ||
| preferences.setValue(LAST_CAPTURED_TIMESTAMP_KEY, exitInfo.timestamp) |
There was a problem hiding this comment.
blocking: This advances the watermark without knowing whether capture() durably queued the event. capture() returns Unit, and queue submission/storage failures are swallowed, so a rejected executor, serialization/disk failure, or close() disabling the client while this scanner is still running can drop the event while permanently marking the crash as handled. Could we advance the watermark only after acknowledged durable queue persistence, and ensure uninstall/close cancels or waits for the scanner so it cannot acknowledge a dropped capture?
|
A few additional follow-ups from the review:
|
|
Two additional behavioral points to address before a stable release:
|
| override fun install(postHog: PostHogInterface) { | ||
| this.postHog = postHog | ||
|
|
||
| if (integrationInstalled || Build.VERSION.SDK_INT < Build.VERSION_CODES.S) { |
There was a problem hiding this comment.
blocking: @Volatile only guarantees visibility; the check followed by assignment is a compound operation and is not atomic, so concurrent installs can both observe false and start duplicate scanners. uninstall() also clears the process-wide flag from any integration instance, even one that did not acquire it, which can allow another scanner to start while the original is still active. Could we use atomic acquisition (for example AtomicBoolean.compareAndSet), track ownership per integration instance, and release the guard only from the owner after its scanner has terminated?
|
left a few comments @cat-ph its in the right direction |
|
you can also check the https://github.com/abovevacant/epitaph TombstoneDecoder impl for parsing the exit metadata |
2 participants
💡 Motivation and Context
Native (NDK) crashes kill the process before any JVM handler runs, so today they are invisible to error tracking. This adds capture for them without shipping any native code in the SDK: on startup, the SDK reads the crash records the OS kept via
ApplicationExitInfo(REASON_CRASH_NATIVE, Android 12+), parses the attached tombstone protobuf, and captures one$exceptionevent per crash using the native stack frame contract PostHog already resolves for the Rust and Go SDKs.Per crash, the event carries:
platform: "native",instruction_addr,image_addr, optional client-resolvedfunction/symbol_addr) in canonical bottom-up order$debug_imagesentries derived from the tombstone's per-frame GNU build ids, so the server matches frames to.sosymbols uploaded withposthog-cli symbol-sets uploadSIGSEGV/SEGV_MAPERR at 0x..., abort message when present),$exception_level: fatal, and the original crash timestampDesign notes:
instruction_addris biased by +1: tombstone pcs are already the correct lookup address (the leaf is the faulting instruction and libunwindstack rewinds caller pcs to the call instruction), so the bias cancels the server's uniform -1 return-address adjustment. This is pinned by a cymbal fixture test on the server side.errorTrackingConfig.captureNativeCrashes, additionally gated on the project's exception autocapture remote toggle. A persisted timestamp watermark prevents duplicate capture across launches, advancing per record so dying mid-scan cannot re-capture./data/are in-app;/system,/apex,/vendorand unknown mappings are not.Known limitations (draft): events are associated with the identity at next launch, not at crash time;
$exception_stepsrecorded in the new process may attach to the previous run's crash; API 31+ only (tombstone protos attach from Android 12).💚 How did you test it?
PostHogAndroidTestconscrypt failures on that box reproduce onmain(arm64 environment issue, no conscrypt aarch64 linux artifact) and are unrelated.ApplicationExitInfotombstone through the scanner, verified one real SDK batch delivery plus watermark deduplication on a second scan, then confirmed issue creation and full Cymbal/UI symbolication against the Android ELF uploaded by feat(gradle-plugin): upload native debug symbols via symbol-sets upload #660, including inline frames and source lines.📝 Checklist
If releasing new changes
pnpm changesetto generate a changeset file