Skip to content

Commit d11c33d

Browse files
tsushanthtsushanth
committed
Move ignoredExceptionTypes filter to beforeSend so manual capture is filtered too
1 parent 7262292 commit d11c33d

4 files changed

Lines changed: 219 additions & 71 deletions

File tree

‎posthog/src/main/java/com/posthog/errortracking/PostHogErrorTrackingAutoCaptureIntegration.kt‎

Lines changed: 27 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ public class PostHogErrorTrackingAutoCaptureIntegration : PostHogIntegration, Th
1111
private val adapterExceptionHandler: UncaughtExceptionHandlerAdapter
1212
private var defaultExceptionHandler: Thread.UncaughtExceptionHandler? = null
1313
private var postHog: PostHogInterface? = null
14+
private var ignoredExceptionsBeforeSend: PostHogIgnoredExceptionTypesBeforeSend? = null
1415

1516
public constructor(config: PostHogConfig) {
1617
this.config = config
@@ -39,6 +40,12 @@ public class PostHogErrorTrackingAutoCaptureIntegration : PostHogIntegration, Th
3940
return
4041
}
4142

43+
// Register the ignored-types filter at the beforeSend layer so it
44+
// applies to manually-captured exceptions too — per @marandaneto's
45+
// review on #569: dropping inside this integration's uncaughtException
46+
// handler missed captureException() callers with the same throwable.
47+
registerIgnoredExceptionsBeforeSend()
48+
4249
val currentExceptionHandler = adapterExceptionHandler.getDefaultUncaughtExceptionHandler()
4350

4451
if (currentExceptionHandler != null) {
@@ -59,11 +66,25 @@ public class PostHogErrorTrackingAutoCaptureIntegration : PostHogIntegration, Th
5966
config.logger.log("Exception autocapture is enabled.")
6067
}
6168

69+
private fun registerIgnoredExceptionsBeforeSend() {
70+
if (ignoredExceptionsBeforeSend != null) return
71+
val hook = PostHogIgnoredExceptionTypesBeforeSend(config)
72+
config.addBeforeSend(hook)
73+
ignoredExceptionsBeforeSend = hook
74+
}
75+
76+
private fun unregisterIgnoredExceptionsBeforeSend() {
77+
val hook = ignoredExceptionsBeforeSend ?: return
78+
config.removeBeforeSend(hook)
79+
ignoredExceptionsBeforeSend = null
80+
}
81+
6282
override fun uninstall() {
6383
if (!integrationInstalled) {
6484
return
6585
}
6686
adapterExceptionHandler.setDefaultUncaughtExceptionHandler(defaultExceptionHandler)
87+
unregisterIgnoredExceptionsBeforeSend()
6788
integrationInstalled = false
6889
config.logger.log("Exception autocapture is disabled.")
6990
}
@@ -82,41 +103,17 @@ public class PostHogErrorTrackingAutoCaptureIntegration : PostHogIntegration, Th
82103
throwable: Throwable,
83104
) {
84105
postHog?.let { postHog ->
85-
if (!isIgnored(throwable)) {
86-
postHog.captureException(PostHogThrowable(throwable, thread))
87-
postHog.flush()
88-
} else {
89-
// The match may have been against a cause anywhere in the chain,
90-
// not necessarily the outermost throwable, so we deliberately
91-
// don't pin the log to `throwable.javaClass.name` — that would
92-
// mislead anyone looking at the logs and seeing e.g. a bare
93-
// RuntimeException reported as suppressed when their ignore
94-
// list only mentions com.facebook.react.common.JavascriptException.
95-
config.logger.log(
96-
"Skipping autocapture: ${throwable.javaClass.name} (or a cause in its chain) matches ignoredExceptionTypes",
97-
)
98-
}
106+
// Always hand the throwable to captureException; the ignored-types
107+
// filter runs inside the beforeSend chain (registered above) so
108+
// both this autocapture path and manual captureException() calls
109+
// go through the same drop logic.
110+
postHog.captureException(PostHogThrowable(throwable, thread))
111+
postHog.flush()
99112
}
100113

101114
// Always chain to the next handler so the process terminates / RN's red-box
102115
// surfaces / etc. behave the same way as before, regardless of whether we
103116
// emitted a $exception event.
104117
defaultExceptionHandler?.uncaughtException(thread, throwable)
105118
}
106-
107-
private fun isIgnored(throwable: Throwable): Boolean {
108-
val ignored = config.errorTrackingConfig.ignoredExceptionTypes
109-
if (ignored.isEmpty()) return false
110-
111-
// Walk the cause chain so a wrapped exception (e.g. RuntimeException wrapping
112-
// a JavascriptException) is matched too. The seen-set guards against the
113-
// pathological self-referential cause chains that some JVM libs construct.
114-
var current: Throwable? = throwable
115-
val seen = mutableSetOf<Throwable>()
116-
while (current != null && seen.add(current)) {
117-
if (ignored.contains(current.javaClass.name)) return true
118-
current = current.cause
119-
}
120-
return false
121-
}
122119
}

‎posthog/src/main/java/com/posthog/errortracking/PostHogIgnoredExceptionTypesBeforeSend.kt‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
package com.posthog.errortracking
2+
3+
import com.posthog.PostHogBeforeSend
4+
import com.posthog.PostHogConfig
5+
import com.posthog.PostHogEvent
6+
import com.posthog.PostHogEventName
7+
8+
/**
9+
* Drops `$exception` events whose `$exception_list` contains an entry whose
10+
* `type` matches any entry of [PostHogErrorTrackingConfig.ignoredExceptionTypes].
11+
*
12+
* Applied at the [PostHogBeforeSend] layer rather than inside the uncaught
13+
* handler so the filter covers manually-captured exceptions (via
14+
* `PostHog.captureException(...)`) as well as autocaptured ones — without the
15+
* filter at this layer, callers could bypass it just by calling
16+
* `captureException` directly, which is the bug @marandaneto called out in
17+
* https://github.com/PostHog/posthog-android/pull/569.
18+
*
19+
* Walks the full `$exception_list` (each entry is one throwable in the cause
20+
* chain — `ThrowableCoercer.fromThrowableToPostHogProperties` writes them in
21+
* outer-to-inner order), so a `RuntimeException` wrapping a
22+
* `JavascriptException` is filtered when the ignore list mentions either.
23+
*/
24+
internal class PostHogIgnoredExceptionTypesBeforeSend(
25+
private val config: PostHogConfig,
26+
) : PostHogBeforeSend {
27+
override fun run(event: PostHogEvent): PostHogEvent? {
28+
if (event.event != PostHogEventName.EXCEPTION.event) return event
29+
30+
val ignored = config.errorTrackingConfig.ignoredExceptionTypes
31+
if (ignored.isEmpty()) return event
32+
33+
val list = event.properties?.get("\$exception_list") as? List<*> ?: return event
34+
for (entry in list) {
35+
val type = (entry as? Map<*, *>)?.get("type") as? String ?: continue
36+
if (type in ignored) {
37+
config.logger.log(
38+
"Skipping \$exception event: $type (or a cause in its chain) matches ignoredExceptionTypes",
39+
)
40+
return null
41+
}
42+
}
43+
return event
44+
}
45+
}

‎posthog/src/test/java/com/posthog/errortracking/PostHogErrorTrackingAutoCaptureIntegrationTest.kt‎

Lines changed: 12 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -154,71 +154,42 @@ internal class PostHogErrorTrackingAutoCaptureIntegrationTest {
154154
}
155155

156156
@Test
157-
fun `uncaughtException skips capture when throwable class is in ignoredExceptionTypes`() {
157+
fun `install registers PostHogIgnoredExceptionTypesBeforeSend so captureException callers are filtered too`() {
158158
whenever(mockConfig.remoteConfigHolder).thenReturn(mockRemoteConfig)
159159
whenever(mockRemoteConfig.isAutocaptureExceptionsEnabled()).thenReturn(true)
160160
whenever(mockAdapter.getDefaultUncaughtExceptionHandler()).thenReturn(mockExceptionHandler)
161161

162-
val thread = Thread.currentThread()
163-
// Simulates the React Native scenario: posthog-js has already captured the
164-
// fatal JS error; React Native rethrows it natively as JavascriptException,
165-
// and the SDK should not emit a duplicate $exception event.
166-
val throwable = ReactNativeJavascriptExceptionStub("Unhandled JS Exception: ReferenceError")
167-
168162
val integration =
169-
getSut(ignoredExceptionTypes = listOf(ReactNativeJavascriptExceptionStub::class.java.name))
163+
getSut(ignoredExceptionTypes = listOf("com.facebook.react.common.JavascriptException"))
170164
integration.install(mockPostHog)
171165

172-
integration.uncaughtException(thread, throwable)
173-
174-
verify(mockPostHog, never()).captureException(any<PostHogThrowable>(), anyOrNull())
175-
// The downstream handler still runs so the process terminates / RN's red-box
176-
// appears as it would without PostHog installed.
177-
verify(mockExceptionHandler).uncaughtException(thread, throwable)
166+
// Per @marandaneto's review on #569: the ignored-types filter must run
167+
// at the beforeSend layer, not inside this integration's uncaughtException
168+
// handler, so manually-captured exceptions are dropped too.
169+
verify(mockConfig).addBeforeSend(any<PostHogIgnoredExceptionTypesBeforeSend>())
178170

179171
integration.uninstall()
172+
verify(mockConfig).removeBeforeSend(any<PostHogIgnoredExceptionTypesBeforeSend>())
180173
}
181174

182175
@Test
183-
fun `uncaughtException skips capture when ignored class is anywhere in the cause chain`() {
176+
fun `uncaughtException always captures — drop decision lives in the beforeSend chain`() {
184177
whenever(mockConfig.remoteConfigHolder).thenReturn(mockRemoteConfig)
185178
whenever(mockRemoteConfig.isAutocaptureExceptionsEnabled()).thenReturn(true)
186179
whenever(mockAdapter.getDefaultUncaughtExceptionHandler()).thenReturn(mockExceptionHandler)
187180

188181
val thread = Thread.currentThread()
189-
// The outermost type is RuntimeException, not the ignored type, but the
190-
// cause chain contains the ignored type. Real RN apps wrap the JS exception
191-
// inside platform-level wrappers, so walking the chain matters.
192-
val inner = ReactNativeJavascriptExceptionStub("inner")
193-
val outer = RuntimeException("outer", inner)
182+
val throwable = ReactNativeJavascriptExceptionStub("Unhandled JS Exception: ReferenceError")
194183

195184
val integration =
196185
getSut(ignoredExceptionTypes = listOf(ReactNativeJavascriptExceptionStub::class.java.name))
197186
integration.install(mockPostHog)
198187

199-
integration.uncaughtException(thread, outer)
200-
201-
verify(mockPostHog, never()).captureException(any<PostHogThrowable>(), anyOrNull())
202-
verify(mockExceptionHandler).uncaughtException(thread, outer)
203-
204-
integration.uninstall()
205-
}
206-
207-
@Test
208-
fun `uncaughtException still captures when throwable class is not in ignoredExceptionTypes`() {
209-
whenever(mockConfig.remoteConfigHolder).thenReturn(mockRemoteConfig)
210-
whenever(mockRemoteConfig.isAutocaptureExceptionsEnabled()).thenReturn(true)
211-
whenever(mockAdapter.getDefaultUncaughtExceptionHandler()).thenReturn(mockExceptionHandler)
212-
213-
val thread = Thread.currentThread()
214-
val throwable = RuntimeException("Genuine native crash")
215-
216-
val integration =
217-
getSut(ignoredExceptionTypes = listOf("com.facebook.react.common.JavascriptException"))
218-
integration.install(mockPostHog)
219-
220188
integration.uncaughtException(thread, throwable)
221189

190+
// captureException always runs; PostHogIgnoredExceptionTypesBeforeSend
191+
// is what drops the resulting `$exception` event downstream. The unit
192+
// tests for that class cover the drop semantics.
222193
verify(mockPostHog).captureException(any<PostHogThrowable>(), anyOrNull())
223194
verify(mockExceptionHandler).uncaughtException(thread, throwable)
224195

‎posthog/src/test/java/com/posthog/errortracking/PostHogIgnoredExceptionTypesBeforeSendTest.kt‎

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
package com.posthog.errortracking
2+
3+
import com.posthog.PostHogConfig
4+
import com.posthog.PostHogEvent
5+
import com.posthog.PostHogEventName
6+
import com.posthog.internal.PostHogPrintLogger
7+
import org.mockito.kotlin.mock
8+
import org.mockito.kotlin.whenever
9+
import java.util.Date
10+
import kotlin.test.BeforeTest
11+
import kotlin.test.Test
12+
import kotlin.test.assertNotNull
13+
import kotlin.test.assertNull
14+
import kotlin.test.assertSame
15+
16+
internal class PostHogIgnoredExceptionTypesBeforeSendTest {
17+
private val mockConfig = mock<PostHogConfig>()
18+
private val mockLogger = mock<PostHogPrintLogger>()
19+
20+
@BeforeTest
21+
fun setUp() {
22+
whenever(mockConfig.logger).thenReturn(mockLogger)
23+
}
24+
25+
private fun configWithIgnored(types: List<String>): PostHogConfig {
26+
whenever(mockConfig.errorTrackingConfig).thenReturn(
27+
PostHogErrorTrackingConfig().apply { ignoredExceptionTypes.addAll(types) },
28+
)
29+
return mockConfig
30+
}
31+
32+
private fun exceptionEvent(types: List<String>): PostHogEvent {
33+
val list =
34+
types.map { type ->
35+
mapOf<String, Any>(
36+
"type" to type,
37+
"value" to "stub-message",
38+
"mechanism" to mapOf("handled" to false, "synthetic" to false),
39+
)
40+
}
41+
return PostHogEvent(
42+
event = PostHogEventName.EXCEPTION.event,
43+
distinctId = "distinct-id",
44+
properties =
45+
mutableMapOf<String, Any>(
46+
"\$exception_level" to "fatal",
47+
"\$exception_list" to list,
48+
),
49+
timestamp = Date(0),
50+
)
51+
}
52+
53+
@Test
54+
fun `drops $exception when outer type matches ignored list`() {
55+
val sut = PostHogIgnoredExceptionTypesBeforeSend(
56+
configWithIgnored(listOf("com.facebook.react.common.JavascriptException")),
57+
)
58+
val event = exceptionEvent(listOf("com.facebook.react.common.JavascriptException"))
59+
60+
assertNull(sut.run(event))
61+
}
62+
63+
@Test
64+
fun `drops $exception when cause chain contains ignored type`() {
65+
// `$exception_list` carries the cause chain in outer-to-inner order
66+
// (ThrowableCoercer writes the outermost throwable first, then each
67+
// cause). Real RN apps wrap the JS exception inside a platform-level
68+
// RuntimeException, so the filter must walk the whole list.
69+
val sut = PostHogIgnoredExceptionTypesBeforeSend(
70+
configWithIgnored(listOf("com.facebook.react.common.JavascriptException")),
71+
)
72+
val event = exceptionEvent(
73+
listOf(
74+
"java.lang.RuntimeException",
75+
"com.facebook.react.common.JavascriptException",
76+
),
77+
)
78+
79+
assertNull(sut.run(event))
80+
}
81+
82+
@Test
83+
fun `passes through $exception when no type matches ignored list`() {
84+
val sut = PostHogIgnoredExceptionTypesBeforeSend(
85+
configWithIgnored(listOf("com.facebook.react.common.JavascriptException")),
86+
)
87+
val event = exceptionEvent(listOf("com.example.GenuineNativeCrash"))
88+
89+
assertSame(event, sut.run(event))
90+
}
91+
92+
@Test
93+
fun `passes through $exception when ignoredExceptionTypes is empty`() {
94+
val sut = PostHogIgnoredExceptionTypesBeforeSend(configWithIgnored(emptyList()))
95+
val event = exceptionEvent(listOf("com.example.AnyException"))
96+
97+
assertSame(event, sut.run(event))
98+
}
99+
100+
@Test
101+
fun `passes through non-exception events untouched`() {
102+
// The filter should be event-type scoped: only `$exception` records
103+
// are subject to ignored-types filtering. A page view (or any other
104+
// event) flows through regardless of `ignoredExceptionTypes`.
105+
val sut = PostHogIgnoredExceptionTypesBeforeSend(
106+
configWithIgnored(listOf("com.facebook.react.common.JavascriptException")),
107+
)
108+
val event = PostHogEvent(
109+
event = "\$pageview",
110+
distinctId = "distinct-id",
111+
properties = mutableMapOf("\$current_url" to "https://example.com"),
112+
timestamp = Date(0),
113+
)
114+
115+
assertSame(event, sut.run(event))
116+
}
117+
118+
@Test
119+
fun `passes through when $exception_list is missing or empty`() {
120+
val sut = PostHogIgnoredExceptionTypesBeforeSend(
121+
configWithIgnored(listOf("com.example.AnyException")),
122+
)
123+
124+
val noList = PostHogEvent(
125+
event = PostHogEventName.EXCEPTION.event,
126+
distinctId = "distinct-id",
127+
properties = mutableMapOf<String, Any>("\$exception_level" to "fatal"),
128+
timestamp = Date(0),
129+
)
130+
assertNotNull(sut.run(noList))
131+
132+
val emptyList = exceptionEvent(emptyList())
133+
assertSame(emptyList, sut.run(emptyList))
134+
}
135+
}

0 commit comments

Comments
 (0)