fix(error-tracking): install autocapture handler by default on first launch - #650
fix(error-tracking): install autocapture handler by default on first launch#650ioannisj wants to merge 4 commits into
Conversation
posthog-android Compliance ReportDate: 2026-07-29 06:41:48 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
|
bf14bc3 to
81ce5d7
Compare
…aunches skip default install
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
posthog/src/main/java/com/posthog/errortracking/PostHogErrorTrackingAutoCaptureIntegration.kt:45-50
**Cross-instance handler ownership breaks**
When two PostHog instances start with local autocapture enabled before remote configuration resolves, the first integration sets the process-wide installation flag and the second skips installation without saving the previous handler. Closing the second instance then observes the shared flag as installed, restores its null per-instance handler, and removes exception capture from the still-active first client while also discarding the original JVM handler.
Reviews (1): Last reviewed commit: "fix(error-tracking): cache disabled auto..." | Re-trigger Greptile |
turnipdabeets
left a comment
There was a problem hiding this comment.
LGTM, left some comments. Do you think we need to address RN as well?
Yeah and we'd probably have to gate flutter (dart side) exceptions as well I think |
| if (adapterExceptionHandler.getDefaultUncaughtExceptionHandler() === this) { | ||
| adapterExceptionHandler.setDefaultUncaughtExceptionHandler(defaultExceptionHandler) | ||
| } | ||
| integrationInstalled = false |
There was a problem hiding this comment.
The guard does fix the clobbering — confirmed with a fake adapter, the app's handler now survives. But when the restore is skipped we still set integrationInstalled = false while we're actually still linked as that handler's delegate, and two things follow from it.
uncaughtException has no disabled check, so we keep capturing $exception for a project that just turned autocapture off — the kill-switch stops killing. And because the flag says uninstalled, a later onRemoteConfig re-enable runs install() again and sets defaultExceptionHandler to the app handler that delegates back to us; one crash then cycles (my probe re-entered the chain 21 times before I capped it, so in production that's a StackOverflowError inside the crash handler).
I think we need to separate "are we linked" from "are we capturing":
captureEnabled = false
if (adapterExceptionHandler.getDefaultUncaughtExceptionHandler() === this) {
adapterExceptionHandler.setDefaultUncaughtExceptionHandler(defaultExceptionHandler)
integrationInstalled = false // only uninstalled once we're actually out of the chain
}with uncaughtException bailing when captureEnabled is false, and install() flipping it back on for the already-installed case.
💡 Motivation and Context
Fixes #648.
On the very first app launch there's no disk-cached remote config yet, so the native crash handler (
PostHogErrorTrackingAutoCaptureIntegration, aThread.UncaughtExceptionHandler) didn't install until the async/flagsremote-config response arrived. Any uncaught exception in that window, from app start to the first config response, was lost. Offline made it worse: a failed first-launch fetch left autocapture off for the whole session.This flips the gate to match the iOS fix. The handler now installs by default at setup when local
errorTrackingConfig.autoCaptureis on, and only skips when a remote config that actually exists (fetched this session or disk-cached from a prior launch) explicitly reportsautocaptureExceptions: false. Remote config becomes a kill-switch rather than a prerequisite: if the freshly-loaded config disables autocapture,onRemoteConfiguninstalls the handler, and a failed fetch leaves the default install in place. LocalautoCapturestays the primary gate, so a host that turned it off still gets no handler.To tell "first launch, no config yet" apart from "config exists and says disabled",
PostHogRemoteConfiggainshasCachedErrorTrackingConfig(), set at startup when the error-tracking config was already on disk. It's the Android analogue of iOS'shasCachedRemoteConfig.This is the Android counterpart of iOS #551, ported from iOS #731. It also covers Flutter-on-Android, which maps
captureNativeExceptionsontoerrorTrackingConfig.autoCaptureand inherited the same gap.💚 How did you test it?
Unit tests mirroring the iOS installation tests, in
PostHogErrorTrackingAutoCaptureIntegrationTest:autoCaptureis off, even with no remote configonRemoteConfiguninstalls the default install when a freshly-loaded config disables autocapture, and keeps it when the config enables autocaptureloaded = false) keeps the default installRan
./gradlew spotlessApply,:posthog:apiCheck, and the full:posthog:testsuite, all green.📝 Checklist
If releasing new changes
pnpm changesetto generate a changeset file🤖 Agent context
DRI: @ioannisj
Autonomy: Human-driven (agent-assisted)