feat(server): expose in-app frame config and captureException options - #670
Conversation
b4e866a to
ffe3132
Compare
ffe3132 to
2174c3c
Compare
7e81e8d to
71672e6
Compare
9c02cbf to
d6d6b47
Compare
32939f8 to
4c2b895
Compare
|
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 |
- PostHogConfig.inAppIncludes/inAppExcludes control in_app frame classification (prefix match, excludes win); inAppExcludes defaults to DEFAULT_IN_APP_EXCLUDES (JDK/Kotlin/framework noise) so zero-config users get a your-code vs framework split out of the box. - New captureException(exception[, distinctId], options) overloads with the same option-merging semantics as capture(..., options): custom props, $groups, $set/$set_once, timestamp, and flag enrichment via snapshot or appendFeatureFlags; reserved props ($exception_level, ...) overridable via options properties; request-context resolution and personless fallback unchanged.
d6d6b47 to
709788c
Compare
🦔 ReviewHog reviewed this pull requestFound 0 must fix, 0 should fix, 1 consider. Published 1 finding (view the review). |
|
ReviewHog Alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏 |
| * overload and the properties one. | ||
| */ | ||
| public fun captureException( | ||
| exception: Throwable, | ||
| distinctId: String?, | ||
| options: PostHogCaptureOptions, |
There was a problem hiding this comment.
The new overload breaks existing Java calls with null properties
Why we think it's a valid issue
- Checked: the four existing
captureExceptionoverloads atposthog-server/src/main/java/com/posthog/server/PostHogInterface.kt:768-819, the two overloads the diff appends atPostHogInterface.kt:835-858, the Java-visible surface recorded inposthog-server/api/posthog-server.api:11-14, the equivalentcaptureoptions pattern atPostHogInterface.kt:148-182, the shape ofPostHogCaptureOptions, and every Java call site in the repo. - Found: the premise holds. The properties overload at
PostHogInterface.kt:768carries no@JvmSynthetic, unlike thecaptureproperties overloads atPostHogInterface.kt:90andPostHogInterface.kt:117. Java therefore sees bothcaptureException(Throwable, String, Map)(posthog-server.api:13) and the newcaptureException(Throwable, String, PostHogCaptureOptions).PostHogCaptureOptionsis a plain class and does not implementMap, so neither method is more specific andcaptureException(e, id, null)from Java fails with an ambiguity error. - Found: the break reaches only an untyped
nullliteral. A typed argument still resolves, and the one Java call site in the repo,posthog-samples/posthog-java-sample/src/main/java/com/posthog/java/sample/PostHogJavaExample.java:302, passes a typedMapvariable and keeps compiling. Kotlin callers are safe because theoptionsparameter is non-nullable, sonullmatches only the properties overload. Binary compatibility also holds, because compiled call sites name theMapdescriptor. - Found: the second suggested remedy does not work here. Hiding the new overload from Java would delete the only Java entry point for options, and the
captureprecedent only avoids the clash because its properties variants were@JvmSyntheticfrom the start. Hiding the existingcaptureExceptionproperties overload now would be a harder break than the one flagged. - Impact: a real but narrow source-compatibility regression in a minor release of a public SDK. Affected Java code fails at compile time with a loud
javacmessage, and one cast repairs it. No runtime behaviour changes, and no existing call silently rebinds to the options overload. - Priority: lowered to
consider. The claim that "existing source code will no longer compile" overstates the reach, since only an untypednullthird argument breaks. The author already states the required cast in the changeset and in the new KDoc atPostHogInterface.kt:831-833, so what remains is a naming preference on a disclosed trade-off rather than an undetected defect.
Issue description
The new three-argument overload makes captureException(exception, distinctId, null) ambiguous in Java. Existing source code will no longer compile. A cast in the changeset does not preserve source compatibility for this minor release.
Suggested fix
Use a distinct Java method name such as captureExceptionWithOptions. Alternatively, hide this overload from Java and expose a separate Java-friendly named method.
Prompt to fix with AI (copy-paste)
## Context
@posthog-server/src/main/java/com/posthog/server/PostHogInterface.kt#L833-838
<issue_description>
The new three-argument overload makes `captureException(exception, distinctId, null)` ambiguous in Java. Existing source code will no longer compile. A cast in the changeset does not preserve source compatibility for this minor release.
</issue_description>
<issue_validation>
- **Checked:** the four existing `captureException` overloads at `posthog-server/src/main/java/com/posthog/server/PostHogInterface.kt:768-819`, the two overloads the diff appends at `PostHogInterface.kt:835-858`, the Java-visible surface recorded in `posthog-server/api/posthog-server.api:11-14`, the equivalent `capture` options pattern at `PostHogInterface.kt:148-182`, the shape of `PostHogCaptureOptions`, and every Java call site in the repo.
- **Found:** the premise holds. The properties overload at `PostHogInterface.kt:768` carries no `@JvmSynthetic`, unlike the `capture` properties overloads at `PostHogInterface.kt:90` and `PostHogInterface.kt:117`. Java therefore sees both `captureException(Throwable, String, Map)` (`posthog-server.api:13`) and the new `captureException(Throwable, String, PostHogCaptureOptions)`. `PostHogCaptureOptions` is a plain class and does not implement `Map`, so neither method is more specific and `captureException(e, id, null)` from Java fails with an ambiguity error.
- **Found:** the break reaches only an untyped `null` literal. A typed argument still resolves, and the one Java call site in the repo, `posthog-samples/posthog-java-sample/src/main/java/com/posthog/java/sample/PostHogJavaExample.java:302`, passes a typed `Map` variable and keeps compiling. Kotlin callers are safe because the `options` parameter is non-nullable, so `null` matches only the properties overload. Binary compatibility also holds, because compiled call sites name the `Map` descriptor.
- **Found:** the second suggested remedy does not work here. Hiding the new overload from Java would delete the only Java entry point for options, and the `capture` precedent only avoids the clash because its properties variants were `@JvmSynthetic` from the start. Hiding the existing `captureException` properties overload now would be a harder break than the one flagged.
- **Impact:** a real but narrow source-compatibility regression in a minor release of a public SDK. Affected Java code fails at compile time with a loud `javac` message, and one cast repairs it. No runtime behaviour changes, and no existing call silently rebinds to the options overload.
- **Priority:** lowered to `consider`. The claim that "existing source code will no longer compile" overstates the reach, since only an untyped `null` third argument breaks. The author already states the required cast in the changeset and in the new KDoc at `PostHogInterface.kt:831-833`, so what remains is a naming preference on a disclosed trade-off rather than an undetected defect.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Use a distinct Java method name such as `captureExceptionWithOptions`. Alternatively, hide this overload from Java and expose a separate Java-friendly named method.
</potential_solution>
There was a problem hiding this comment.
Thanks — the analysis is accurate: because the existing captureException(Throwable, String?, Map?) overload isn't @JvmSynthetic (unlike the capture properties overloads), Java sees both it and the new PostHogCaptureOptions overload, so an untyped null third argument is ambiguous. As you note, this is narrow — only an untyped null literal breaks; typed arguments, Kotlin callers, and binary compatibility are all fine, and it fails loudly at compile time with a one-cast repair.
Declining, because this is a deliberate trade-off that's already documented, not an undetected defect. The required cast (Map<String, Object>) null is called out in both the KDoc on the overload and the changeset (.changeset/server-error-tracking-config.md). The two suggested remedies each cost more than the break they'd avoid:
- Renaming the Java entry point (
@JvmName("captureExceptionWithOptions")) would diverge from the siblingcapture(distinctId, event, options)API, which keeps the naturalcapturename for the options overload — socaptureExceptionwould be the odd one out. - Making the existing
captureException(Throwable, String?, Map?)overload@JvmSyntheticwould remove an already-shipped public Java method, which is a harder source/binary break than the one flagged here.
Given the impact is a compile-time-only, loudly-diagnosed, one-cast source change on a disclosed trade-off, the API stays as designed. Happy to revisit if a maintainer decides source compatibility should win over keeping the captureException/capture option APIs symmetric.
There was a problem hiding this comment.
leaving unresolved for thoughts, but I think it makes sense and simpler than 'polluting' with another method etc
Prompt To Fix All With AI### Issue 1
posthog-server/src/main/java/com/posthog/server/PostHog.kt:278-282
**Ignored exceptions bypass filtering**
When a throwable matching `errorTrackingConfig.ignoredExceptionTypes` is passed to either new options overload, this path proceeds directly to coercion without the filter used by `captureExceptionStateless`, causing an explicitly suppressed exception to be queued and sent as a `$exception` event.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "docs(server): use the canonical proguard..." | Re-trigger Greptile |
… options path The new captureException(throwable, distinctId, options) overload coerced the throwable itself and called captureStateless directly, so it never consulted errorTrackingConfig.ignoredExceptionTypes: a type the user suppressed still shipped as a $exception event, silently, when captured through options. Rather than paste the guard into the second path, route both through one entry point. PostHogStateless.captureExceptionEvent is now the single pre-capture route: it owns the ignoredExceptionTypes prefilter, the coerce-then-merge property order and the personless distinct-id fallback, and it accepts the event fields captureExceptionStateless cannot carry (groups, $set/$set_once, timestamp). captureExceptionStateless delegates to it and the server options overload calls it instead of hand-rolling the same steps, so the two paths cannot diverge again. Regression tests cover an ignored root type, an ignored type appearing only as a cause, and a non-ignored type still shipping with its merged options.
posthog-android Compliance ReportDate: 2026-08-19 12:25:37 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
|
…warding The captureException(throwable, distinctId, options) overload merged the caller properties before handing them to the shared core route, so with appendFeatureFlags=true a synchronous /flags request (carrying the distinct ID and person properties) fired even when the throwable matched ignoredExceptionTypes or the client was opted out — work and a network round trip spent on an event that was then dropped. captureExceptionEvent now takes the caller properties as a provider and checks the gates first: enabled, opt-out (mirroring captureStateless), then the ignoredExceptionTypes prefilter. The provider runs only after all three pass, so the server overload's mergeCaptureProperties (and the flag evaluation it may trigger) is never computed for a suppressed capture. captureStateless keeps its own checks — they are cheap and it is still reachable directly. Also drop the userProperties/userPropertiesSetOnce parameters: $exception is in the ingestion pipeline's no-person-update set and the error-tracking prepare step deletes $set/$set_once, so forwarding them onto an exception event could never update a person. options.userProperties stays as flag-evaluation input for the appendFeatureFlags path, and the KDoc on both options overloads now states the no-person-updates contract so it does not get re-added. The core api dump changes for the captureExceptionEvent signature only.
marandaneto
left a comment
There was a problem hiding this comment.
still some comments, but unblocking
DEFAULT_IN_APP_EXCLUDES is a @JvmField, so Java callers see a plain java.util.List, and `listOf(...)` with more than one element returns Arrays$ArrayList — fixed-size but mutable through set(). Every PostHogConfig and Builder that does not override inAppExcludes shares that one instance, so a single set() call could silently repoint in-app classification for the whole process. Wrap it in Collections.unmodifiableList (matching PostHogFeatureFlagEvaluations) so the attempt throws instead. The builder setters now copy the list they are handed for the same reason in reverse: a List<String> parameter is only read-only from Kotlin, so a caller that kept its own reference could mutate the config after build(). Core's errorTrackingConfig lists are per-instance mutableListOf() with no shared default, and inAppIncludes defaults to the immutable emptyList(), so neither needs the same treatment. Also complete the changeset metadata: the shared-core route change bumps posthog-server too, since posthog-server depends on :posthog.
💡 Motivation and Context
Second PR in the 4-PR JVM error-tracking stack. It exposes the in-app classification configuration surface on the server SDK and adds the missing
captureExceptionoverloads.releaseIdentifierexposure + stateless parity fix shipped separately in #668 (merged); this PR now carries only the in-app classification config and capture options.Contains:
feat(server): expose in-app frame config and captureException options:PostHogConfig.inAppIncludes/inAppExcludescontrolin_appclassification of captured frames (class-name prefix match; excludes always win).inAppExcludesdefaults to a newPostHogConfig.DEFAULT_IN_APP_EXCLUDESlist of common JVM/framework prefixes (JDK, Kotlin, Spring, Netty, servlet containers, HTTP clients, the PostHog SDK itself) so zero-config users get a sensible your-code vs framework split. Assigning your own list replaces the defaults.Builder.captureException(exception, distinctId, options)/captureException(exception, options)overloads takePostHogCaptureOptionswith the same merging semantics ascapture(..., options): custom properties,$groups,$set/$set_once, timestamp, and feature-flag enrichment via a pre-evaluatedflagssnapshot orappendFeatureFlags. Reserved exception properties (e.g.$exception_level,$exception_fingerprint) can be overridden through options properties. Request-context distinct-id resolution and personless fallback behave exactly like the existing overloads. Because the options overload sits next to the existing properties overload at the same arity, Java callers that passed an explicit untypednullas the third argument now need to cast it — called out in the changeset and the KDoc.docs(server): use the canonical proguard upload command in releaseIdentifier KDoc— the KDoc landed in fix: attach map_id to frames from stateless exception capture #668 cites the hiddenposthog-cli exp proguard uploadalias; use the canonicalposthog-cli proguard upload.💚 How did you test it?
posthog-server(7 inPostHogConfigTest, 6 inPostHogTest): defaults and overrides for the new config,DEFAULT_IN_APP_EXCLUDEScontents, exclude-beats-include classification, and each newcaptureExceptionoverload including options merging, reserved-property overrides, personless fallback, andmap_idpresence/absence on the options path../gradlew :posthog:test :posthog-server:test :posthog:apiCheck :posthog-server:apiCheckpass;spotlessCheckclean. API dumps regenerated; theposthog-serverdiff is additive only on top of fix: attach map_id to frames from stateless exception capture #668's entries, which now come frommain.📝 Checklist
If releasing new changes
pnpm changesetto generate a changeset file🔗 Stacked PR
Position 2 of 4. Base:
cat/java-et-coercer(PR #669).captureExceptionoptionscat/java-et-uncaught— opt-in server uncaught-exception capturecat/java-et-logback— newposthog-server-logbackappender moduleReview after its parent; the diff shown here is only this PR's own commits once the parent merges.