feat(error-tracking): complete exception chain metadata and in-app classification - #669
Open
cat-ph wants to merge 8 commits into
Open
feat(error-tracking): complete exception chain metadata and in-app classification#669cat-ph wants to merge 8 commits into
cat-ph wants to merge 8 commits into
Conversation
This was referenced Aug 3, 2026
Contributor
posthog-android Compliance ReportDate: 2026-08-07 22:27:34 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
|
cat-ph
force-pushed
the
cat/java-et-coercer
branch
from
August 5, 2026 11:41
7e81e8d to
71672e6
Compare
Contributor
🦔 ReviewHog reviewed this pull requestNothing worth raising this time, so here's a calming picture instead:
|
Contributor
|
Reviews (1): Last reviewed commit: "fix(error-tracking): detect Android D8/R..." | Re-trigger Greptile |
marandaneto
reviewed
Aug 7, 2026
marandaneto
approved these changes
Aug 7, 2026
marandaneto
reviewed
Aug 7, 2026
Comment on lines
+10
to
+12
| // Do NOT add new options as constructor params: appending even a defaulted param rewrites the | ||
| // Kotlin `$default` synthetic constructor descriptor, so already-compiled consumers hit a | ||
| // NoSuchMethodError on `PostHogErrorTrackingConfig()`. New options go in the class body. |
Member
There was a problem hiding this comment.
In a major release, remove constructor defaults and declare every trailing overload explicitly. Then no $default constructor is generated, and future
options can add a new full constructor while retaining all previously shipped overloads. For arbitrary named optional settings, a builder/configuration
block is the better API.
Contributor
Author
There was a problem hiding this comment.
Agreed, and worth writing down. The synthetic $default constructor is exactly the hazard you describe: adding a defaulted parameter changes its descriptor, so every previously compiled call site breaks at runtime — this area has already bitten us twice.
The body-property shape here was the deliberate choice for that reason (properties are additive, no constructor descriptor to break), but it does not solve the general case.
No code change on this PR. Noting the explicit-trailing-overloads / builder-or-configuration-block approach for the next major, where we can drop the constructor defaults outright.
Contributor
Author
There was a problem hiding this comment.
🤖 ^ but true, didn't change anything, did you want me to add a note in the comments here?
5 tasks
…assification Fills in the exception-item model the shared ThrowableCoercer emits: - Mechanisms carry exception_id (0-based position in $exception_list); cause items get parent_id and mechanism type "chained". A single-item list carries no ids at all, matching the other SDKs. - Suppressed exceptions (Throwable.suppressed, one level) are serialized after the cause chain with mechanism type "suppressed" and their holder's parent_id. - Caps: 50 items per $exception_list (keeping the primary and nearest causes) and 64 frames per stacktrace (keeping the frames nearest the crash). - JVM-synthesized frames (lambdas, Spring CGLIB proxies, reflection accessors, dynamic proxies) are flagged synthetic: true instead of being dropped. - New PostHogErrorTrackingConfig.inAppExcludes forces frames out of in_app; excludes win over inAppIncludes. All key names and platform: "java" are unchanged, so the additions are backwards compatible on the wire.
…stable Appending `inAppExcludes` as a trailing defaulted constructor param rewrote the Kotlin `$default` synthetic constructor descriptor, so a consumer compiled against the previous release hit a NoSuchMethodError even for a bare `PostHogErrorTrackingConfig()`. Declare it as a body property instead, which restores every constructor descriptor byte-for-byte and leaves only an additive getter in the API dump. Call sites are unchanged: the list is mutated through the property either way. Also document the matching caveats on both in-app lists: prefixes are compared against runtime class names before symbolication, so on minified (ProGuard/R8) builds they generally will not match, and PostHog re-derives `in_app` server-side after deobfuscation. Making excludes survive deobfuscation needs a server-side in-app contract (follow-up).
Frame-level `synthetic` is the common field meaning "the SDK constructed this frame", which is not what the lambda/CGLIB/reflection/proxy heuristics detect. Java frames have a dedicated `method_synthetic` field for "the compiler generated this method", so emit that instead (still omitted when false). Adds a regression assertion that the common `synthetic` frame field is never emitted.
The 50-item cap only trimmed the output: the coercer walked the whole cause chain plus every suppressed set into intermediate lists and sliced afterwards, so the cap did not bound the work at all. Follow `cause` only while there is capacity left, then let suppressed exceptions fill the remainder — same deterministic order and same output, no unbounded intermediate collections. The identity-based circular guard cannot stop a chain whose `cause` returns a fresh instance on every read, so the walk bound is what makes that terminate; covered by a test that asserts both the item count and the number of `cause` reads.
…in-app `exception_id`/`parent_id` are emitted on the wire, but PostHog's ingestion drops them today — its mechanism schema does not model the ids yet, so the chain relationships are not persisted until that server-side change (in flight) lands. Say so in the changeset and next to the code that emits them instead of implying end-to-end support. Also record the in-app matching caveat (runtime class names, ProGuard/R8 obfuscation, server-side reclassification after deobfuscation) and the fact that the item cap now bounds the traversal.
… synthetic The heuristics only matched javac's `lambda$...` methods and the `$$Lambda` class marker, so on the Android runtime — the SDK's primary target — modern D8/R8 output slipped through: desugared lambdas are named `Foo$$ExternalSyntheticLambda0` (only the legacy `-$$Lambda$Foo$hash` form contained `$$Lambda`) and Kotlin's invokedynamic lambda bodies are named `onCreate$lambda$3`. Match the D8/R8 `$$ExternalSynthetic`/`$$InternalSynthetic` markers (which also cover outlined methods) and the Kotlin `$lambda$` method marker, with regression cases for each plus a negative for class names that merely contain "Synthetic".
cat-ph
force-pushed
the
cat/java-et-coercer
branch
from
August 7, 2026 22:21
32939f8 to
4c2b895
Compare
Contributor
|
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
2 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
First PR in a 4-PR stack that brings JVM/server error tracking up to parity with the other PostHog SDKs.
This one completes the exception-item model that the shared
ThrowableCoerceremits. Today every$exception_listitem is serialized in isolation: there are no chain ids (the code still carried a// TODO: exception_id and parent_id), suppressed exceptions are dropped, nothing bounds the payload, and there is no way to force third-party frames out ofin_app.What changed:
exception_id(0-based position); cause items also getparent_idand mechanismtype: "chained", while the primary item keeps its own mechanism type. A single-item list carries no ids at all, matchingposthog-rs(which only links a chain when there is more than one exception). The ids are emitted on the wire; persisting the relationships needs the server-side mechanism-schema change (see "Review round 1" below).Throwable.suppressed(one level, bounded) is serialized after the cause chain with mechanismtype: "suppressed"and the holder'sparent_id.$exception_listand 64 frames per stacktrace (keeping the frames nearest the crash). The 50-item cap bounds the traversal itself, not just the output.method_synthetic: truerather than dropped.inAppExcludes— newPostHogErrorTrackingConfig.inAppExcludesforces frames out ofin_app; excludes win overinAppIncludes.Notes for reviewers:
platform: "java"is unchanged, and the new fields are omitted rather than sent asfalse/nullwhen they do not apply.inAppExcludesis a body property, not a constructor param, so every constructor descriptor ofPostHogErrorTrackingConfig— including the Kotlin$defaultsynthetic — is byte-identical tomain; the only API-dump change for that class is the added getter.ThrowableCoercer.fromThrowableToPostHogPropertiesgains a trailing defaultedinAppExcludesparam. Kotlin callers are source-compatible; the JVM descriptor changes, which is fine for a@PostHogInternalentry point.💚 How did you test it?
ThrowableCoercerTest(11 tests) covering single-item id omission, a 3-deep cause chain, suppressed exceptions, both caps, bounded traversal of an endless cause chain, suppressed-fills-leftover-capacity, every synthetic-frame heuristic (with negatives), and excludes-beat-includes.PostHogTestexception assertions for the new mechanism fields (including the single-item case), on top of the ordering assertions from feat: send error tracking stack frames in canonical bottom-up order #603../gradlew :posthog:testand:posthog:apiCheckpass;posthog/api/posthog.apiregenerated withapiDumpand the diff is additive apart from the documented@PostHogInternaldefaulted-arg descriptors.spotlessCheckclean.📝 Checklist
If releasing new changes
pnpm changesetto generate a changeset file🔗 Stacked PR
Position 1 of 4. Base:
main.cat/java-et-server-config— server error-tracking config andcaptureExceptionoptionscat/java-et-uncaught— opt-in server uncaught-exception capturecat/java-et-logback— newposthog-server-logbackappender modulePlease review and merge in stack order; each PR targets the previous branch.