Skip to content

fix: report page views to Customer.io in-app SDK #13560

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
benceruleanlu wants to merge 2 commits into
base: main
Choose a base branch
from
+78 −1
Open

fix: report page views to Customer.io in-app SDK #13560

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b7c7f16
fix: report page views to Customer.io
benceruleanlu Jul 10, 2026
190df7e
fix: preserve Customer.io tracking on in-app failure
benceruleanlu Jul 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions src/platform/telemetry/providers/cloud/CustomerIoTelemetryProvider.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
const hoisted = vi.hoisted(() => {
const analytics = {
identify: vi.fn(),
page: vi.fn(),
track: vi.fn(),
reset: vi.fn(),
register: vi.fn().mockResolvedValue(undefined)
Expand Down Expand Up @@ -73,6 +74,51 @@ describe('CustomerIoTelemetryProvider', () => {
expect(hoisted.analytics.register).toHaveBeenCalled()
})

it('reports the current page after registering the in-app plugin', async () => {
createProvider()
await vi.dynamicImportSettled()

expect(hoisted.analytics.page).toHaveBeenCalledOnce()
expect(hoisted.analytics.page).toHaveBeenCalledWith()
expect(hoisted.analytics.register.mock.invocationCallOrder[0]).toBeLessThan(
hoisted.analytics.page.mock.invocationCallOrder[0]
)
})

it('reports client-side route changes', async () => {
const provider = createProvider()
await vi.dynamicImportSettled()
hoisted.analytics.page.mockClear()

provider.trackPageView('workflow_editor', {
path: 'https://cloud.comfy.org/'
})

expect(hoisted.analytics.page).toHaveBeenCalledOnce()
expect(hoisted.analytics.page).toHaveBeenCalledWith()
})
Comment on lines +77 to +99

@coderabbitai coderabbitai Bot Jul 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider adding a test for the "analytics not ready → queued" edge case.

The path instructions mention covering the "analytics not ready → queued" scenario. The first test implicitly covers this (the initial page view is queued via pageViewQueued = true and flushed after registration), but there's no explicit test calling trackPageView before analytics is ready and verifying the queue-then-flush behavior. As per test-quality guidelines, key edge cases implied by the change should be covered.

🧪 Suggested additional test 
+  it('queues page views until analytics is ready', async () => {
+    const provider = createProvider()
+    // trackPageView called before dynamic import resolves — analytics is null
+    provider.trackPageView('workflow_editor', {
+      path: 'https://cloud.comfy.org/'
+    })
+    expect(hoisted.analytics.page).not.toHaveBeenCalled()
+
+    await vi.dynamicImportSettled()
+
+    expect(hoisted.analytics.page).toHaveBeenCalledOnce()
+    expect(hoisted.analytics.page).toHaveBeenCalledWith()
+  })

Based on learnings, only add call-count assertions when the call-count itself is part of what the test is meant to verify — here, the "not called before ready, then called once after" sequence is the core behavior under test. As per path instructions, .agents/checks/test-quality.md requires covering the "analytics not ready → queued" edge case.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
it('reports the current page after registering the in-app plugin', async () => {
createProvider()
await vi.dynamicImportSettled()
expect(hoisted.analytics.page).toHaveBeenCalledOnce()
expect(hoisted.analytics.page).toHaveBeenCalledWith()
expect(hoisted.analytics.register.mock.invocationCallOrder[0]).toBeLessThan(
hoisted.analytics.page.mock.invocationCallOrder[0]
)
})
it('reports client-side route changes', async () => {
const provider = createProvider()
await vi.dynamicImportSettled()
hoisted.analytics.page.mockClear()
provider.trackPageView('workflow_editor', {
path: 'https://cloud.comfy.org/'
})
expect(hoisted.analytics.page).toHaveBeenCalledOnce()
expect(hoisted.analytics.page).toHaveBeenCalledWith()
})
it('reports the current page after registering the in-app plugin', async () => {
createProvider()
await vi.dynamicImportSettled()
expect(hoisted.analytics.page).toHaveBeenCalledOnce()
expect(hoisted.analytics.page).toHaveBeenCalledWith()
expect(hoisted.analytics.register.mock.invocationCallOrder[0]).toBeLessThan(
hoisted.analytics.page.mock.invocationCallOrder[0]
)
})
it('queues page views until analytics is ready', async () => {
const provider = createProvider()
// trackPageView called before dynamic import resolves — analytics is null
provider.trackPageView('workflow_editor', {
path: 'https://cloud.comfy.org/'
})
expect(hoisted.analytics.page).not.toHaveBeenCalled()
await vi.dynamicImportSettled()
expect(hoisted.analytics.page).toHaveBeenCalledOnce()
expect(hoisted.analytics.page).toHaveBeenCalledWith()
})
it('reports client-side route changes', async () => {
const provider = createProvider()
await vi.dynamicImportSettled()
hoisted.analytics.page.mockClear()
provider.trackPageView('workflow_editor', {
path: 'https://cloud.comfy.org/'
})
expect(hoisted.analytics.page).toHaveBeenCalledOnce()
expect(hoisted.analytics.page).toHaveBeenCalledWith()
})
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/platform/telemetry/providers/cloud/CustomerIoTelemetryProvider.test.ts`
around lines 77 - 99, Add an explicit test covering trackPageView before
analytics initialization completes: create the provider, call
provider.trackPageView before awaiting dynamic imports, assert analytics.page
has not been called yet, then await vi.dynamicImportSettled and assert it is
called exactly once after registration. Use the existing createProvider and
analytics mocks, and retain assertions verifying registration occurs before the
queued page view is flushed.

Sources: Path instructions, Learnings


it('continues tracking when the in-app plugin fails to register', async () => {
vi.spyOn(console, 'error').mockImplementation(() => {})
hoisted.analytics.register.mockRejectedValue(
new Error('in-app setup failed')
)
const provider = createProvider()
provider.trackWorkflowExecution()

await vi.dynamicImportSettled()

expect(hoisted.analytics.track).toHaveBeenCalledWith(
'execution_start',
SOURCE
)
provider.trackAddApiCreditButtonClicked()
expect(hoisted.analytics.track).toHaveBeenCalledWith(
'app:add_api_credit_button_clicked',
SOURCE
)
})
Comment on lines +101 to +120

@coderabbitai coderabbitai Bot Jul 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider asserting console.error was called with the registration error.

The test mocks console.error to suppress output but doesn't verify the error was logged. Asserting the error message confirms the registration failure is properly observed and logged, strengthening the behavioral coverage of the failure path.

🧪 Suggested addition 
     expect(hoisted.analytics.track).toHaveBeenCalledWith(
       'app:add_api_credit_button_clicked',
       SOURCE
     )
+    expect(console.error).toHaveBeenCalledWith(
+      'Failed to initialize Customer.io in-app plugin:',
+      expect.any(Error)
+    )
   })

Based on learnings, only add call-count assertions when the call-count itself is part of what the test is meant to verify — here, verifying the error is logged is a meaningful behavioral assertion for the failure path. As per path instructions, .agents/checks/test-quality.md requires covering continued tracking when registration fails, including error handling behavior.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
it('continues tracking when the in-app plugin fails to register', async () => {
vi.spyOn(console, 'error').mockImplementation(() => {})
hoisted.analytics.register.mockRejectedValue(
new Error('in-app setup failed')
)
const provider = createProvider()
provider.trackWorkflowExecution()
await vi.dynamicImportSettled()
expect(hoisted.analytics.track).toHaveBeenCalledWith(
'execution_start',
SOURCE
)
provider.trackAddApiCreditButtonClicked()
expect(hoisted.analytics.track).toHaveBeenCalledWith(
'app:add_api_credit_button_clicked',
SOURCE
)
})
it('continues tracking when the in-app plugin fails to register', async () => {
vi.spyOn(console, 'error').mockImplementation(() => {})
hoisted.analytics.register.mockRejectedValue(
new Error('in-app setup failed')
)
const provider = createProvider()
provider.trackWorkflowExecution()
await vi.dynamicImportSettled()
expect(hoisted.analytics.track).toHaveBeenCalledWith(
'execution_start',
SOURCE
)
provider.trackAddApiCreditButtonClicked()
expect(hoisted.analytics.track).toHaveBeenCalledWith(
'app:add_api_credit_button_clicked',
SOURCE
)
expect(console.error).toHaveBeenCalledWith(
'Failed to initialize Customer.io in-app plugin:',
expect.any(Error)
)
})
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/platform/telemetry/providers/cloud/CustomerIoTelemetryProvider.test.ts`
around lines 101 - 120, Extend the test case “continues tracking when the in-app
plugin fails to register” to assert that the mocked console.error was called
with the registration failure, using the captured Error or its message. Keep the
existing tracking assertions to verify tracking continues after registration
fails.

Sources: Path instructions, Learnings


it('does not initialize without a write key', async () => {
const provider = createProvider({ customer_io: { site_id: SITE_ID } })
await vi.dynamicImportSettled()
Expand Down
33 changes: 32 additions & 1 deletion src/platform/telemetry/providers/cloud/CustomerIoTelemetryProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { TelemetryEvents } from '../../types'
import type {
AuthMetadata,
ExecutionSuccessMetadata,
PageViewMetadata,
ShareFlowMetadata,
SubscriptionMetadata,
TelemetryEventProperties,
Expand All @@ -32,6 +33,7 @@ export class CustomerIoTelemetryProvider implements TelemetryProvider {
private analytics: AnalyticsBrowser | null = null
private isEnabled = true
private eventQueue: QueuedEvent[] = []
private pageViewQueued = true

constructor() {
const {
Expand All @@ -47,7 +49,7 @@ export class CustomerIoTelemetryProvider implements TelemetryProvider {
void import('@customerio/cdp-analytics-browser')
.then(({ AnalyticsBrowser, InAppPlugin }) => {
const analytics = AnalyticsBrowser.load({ writeKey })
void analytics.register(
const inAppRegistration = analytics.register(
InAppPlugin({
siteId,
events: null,
Expand All @@ -68,6 +70,14 @@ export class CustomerIoTelemetryProvider implements TelemetryProvider {
currentUser.onUserLogout(() => void analytics.reset())

this.flushQueue()
void inAppRegistration
.then(() => this.flushPageView())
.catch((error) => {
console.error(
'Failed to initialize Customer.io in-app plugin:',
error
)
})
})
.catch((error) => {
console.error('Failed to load Customer.io:', error)
Expand Down Expand Up @@ -100,6 +110,27 @@ export class CustomerIoTelemetryProvider implements TelemetryProvider {
this.eventQueue = []
}

private sendPageView(): void {
void this.analytics?.page()?.catch((error) => {
console.error('Failed to track Customer.io page view:', error)
})
}

private flushPageView(): void {
if (!this.analytics || !this.pageViewQueued) return
this.pageViewQueued = false
this.sendPageView()
}

trackPageView(_pageName: string, _properties?: PageViewMetadata): void {
if (!this.isEnabled) return
if (!this.analytics) {
this.pageViewQueued = true
return
}
this.sendPageView()
}

trackAuth(metadata: AuthMetadata): void {
this.track(TelemetryEvents.USER_AUTH_COMPLETED, metadata)
}
Expand Down
Loading