feat(server): opt-in uncaught exception capture - #671
Merged
Conversation
This was referenced Aug 3, 2026
cat-ph
force-pushed
the
cat/java-et-server-config
branch
from
August 3, 2026 23:03
b4e866a to
ffe3132
Compare
cat-ph
force-pushed
the
cat/java-et-uncaught
branch
from
August 3, 2026 23:03
29a1276 to
c9db1af
Compare
cat-ph
force-pushed
the
cat/java-et-server-config
branch
from
August 5, 2026 11:41
ffe3132 to
2174c3c
Compare
cat-ph
force-pushed
the
cat/java-et-uncaught
branch
from
August 5, 2026 11:41
c9db1af to
cf4a852
Compare
cat-ph
force-pushed
the
cat/java-et-server-config
branch
from
August 7, 2026 21:54
2174c3c to
9c02cbf
Compare
cat-ph
force-pushed
the
cat/java-et-uncaught
branch
from
August 7, 2026 21:54
cf4a852 to
5052ac5
Compare
cat-ph
force-pushed
the
cat/java-et-server-config
branch
from
August 7, 2026 22:21
9c02cbf to
d6d6b47
Compare
cat-ph
force-pushed
the
cat/java-et-uncaught
branch
from
August 7, 2026 22:21
5052ac5 to
b276e0d
Compare
Contributor
|
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the |
cat-ph
force-pushed
the
cat/java-et-uncaught
branch
from
August 18, 2026 15:09
b276e0d to
899946e
Compare
cat-ph
force-pushed
the
cat/java-et-server-config
branch
from
August 18, 2026 15:09
d6d6b47 to
709788c
Compare
Contributor
🦔 ReviewHog reviewed this pull requestFound 2 must fix, 1 should fix, 0 consider. Published 3 findings (view the review). |
Contributor
|
ReviewHog Alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏 |
![posthog[bot]](https://avatars.githubusercontent.com/in/1203049?s=60&v=4){kind=small}
Adds captureUncaughtExceptions (default false) to the server PostHogConfig. When enabled, the core PostHogErrorTrackingAutoCaptureIntegration installs a Thread.defaultUncaughtExceptionHandler that captures the throwable as a fatal, unhandled $exception (mechanism UncaughtExceptionHandler), flushes, then delegates to the previously installed handler. Core changes (all additive; Android behavior and the released install(PostHogInterface) path unchanged): - Gate strategy seam on the integration so the server can install with a local-only gate (no remote config, which the server SDK never fetches); Android keeps the remote errorTracking.autocaptureExceptions gate. - Captures flow through an internal CaptureTarget seam so the server's stateless client can drive the integration. - Handler-install ownership is tracked per integration instance, so closing a second opted-in client (whose install was a process-wide no-op) does not tear down the handler a still-open first client owns. - New @PostHogInternal PostHogCapturedThrowables identity marker (weak, ReferenceQueue-pruned). The guard is directional: log mirrors consult it, the uncaught handler only marks — a crash is always captured as the authoritative fatal/unhandled record even if the same instance was logged first (logger.error(..., e); throw e), and marking keeps post-crash log mirrors from re-reporting it. - Repeated setup() cannot replace the owning integration with a non-owning one, which would leave the global handler installed after close(). - With no previous default handler to chain to, the handler reproduces the JVM's built-in stderr crash output so enabling capture never hides crashes from stderr log collection. - Server config KDoc documents the flushAt implication for the crash path.
The server PostHog.setup/close overrides did their integration lifecycle work outside setupLock. Two concurrent setup() calls could both read alreadySetUp as false, letting a rejected call install an uncaught-exception handler bound to a config the base discarded; and a close() racing setup() could read uncaughtExceptionIntegration before it was assigned, leaking the process-wide handler after the client closed. Wrap both the setup and close bodies in synchronized(setupLock) — the same reentrant monitor the base uses and the core client installs its integrations under — so the enabled-transition check + handler install are atomic with the base's setup, and the field is only ever touched under the lock. No behavior change for the single-client-per-process path. Paths: posthog-server/src/main/java/com/posthog/server/PostHog.kt Generated-By: PostHog Desktop Task-Id: 468ded4c-ba2e-440c-8096-5d78489afcdc
cat-ph
force-pushed
the
cat/java-et-uncaught
branch
from
August 20, 2026 16:59
598f163 to
06affef
Compare
Contributor
posthog-android Compliance ReportDate: 2026-08-25 17:42:38 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
|
Contributor
Prompt To Fix All With AI### Issue 1
posthog-server/src/main/java/com/posthog/server/internal/PostHogMemoryQueue.kt:107-113
**Bounded flush misses crash event**
When at least `maxBatchSize` older events are queued, this single barrier flush sends only the oldest bounded batch and returns while the newly appended fatal event remains queued, causing JVM termination to discard the crash without a network attempt.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "fix(server): flush the crash capture beh..." | Re-trigger Greptile |
If capture or flush throws inside uncaughtException, the failure used to escape the handler, so the previous default handler (the app's own crash handling) and the stderr fallback never ran — enabling PostHog could replace the application's crash handling. Catch capture-path failures, log them, and always fall through to delegation.
…path Key the crash path off the same fatal-record marker the core PostHogQueue uses (PostHogEvent.isFatalExceptionEvent) instead of a client-side downcast to flushBlocking: a fatal $exception event's enqueue and send now run as one ordered task on the queue executor, draining batch by batch (ignoring flushAt) until the queue is empty or the bounded timeout is spent, while the crashing thread waits at most FATAL_FLUSH_TIMEOUT_MS. This moves the crash-delivery logic out of PostHog.kt (the capture target now just flushes), narrows the scheduling catch to RejectedExecutionException, and fixes the bounded-batch gap where a backlog of maxBatchSize or more older events left the crash event stranded behind a single capped batch.
PostHogCapturedThrowables has no consumer in this PR — the consulting side is the posthog-server-logback appender's log-mirror dedup. Move the guard, its marking call, its tests and its changeset to that stacked PR so the guard lands together with its consumer.
…he capture hook Per the sdk-specs exception-event-metadata spec, mechanism.type names the semantic capture-boundary category — the canonical one for an uncaught handler is onuncaughtexception, not the noncanonical UncaughtExceptionHandler — and the concrete integration hook belongs in event-level $exception_source. The server SDK now stamps jvm.uncaught_exception_handler on its uncaught captures, following the lowercase <technology>.<stable_hook> convention.
…fatal On the JVM an uncaught exception kills only its thread; the process continues unless the crashing thread is main. Per the sdk-specs boundary table, fatal is reserved for boundaries expected to terminate the process, so the integration now takes a layer-supplied per-thread fatal policy: the server SDK marks a main-thread crash fatal (blocking bounded delivery) and any other thread's error (regular async delivery, handled=false either way). Android keeps the always-fatal default, which is correct there — any uncaught exception kills the app.
The process-wide handler is first-owner-wins and does not transfer when the owning client closes; state that in the public captureUncaughtExceptions documentation instead of only in an implementation comment.
The memory queue created its Timer non-daemon, unlike the core PostHogQueue's Timer(true) and the SDK's daemon executor threads, so any set-up client kept the JVM alive after main finished — or crashed — until close() was called. Found by the new forked-JVM crash tests: the main-crash fixture never exited.
The in-process suite invokes the handler directly, which cannot prove real process exit, worker-thread survival, or the stderr the JVM actually emits. Add a CrashFixture main class launched via ProcessBuilder: a main-thread crash exits 1 with the default crash banner on stderr and delivers a fatal event first; a worker-thread crash leaves the process running (exit 0) and delivers an error-level event.
A periodic-timer flush racing the crash made the fatal drain a silent no-op: the drain saw the isFlushing flag held, made no progress, and returned with the crash event still queued. Poll the flag within the bounded budget instead of bailing; a failed send (batch requeued, no progress) still exits — one straight-line attempt per batch, never a retry loop.
A backlog batch failing with a retriable error is requeued and stops the drain (one straight-line attempt per batch), which consumed the whole crash budget before the appended fatal event ever reached the wire. Front-insert the fatal event instead, so the first — possibly only — wire attempt carries it; batch order does not matter to ingestion. Found by codex review.
The capture target's post-capture sweep called the public flush(), which runs an HTTP batch inline on the crashing thread with no timeout — e.g. retrying a batch a 5xx had just requeued — stalling crash delegation past the advertised bound. The fatal path already drains the queue within its budget, so the sweep is now a documented no-op; worker-thread (non-fatal) captures leave the process alive for the periodic flush. Also hardened the subprocess harness (stream drains on their own threads + destroyForcibly on timeout) and documented the fatal-policy approximation limits. Found by codex review.
Front-inserting the fatal event broke the head-is-oldest eviction invariant: in a process that survived a failed crash-send, ordinary captures reaching maxQueueSize evicted the deque head — the crash event awaiting retry. Capacity trimming now evicts the oldest non-fatal event (O(1) in the common case). Found by codex review.
The fatal-eviction preference returned no victim when every queued event was fatal, letting repeated failed fatal submissions in a surviving process grow the deque past the cap. Evict the oldest event in that case — the cap wins over fatal priority. Found by codex review.
Two corner cases in the all-fatal-queue fallback: ordinary traffic could displace a parked crash report (the incoming ordinary event is now dropped instead), and fatal-on-fatal trimming removed the head — the NEWEST record, since fatal events are front-inserted — instead of the oldest at the tail. maxQueueSize stays a hard bound throughout. Found by codex review.
A concurrent failed flush requeues its batch at the head, which can reorder parked fatal events; which of several parked crash reports gets trimmed at capacity is deliberately not age-tracked. Comment-only.
…lush (#717) and shutdown flush
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
removedEvent is null-guarded right above; K2 smart-casts the local captured by the inline synchronized block, so the safe-call was a useless null check.
marandaneto
reviewed
Aug 25, 2026
marandaneto
left a comment
marandaneto
left a comment
Member
There was a problem hiding this comment.
Automated advisory code review.
marandaneto
reviewed
Aug 25, 2026
marandaneto
reviewed
Aug 25, 2026
marandaneto
approved these changes
Aug 25, 2026
marandaneto
left a comment
marandaneto
left a comment
Member
There was a problem hiding this comment.
approving to unblock, still a few comments left
Thread.setDefaultUncaughtExceptionHandler can throw (SecurityException under a SecurityManager), and the process-wide install flag was set before that call — a denied install leaked the flag, permanently blocking any later installation, and could leave a half-initialized client. The ownership state now rolls back before the failure propagates to the caller's per-integration handling.
…class The capture target, fatal policy and $exception_source stamping now live in an internal PostHogUncaughtExceptionCapture helper; PostHog.setup only calls install(). A denied handler installation logs and leaves the client running with capture disabled instead of failing setup. Changesets trimmed to customer-facing one-liners.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
💡 Motivation and Context
Third PR in the 4-PR JVM error-tracking stack: opt-in capture of uncaught JVM exceptions for the server SDK.
PostHogConfig.captureUncaughtExceptions(also on theBuilder), default off. When enabled,PostHog.setupinstalls a globalThread.defaultUncaughtExceptionHandlerthat captures the crashing exception as a fatal, unhandled$exceptionevent (mechanismUncaughtExceptionHandler), flushes, then delegates to the previously registered handler.close()removes it again.PostHogErrorTrackingAutoCaptureIntegrationtakes an optional caller-suppliedenabledGateinstead of always using the built-in gate (localerrorTrackingConfig.autoCapture+ remote config as a kill-switch, i.e. the behavior from Bug: error tracking autocapture misses crashes on first launch before remote config resolves #648 is preserved for Android).CaptureTargetseam (installWith), because the corePostHogInterfaceand the stateless server client share no capture supertype.Exception in thread "..."stderr output, so opting into capture never hides a crash from stderr log collection.PostHogCapturedThrowablesguard: a weak, identity-keyed, process-wide set letting independent capture paths avoid double-reporting the sameThrowableinstance. It is deliberately directional — the uncaught handler only marks (a crash is always captured as the authoritative fatal/unhandled record, even if the same instance was logged first), while log-mirror paths (the appender in PR 4) consult it and skip instances already reported. It never keeps a throwable or its stack alive.Delivery caveat documented in the KDoc and changeset:
flush()drains the queue synchronously on the crashing thread, but the preceding capture enqueues asynchronously, so under an immediate hard exit the final exception is best-effort — the same guarantee the Android SDK gives. Services that care should keepflushAtlow.💚 How did you test it?
PostHogErrorTrackingAutoCaptureIntegrationTest(local-only gate installs/refuses,CaptureTargetcapture+flush, capture-not-suppressed-by-dedup, marking for later log mirrors, JVM stderr fallback, non-owning instance cannot tear down the installed handler), 2 inPostHogCapturedThrowablesTest(identity keying, value-equal throwables both captured), and 6 in the new serverPostHogUncaughtExceptionTest(off by default, install+chain+restore on close, fatal/unhandled/mechanism assertions over a real/batchrequest, repeated setup keeps ownership, second client does not stack a handler, works with no remote config)../gradlew :posthog:test :posthog-server:testpass (PostHogErrorTrackingAutoCaptureIntegrationTest33 tests,PostHogCapturedThrowablesTest2,PostHogUncaughtExceptionTest6), plus:posthog:apiCheck/:posthog-server:apiCheckandspotlessCheck. API dumps regenerated; additive only.📝 Checklist
If releasing new changes
pnpm changesetto generate a changeset file🔗 Stacked PR
Position 3 of 4. Base:
cat/java-et-server-config(PR #670).captureExceptionoptionscat/java-et-logback— newposthog-server-logbackappender module