Skip to content

Commit 3dcc921

Browse files
committed
fix: stop third-party analytics load failures from reporting as product errors
1 parent 15a8601 commit 3dcc921

11 files changed

Lines changed: 390 additions & 10 deletions

src/main.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ import {
1919
configValueOrDefault,
2020
remoteConfig
2121
} from '@/platform/remoteConfig/remoteConfig'
22+
import {
23+
markStoresPending,
24+
markStoresReady
25+
} from '@/platform/telemetry/storeReadiness'
2226
import { syncHostUserIdWithFirebaseAuth } from '@/platform/telemetry/hostUserIdSync'
2327
import '@/lib/litegraph/public/css/litegraph.css'
2428
import router from '@/router'
@@ -40,6 +44,8 @@ const { refreshRemoteConfig } =
4044
await import('@/platform/remoteConfig/refreshRemoteConfig')
4145
await refreshRemoteConfig({ useAuth: false })
4246

47+
markStoresPending()
48+
4349
if (isCloud) {
4450
const { initTelemetry } = await import('@/platform/telemetry/initTelemetry')
4551
await initTelemetry()
@@ -142,6 +148,8 @@ app
142148
modules: [VueFireAuth()]
143149
})
144150

151+
markStoresReady()
152+
145153
if (isCloud && hasHostTelemetryBridge) {
146154
syncHostUserIdWithFirebaseAuth()
147155
}

src/platform/telemetry/datadogRumBeforeSend.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,41 @@ describe('rumBeforeSend', () => {
2020
expect(rumBeforeSend(event, fromPartial({}))).toBe(false)
2121
})
2222

23+
it('drops resource load failures from origins we do not control', () => {
24+
const event = fromPartial<RumErrorEvent>({
25+
type: 'error',
26+
error: {
27+
message: '[resource:loadError]',
28+
source: 'source',
29+
resource: { url: 'https://connect.facebook.net/en_US/fbevents.js' }
30+
}
31+
})
32+
33+
expect(rumBeforeSend(event, fromPartial({}))).toBe(false)
34+
})
35+
36+
it('keeps resource load failures from our own origin', () => {
37+
const event = fromPartial<RumErrorEvent>({
38+
type: 'error',
39+
error: {
40+
message: '[resource:loadError]',
41+
source: 'source',
42+
resource: { url: 'https://cloud.comfy.org/assets/app.js' }
43+
}
44+
})
45+
46+
expect(rumBeforeSend(event, fromPartial({}))).toBe(true)
47+
})
48+
49+
it('keeps a runtime error thrown by a third-party script', () => {
50+
const event = createErrorEvent(
51+
'gtag is not a function',
52+
'at push (https://www.googletagmanager.com/gtm.js:1:2)'
53+
)
54+
55+
expect(rumBeforeSend(event, fromPartial({}))).toBe(true)
56+
})
57+
2358
it('keeps application errors and tags their origin', () => {
2459
const event = createErrorEvent(
2560
'Application failed',

src/platform/telemetry/datadogRumBeforeSend.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,32 @@ const RUM_NOISE_HOSTS = [
99
'googletagmanager.com'
1010
]
1111

12+
/**
13+
* Origins of analytics/marketing scripts we embed but do not host. A load
14+
* failure from one of these is a client-side blocker (ad blocker, tracking
15+
* protection, corporate proxy, strict DNS), not a product defect, so it must
16+
* not count the session as errored. Only load failures are dropped — a runtime
17+
* error thrown by one of these scripts still reports.
18+
*/
19+
const THIRD_PARTY_SCRIPT_ORIGINS = [
20+
'connect.facebook.net',
21+
'cdn.sy-d.io',
22+
'e2.sy-d.io',
23+
'googletagmanager.com',
24+
'google-analytics.com',
25+
'utt.impactcdn.com',
26+
'px.ads.linkedin.com'
27+
]
28+
29+
const RESOURCE_LOAD_FAILURE_MARKERS = [
30+
'resource:loadError',
31+
'loadError',
32+
'Failed to load',
33+
'Failed to fetch',
34+
'Load failed',
35+
'csp_violation'
36+
]
37+
1238
const FIRST_PARTY_EXTENSION_FOLDERS = new Set(['cloud', 'core'])
1339

1440
type RumErrorOrigin =
@@ -33,12 +59,25 @@ export function classifyRumErrorOrigin(stack?: string): RumErrorOrigin {
3359
return { origin: 'third_party' }
3460
}
3561

62+
function isThirdPartyLoadFailure(event: RumErrorEvent): boolean {
63+
const resourceUrl = event.error.resource?.url
64+
const target = resourceUrl ?? event.error.message
65+
if (
66+
!resourceUrl &&
67+
!RESOURCE_LOAD_FAILURE_MARKERS.some((marker) => target.includes(marker))
68+
) {
69+
return false
70+
}
71+
return THIRD_PARTY_SCRIPT_ORIGINS.some((origin) => target.includes(origin))
72+
}
73+
3674
function shouldKeepRumEvent(event: Parameters<RumBeforeSend>[0]): boolean {
3775
if (event.type !== 'error') return true
3876

3977
const message = event.error.message
4078
if (message.startsWith('intervention:')) return false
4179
if (message.includes('ResizeObserver loop')) return false
80+
if (isThirdPartyLoadFailure(event)) return false
4281

4382
const isNetworkNoise =
4483
message.includes('csp_violation') || message.includes('Failed to fetch')

src/platform/telemetry/providers/cloud/CustomerIoTelemetryProvider.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import { watch } from 'vue'
44

55
import { useCurrentUser } from '@/composables/auth/useCurrentUser'
66
import { i18n } from '@/i18n'
7+
import { whenStoresReady } from '@/platform/telemetry/storeReadiness'
8+
import { reportThirdPartyLoadFailure } from '@/platform/telemetry/thirdPartyLoadFailure'
79
import type { AuthUserInfo } from '@/types/authTypes'
810

911
import { TelemetryEvents } from '../../types'
@@ -67,7 +69,15 @@ export class CustomerIoTelemetryProvider implements TelemetryProvider {
6769
}
6870

6971
void import('@customerio/cdp-analytics-browser')
70-
.then(({ AnalyticsBrowser, InAppPlugin }) => {
72+
.catch((error) => {
73+
reportThirdPartyLoadFailure('Customer.io', error)
74+
this.isEnabled = false
75+
this.eventQueue = []
76+
return null
77+
})
78+
.then(async (sdk) => {
79+
if (!sdk) return
80+
const { AnalyticsBrowser, InAppPlugin } = sdk
7181
const analytics = AnalyticsBrowser.load({ writeKey })
7282
const inAppRegistration = analytics.register(
7383
InAppPlugin({
@@ -81,6 +91,7 @@ export class CustomerIoTelemetryProvider implements TelemetryProvider {
8191
)
8292
this.analytics = analytics
8393

94+
await whenStoresReady()
8495
const currentUser = useCurrentUser()
8596
const identifyResolvedUser = (user: AuthUserInfo) => {
8697
const identity = {
@@ -130,7 +141,7 @@ export class CustomerIoTelemetryProvider implements TelemetryProvider {
130141
})
131142
})
132143
.catch((error) => {
133-
console.error('Failed to load Customer.io:', error)
144+
console.error('Failed to initialize Customer.io:', error)
134145
this.isEnabled = false
135146
this.eventQueue = []
136147
})

src/platform/telemetry/providers/cloud/MixpanelTelemetryProvider.ts

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import { omit } from 'es-toolkit'
33
import { watch } from 'vue'
44

55
import { useCurrentUser } from '@/composables/auth/useCurrentUser'
6+
import { whenStoresReady } from '@/platform/telemetry/storeReadiness'
7+
import { reportThirdPartyLoadFailure } from '@/platform/telemetry/thirdPartyLoadFailure'
68
import {
79
checkForCompletedTopup as checkTopupUtil,
810
clearTopupTracking as clearTopupUtil,
@@ -106,7 +108,13 @@ export class MixpanelTelemetryProvider implements TelemetryProvider {
106108
try {
107109
// Dynamic import to avoid bundling mixpanel in OSS builds
108110
void import('mixpanel-browser')
111+
.catch((error) => {
112+
reportThirdPartyLoadFailure('Mixpanel', error)
113+
this.isEnabled = false
114+
return null
115+
})
109116
.then((mixpanelModule) => {
117+
if (!mixpanelModule) return
110118
this.mixpanel = mixpanelModule.default
111119
this.mixpanel.init(token, {
112120
debug: import.meta.env.DEV,
@@ -117,16 +125,22 @@ export class MixpanelTelemetryProvider implements TelemetryProvider {
117125
loaded: () => {
118126
this.isInitialized = true
119127
this.flushEventQueue() // flush events that were queued while initializing
120-
useCurrentUser().onUserResolved((user) => {
121-
if (this.mixpanel && user.id) {
122-
this.mixpanel.identify(user.id)
123-
}
124-
})
128+
void whenStoresReady()
129+
.then(() => {
130+
useCurrentUser().onUserResolved((user) => {
131+
if (this.mixpanel && user.id) {
132+
this.mixpanel.identify(user.id)
133+
}
134+
})
135+
})
136+
.catch((error) => {
137+
console.error('Failed to identify Mixpanel user:', error)
138+
})
125139
}
126140
})
127141
})
128142
.catch((error) => {
129-
console.error('Failed to load Mixpanel:', error)
143+
console.error('Failed to initialize Mixpanel:', error)
130144
this.isEnabled = false
131145
})
132146
} catch (error) {
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
2+
import type * as VueModule from 'vue'
3+
4+
const hoisted = vi.hoisted(() => ({
5+
init: vi.fn(),
6+
onUserResolved: vi.fn(),
7+
onUserLogout: vi.fn()
8+
}))
9+
10+
vi.mock('posthog-js', () => ({
11+
default: {
12+
init: hoisted.init,
13+
capture: vi.fn(),
14+
identify: vi.fn(),
15+
register: vi.fn(),
16+
people: { set: vi.fn(), set_once: vi.fn() },
17+
reset: vi.fn()
18+
}
19+
}))
20+
21+
vi.mock('@/composables/auth/useCurrentUser', () => ({
22+
useCurrentUser: () => ({
23+
onUserResolved: hoisted.onUserResolved,
24+
onUserLogout: hoisted.onUserLogout
25+
})
26+
}))
27+
28+
vi.mock('@/platform/remoteConfig/remoteConfig', async () => {
29+
const { ref } = await vi.importActual<typeof VueModule>('vue')
30+
return { remoteConfig: ref(null) }
31+
})
32+
33+
vi.mock('@/composables/billing/useBillingContext', async () => {
34+
const { ref } = await vi.importActual<typeof VueModule>('vue')
35+
return { useBillingContext: () => ({ tier: ref(null) }) }
36+
})
37+
38+
import { PostHogTelemetryProvider } from './PostHogTelemetryProvider'
39+
40+
function messagesFor(spy: { mock: { calls: unknown[][] } }): string[] {
41+
return spy.mock.calls
42+
.map((args) => String(args[0]))
43+
.filter((message) => message.includes('PostHog'))
44+
}
45+
46+
describe('PostHogTelemetryProvider initialisation failures', () => {
47+
beforeEach(() => {
48+
vi.clearAllMocks()
49+
window.__CONFIG__ = {
50+
posthog_project_token: 'phc_test_token'
51+
} as typeof window.__CONFIG__
52+
})
53+
54+
afterEach(() => {
55+
vi.restoreAllMocks()
56+
})
57+
58+
it('surfaces an error when our own initialisation throws', async () => {
59+
hoisted.init.mockImplementation(() => {
60+
throw new TypeError("Cannot read properties of undefined (reading '_s')")
61+
})
62+
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
63+
const error = vi.spyOn(console, 'error').mockImplementation(() => {})
64+
65+
new PostHogTelemetryProvider()
66+
await new Promise((resolve) => setTimeout(resolve, 0))
67+
68+
expect(messagesFor(error)).toEqual(['Failed to initialize PostHog:'])
69+
expect(messagesFor(warn)).toEqual([])
70+
})
71+
})

src/platform/telemetry/providers/cloud/PostHogTelemetryProvider.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import { createPostHogBeforeSend } from '@comfyorg/shared-frontend-utils/piiUtil
77
import { useCurrentUser } from '@/composables/auth/useCurrentUser'
88
import { useBillingContext } from '@/composables/billing/useBillingContext'
99
import { remoteConfig } from '@/platform/remoteConfig/remoteConfig'
10+
import { whenStoresReady } from '@/platform/telemetry/storeReadiness'
11+
import { reportThirdPartyLoadFailure } from '@/platform/telemetry/thirdPartyLoadFailure'
1012
import type { RemoteConfig } from '@/platform/remoteConfig/types'
1113
import { getExecutionContext } from '@/platform/telemetry/utils/getExecutionContext'
1214

@@ -141,7 +143,13 @@ export class PostHogTelemetryProvider implements TelemetryProvider {
141143
if (apiKey) {
142144
try {
143145
void import('posthog-js')
144-
.then((posthogModule) => {
146+
.catch((error) => {
147+
reportThirdPartyLoadFailure('PostHog', error)
148+
this.isEnabled = false
149+
return null
150+
})
151+
.then(async (posthogModule) => {
152+
if (!posthogModule) return
145153
this.posthog = posthogModule.default
146154
const serverConfig = remoteConfig.value?.posthog_config ?? {}
147155
this.posthog!.init(apiKey, {
@@ -168,6 +176,7 @@ export class PostHogTelemetryProvider implements TelemetryProvider {
168176
this.flushEventQueue()
169177
this.registerDesktopEntryProps()
170178

179+
await whenStoresReady()
171180
const currentUser = useCurrentUser()
172181
currentUser.onUserResolved((user) => {
173182
if (this.posthog && user.id) {
@@ -190,7 +199,7 @@ export class PostHogTelemetryProvider implements TelemetryProvider {
190199
})
191200
})
192201
.catch((error) => {
193-
console.error('Failed to load PostHog:', error)
202+
console.error('Failed to initialize PostHog:', error)
194203
this.isEnabled = false
195204
})
196205
} catch (error) {
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import { createPinia, setActivePinia } from 'pinia'
2+
import { afterEach, describe, expect, it, vi } from 'vitest'
3+
4+
import {
5+
markStoresPending,
6+
markStoresReady,
7+
whenStoresReady
8+
} from './storeReadiness'
9+
10+
afterEach(() => {
11+
markStoresReady()
12+
setActivePinia(undefined)
13+
})
14+
15+
describe('whenStoresReady', () => {
16+
it('resolves immediately when no pending window was opened', async () => {
17+
await expect(whenStoresReady()).resolves.toBeUndefined()
18+
})
19+
20+
it('blocks while stores are pending and resolves when they are ready', async () => {
21+
markStoresPending()
22+
const resolved = vi.fn()
23+
void whenStoresReady().then(resolved)
24+
25+
await Promise.resolve()
26+
expect(resolved).not.toHaveBeenCalled()
27+
28+
markStoresReady()
29+
await whenStoresReady()
30+
expect(resolved).toHaveBeenCalledOnce()
31+
})
32+
33+
it('resolves during a pending window once a Pinia instance is active', async () => {
34+
markStoresPending()
35+
setActivePinia(createPinia())
36+
37+
await expect(whenStoresReady()).resolves.toBeUndefined()
38+
})
39+
})

0 commit comments

Comments
 (0)