Skip to content

Commit 8419c6f

Browse files
committed
fix(analytics): match cancellations on error name, not coerced type
Two holes in the exception filter, both found reading PostHog's coercers rather than trusting the shape. The DOMException coercer always reports type "DOMException" and folds the name into the value as "AbortError: signal is aborted without reason", so matching AbortError on type alone never fired for a real aborted fetch — the filter was dead code for exactly the events it was written for. Match the error name wherever the coercer put it. The filter also applied to deliberate captureException reports, which our error boundaries make. Only exceptions the browser raised itself (mechanism.handled === false) are now eligible; a report we chose to send is never dropped.
1 parent 4a6cba5 commit 8419c6f

2 files changed

Lines changed: 117 additions & 17 deletions

File tree

apps/sim/lib/posthog/exception-filter.test.ts

Lines changed: 67 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,34 @@ import type { CaptureResult } from 'posthog-js'
55
import { describe, expect, it } from 'vitest'
66
import { dropUnactionableExceptions } from '@/lib/posthog/exception-filter'
77

8-
function exceptionEvent(...exceptions: Array<{ type?: string; value?: string }>): CaptureResult {
8+
interface TestException {
9+
type?: string
10+
value?: string
11+
mechanism?: { handled?: boolean }
12+
}
13+
14+
/** Mirrors what PostHog's `window.onerror` / `unhandledrejection` wrappers build. */
15+
function browserRaised(...exceptions: TestException[]): CaptureResult {
16+
const [head, ...rest] = exceptions
17+
return {
18+
uuid: 'test-uuid',
19+
event: '$exception',
20+
properties: {
21+
$exception_list: [
22+
{ ...head, mechanism: { handled: false } },
23+
// PostHog forces chained cause links to handled: true regardless of the source.
24+
...rest.map((exception) => ({ ...exception, mechanism: { handled: true } })),
25+
],
26+
},
27+
} as CaptureResult
28+
}
29+
30+
/** Mirrors what `posthog.captureException` builds — a deliberate report. */
31+
function deliberatelyReported(exception: TestException): CaptureResult {
932
return {
1033
uuid: 'test-uuid',
1134
event: '$exception',
12-
properties: { $exception_list: exceptions },
35+
properties: { $exception_list: [{ ...exception, mechanism: { handled: true } }] },
1336
} as CaptureResult
1437
}
1538

@@ -34,24 +57,60 @@ describe('dropUnactionableExceptions', () => {
3457
'ResizeObserver loop limit exceeded',
3558
'Script error.',
3659
])('drops the undiagnosable browser artifact %j', (value) => {
37-
expect(dropUnactionableExceptions(exceptionEvent({ type: 'Error', value }))).toBeNull()
60+
expect(dropUnactionableExceptions(browserRaised({ type: 'Error', value }))).toBeNull()
61+
})
62+
63+
it('drops a cancellation whose name is the coerced type', () => {
64+
expect(
65+
dropUnactionableExceptions(browserRaised({ type: 'Canceled', value: 'Canceled' }))
66+
).toBeNull()
3867
})
3968

40-
it.each(['AbortError', 'Canceled'])('drops the cancellation signal %j', (type) => {
41-
expect(dropUnactionableExceptions(exceptionEvent({ type, value: 'whatever' }))).toBeNull()
69+
/**
70+
* The shape a real aborted `fetch` produces: PostHog's DOMException coercer
71+
* reports type `DOMException` and folds the name into the value, so a filter
72+
* that only tested `type` would let every one of these through.
73+
*/
74+
it('drops a cancellation whose name is folded into a DOMException value', () => {
75+
expect(
76+
dropUnactionableExceptions(
77+
browserRaised({
78+
type: 'DOMException',
79+
value: 'AbortError: signal is aborted without reason',
80+
})
81+
)
82+
).toBeNull()
83+
})
84+
85+
it('keeps a DOMException that is not a cancellation', () => {
86+
const event = browserRaised({
87+
type: 'DOMException',
88+
value: "NotFoundError: Failed to execute 'removeChild' on 'Node'",
89+
})
90+
91+
expect(dropUnactionableExceptions(event)).toBe(event)
4292
})
4393

4494
it('keeps a real exception', () => {
45-
const event = exceptionEvent({
95+
const event = browserRaised({
4696
type: 'TypeError',
4797
value: "Cannot read properties of undefined (reading 'id')",
4898
})
4999

50100
expect(dropUnactionableExceptions(event)).toBe(event)
51101
})
52102

103+
it('keeps a deliberately reported exception even when it looks like noise', () => {
104+
const event = deliberatelyReported({
105+
type: 'AbortError',
106+
value: 'signal is aborted without reason',
107+
})
108+
109+
expect(dropUnactionableExceptions(event)).toBe(event)
110+
})
111+
53112
it('keeps a chained exception when only one link is noise', () => {
54-
const event = exceptionEvent(
113+
const event = browserRaised(
55114
{ type: 'AbortError', value: 'signal is aborted without reason' },
56115
{ type: 'RangeError', value: 'Maximum call stack size exceeded.' }
57116
)
@@ -60,7 +119,7 @@ describe('dropUnactionableExceptions', () => {
60119
})
61120

62121
it('keeps an exception whose message merely mentions a filtered one', () => {
63-
const event = exceptionEvent({
122+
const event = browserRaised({
64123
type: 'TypeError',
65124
value: 'Failed to patch ResizeObserver loop completed with undelivered notifications',
66125
})

apps/sim/lib/posthog/exception-filter.ts

Lines changed: 50 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import type { CaptureResult } from 'posthog-js'
1313
* Neither can be acted on: there is no defect to fix and no user impact, but
1414
* both fire often enough per session to bury real crashes in the issue list.
1515
*/
16-
const CANCELLATION_EXCEPTION_TYPES = new Set(['AbortError', 'Canceled'])
16+
const CANCELLATION_ERROR_NAMES = new Set(['AbortError', 'Canceled'])
1717

1818
/**
1919
* Exception messages that carry no diagnosable content.
@@ -41,13 +41,50 @@ const UNDIAGNOSABLE_EXCEPTION_MESSAGES = [
4141
interface CapturedException {
4242
type?: unknown
4343
value?: unknown
44+
mechanism?: { handled?: unknown }
4445
}
4546

46-
function isNoise(exception: CapturedException): boolean {
47-
if (typeof exception.type === 'string' && CANCELLATION_EXCEPTION_TYPES.has(exception.type)) {
47+
/**
48+
* Whether the browser raised this itself, rather than us reporting it on purpose.
49+
*
50+
* PostHog's `window.onerror` and `unhandledrejection` wrappers both build their
51+
* exception with `mechanism.handled: false`, while `captureException` — the call
52+
* our error boundaries make — builds with `true`. Only the browser's own reports
53+
* are eligible for filtering: a deliberate report means someone decided the
54+
* failure was worth knowing about, and dropping it would repeat the silent loss
55+
* this filter's sibling gate exists to prevent.
56+
*
57+
* Read from the first entry only. When an error carries a `cause`, PostHog
58+
* appends the chained links with `handled: true` regardless of how the original
59+
* was raised, so the head of the list is the one that reflects the source.
60+
*/
61+
function isBrowserRaised(exceptions: CapturedException[]): boolean {
62+
return exceptions[0]?.mechanism?.handled === false
63+
}
64+
65+
/**
66+
* Whether this is a cancellation, testing both shapes PostHog can produce.
67+
*
68+
* Which field carries the error's `name` depends on which coercer ran. A plain
69+
* `Error` keeps its `name` as `type`, so Monaco's `CancellationError` arrives as
70+
* type `Canceled`. A `DOMException` — what `fetch` rejects with when its signal
71+
* fires — always coerces to type `DOMException`, with the name folded into the
72+
* front of the value as `"AbortError: signal is aborted without reason"`.
73+
* Matching on `type` alone therefore misses every real aborted request.
74+
*/
75+
function isCancellation(exception: CapturedException): boolean {
76+
if (typeof exception.type === 'string' && CANCELLATION_ERROR_NAMES.has(exception.type)) {
4877
return true
4978
}
5079

80+
if (typeof exception.value !== 'string') return false
81+
82+
return CANCELLATION_ERROR_NAMES.has(exception.value.split(':', 1)[0])
83+
}
84+
85+
function isNoise(exception: CapturedException): boolean {
86+
if (isCancellation(exception)) return true
87+
5188
if (typeof exception.value !== 'string') return false
5289
const message = exception.value.trim()
5390

@@ -57,8 +94,9 @@ function isNoise(exception: CapturedException): boolean {
5794
/**
5895
* `before_send` hook that drops browser noise from error tracking.
5996
*
60-
* Fails open in every direction: anything that is not a `$exception`, and any
61-
* `$exception` whose list is missing or unrecognizable, passes through
97+
* Fails open in every direction: anything that is not a `$exception`, any
98+
* `$exception` whose list is missing or unrecognizable, and anything we
99+
* reported deliberately rather than caught from the browser, all pass through
62100
* untouched. This runs on **every** captured event, so a filter that guessed
63101
* wrong would silently delete product analytics rather than merely over-report.
64102
*
@@ -75,10 +113,13 @@ export function dropUnactionableExceptions(event: CaptureResult | null): Capture
75113
const exceptions: unknown = event.properties?.$exception_list
76114
if (!Array.isArray(exceptions) || exceptions.length === 0) return event
77115

78-
const allNoise = exceptions.every(
79-
(exception) =>
80-
typeof exception === 'object' && exception !== null && isNoise(exception as CapturedException)
116+
const entries: CapturedException[] = exceptions.filter(
117+
(exception): exception is CapturedException =>
118+
typeof exception === 'object' && exception !== null
81119
)
120+
if (entries.length !== exceptions.length) return event
121+
122+
if (!isBrowserRaised(entries)) return event
82123

83-
return allNoise ? null : event
124+
return entries.every(isNoise) ? null : event
84125
}

0 commit comments

Comments
 (0)