mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-20 14:30:41 +00:00
refactor: move WebSocket reconnect tracking from PostHog/Mixpanel to Sentry
Reconnect events are operational/debug signals, not product analytics. Sentry is the right home for this — breadcrumbs for all reconnects, captureMessage (warning) when jobs were in flight. Also addresses review feedback: - Extract to useWebSocketReconnectTracking composable - Use performance.now() instead of Date.now() - Add tests for the composable (6 cases) - Remove PostHog/Mixpanel telemetry plumbing Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
101
src/composables/useWebSocketReconnectTracking.test.ts
Normal file
101
src/composables/useWebSocketReconnectTracking.test.ts
Normal file
@@ -0,0 +1,101 @@
|
||||
import * as Sentry from '@sentry/vue'
|
||||
import { createPinia, setActivePinia } from 'pinia'
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import { useWebSocketReconnectTracking } from './useWebSocketReconnectTracking'
|
||||
|
||||
vi.mock('@sentry/vue', () => ({
|
||||
addBreadcrumb: vi.fn(),
|
||||
captureMessage: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('@/stores/queueStore', () => ({
|
||||
useQueueStore: () => ({
|
||||
activeJobsCount: mockActiveJobsCount
|
||||
})
|
||||
}))
|
||||
|
||||
let mockActiveJobsCount = 0
|
||||
|
||||
describe('useWebSocketReconnectTracking', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
mockActiveJobsCount = 0
|
||||
setActivePinia(createPinia())
|
||||
})
|
||||
|
||||
it('does nothing on reconnect without prior disconnect', () => {
|
||||
const { onReconnect } = useWebSocketReconnectTracking()
|
||||
onReconnect()
|
||||
expect(Sentry.addBreadcrumb).not.toHaveBeenCalled()
|
||||
expect(Sentry.captureMessage).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('adds breadcrumb on reconnect after disconnect', () => {
|
||||
const { onDisconnect, onReconnect } = useWebSocketReconnectTracking()
|
||||
onDisconnect()
|
||||
onReconnect()
|
||||
|
||||
expect(Sentry.addBreadcrumb).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
category: 'websocket',
|
||||
message: 'WebSocket reconnected',
|
||||
level: 'info'
|
||||
})
|
||||
)
|
||||
})
|
||||
|
||||
it('captures warning when jobs were active at disconnect', () => {
|
||||
mockActiveJobsCount = 3
|
||||
const { onDisconnect, onReconnect } = useWebSocketReconnectTracking()
|
||||
onDisconnect()
|
||||
onReconnect()
|
||||
|
||||
expect(Sentry.captureMessage).toHaveBeenCalledWith(
|
||||
'WebSocket reconnected with active jobs',
|
||||
expect.objectContaining({
|
||||
level: 'warning',
|
||||
tags: { incident: 'incident-39' },
|
||||
extra: expect.objectContaining({ active_job_count: 3 })
|
||||
})
|
||||
)
|
||||
})
|
||||
|
||||
it('does not capture warning when no jobs were active', () => {
|
||||
mockActiveJobsCount = 0
|
||||
const { onDisconnect, onReconnect } = useWebSocketReconnectTracking()
|
||||
onDisconnect()
|
||||
onReconnect()
|
||||
|
||||
expect(Sentry.addBreadcrumb).toHaveBeenCalled()
|
||||
expect(Sentry.captureMessage).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('ignores duplicate disconnect calls', () => {
|
||||
mockActiveJobsCount = 2
|
||||
const { onDisconnect, onReconnect } = useWebSocketReconnectTracking()
|
||||
onDisconnect()
|
||||
|
||||
// Job count changes between disconnect calls
|
||||
mockActiveJobsCount = 5
|
||||
onDisconnect()
|
||||
|
||||
onReconnect()
|
||||
|
||||
// Should use the count from the first disconnect (2), not the second (5)
|
||||
expect(Sentry.addBreadcrumb).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
data: expect.objectContaining({ active_job_count: 2 })
|
||||
})
|
||||
)
|
||||
})
|
||||
|
||||
it('resets state and prevents stale reconnect', () => {
|
||||
const { onDisconnect, onReconnect, reset } = useWebSocketReconnectTracking()
|
||||
onDisconnect()
|
||||
reset()
|
||||
onReconnect()
|
||||
|
||||
expect(Sentry.addBreadcrumb).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
66
src/composables/useWebSocketReconnectTracking.ts
Normal file
66
src/composables/useWebSocketReconnectTracking.ts
Normal file
@@ -0,0 +1,66 @@
|
||||
import * as Sentry from '@sentry/vue'
|
||||
|
||||
import { useQueueStore } from '@/stores/queueStore'
|
||||
|
||||
/**
|
||||
* Tracks WebSocket disconnect/reconnect events via Sentry for incident-39
|
||||
* observability. Captures disconnect duration and whether jobs were in flight
|
||||
* when the connection dropped.
|
||||
*/
|
||||
export function useWebSocketReconnectTracking() {
|
||||
const queueStore = useQueueStore()
|
||||
|
||||
let disconnectedAt: number | null = null
|
||||
let activeJobCountAtDisconnect = 0
|
||||
|
||||
function onDisconnect() {
|
||||
if (disconnectedAt !== null) return
|
||||
|
||||
disconnectedAt = performance.now()
|
||||
// Includes both pending and running tasks. Pending tasks matter because
|
||||
// their pending->running transition is delivered via WebSocket -- if the
|
||||
// connection drops while tasks are queued, the user never sees them start.
|
||||
activeJobCountAtDisconnect = queueStore.activeJobsCount
|
||||
}
|
||||
|
||||
function onReconnect() {
|
||||
if (disconnectedAt === null) return
|
||||
|
||||
const durationMs = Math.round(performance.now() - disconnectedAt)
|
||||
const hadActiveJobs = activeJobCountAtDisconnect > 0
|
||||
|
||||
Sentry.addBreadcrumb({
|
||||
category: 'websocket',
|
||||
message: 'WebSocket reconnected',
|
||||
level: 'info',
|
||||
data: {
|
||||
disconnect_duration_ms: durationMs,
|
||||
had_active_jobs: hadActiveJobs,
|
||||
active_job_count: activeJobCountAtDisconnect
|
||||
}
|
||||
})
|
||||
|
||||
if (hadActiveJobs) {
|
||||
Sentry.captureMessage('WebSocket reconnected with active jobs', {
|
||||
level: 'warning',
|
||||
tags: {
|
||||
incident: 'incident-39'
|
||||
},
|
||||
extra: {
|
||||
disconnect_duration_ms: durationMs,
|
||||
active_job_count: activeJobCountAtDisconnect
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
disconnectedAt = null
|
||||
activeJobCountAtDisconnect = 0
|
||||
}
|
||||
|
||||
function reset() {
|
||||
disconnectedAt = null
|
||||
activeJobCountAtDisconnect = 0
|
||||
}
|
||||
|
||||
return { onDisconnect, onReconnect, reset }
|
||||
}
|
||||
@@ -27,7 +27,6 @@ import type {
|
||||
TemplateLibraryMetadata,
|
||||
TemplateMetadata,
|
||||
UiButtonClickMetadata,
|
||||
WebSocketReconnectedMetadata,
|
||||
WorkflowCreatedMetadata,
|
||||
WorkflowImportMetadata,
|
||||
WorkflowSavedMetadata
|
||||
@@ -237,8 +236,4 @@ export class TelemetryRegistry implements TelemetryDispatcher {
|
||||
trackPageView(pageName: string, properties?: PageViewMetadata): void {
|
||||
this.dispatch((provider) => provider.trackPageView?.(pageName, properties))
|
||||
}
|
||||
|
||||
trackWebSocketReconnected(metadata: WebSocketReconnectedMetadata): void {
|
||||
this.dispatch((provider) => provider.trackWebSocketReconnected?.(metadata))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -41,7 +41,6 @@ import type {
|
||||
TemplateLibraryMetadata,
|
||||
TemplateMetadata,
|
||||
UiButtonClickMetadata,
|
||||
WebSocketReconnectedMetadata,
|
||||
WorkflowCreatedMetadata,
|
||||
WorkflowImportMetadata,
|
||||
WorkflowSavedMetadata
|
||||
@@ -443,8 +442,4 @@ export class MixpanelTelemetryProvider implements TelemetryProvider {
|
||||
trackUiButtonClicked(metadata: UiButtonClickMetadata): void {
|
||||
this.trackEvent(TelemetryEvents.UI_BUTTON_CLICKED, metadata)
|
||||
}
|
||||
|
||||
trackWebSocketReconnected(metadata: WebSocketReconnectedMetadata): void {
|
||||
this.trackEvent(TelemetryEvents.WEBSOCKET_RECONNECTED, metadata)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -236,48 +236,6 @@ describe('PostHogTelemetryProvider', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('websocket reconnect', () => {
|
||||
it('captures reconnect event with metadata', async () => {
|
||||
const provider = createProvider()
|
||||
await vi.dynamicImportSettled()
|
||||
|
||||
provider.trackWebSocketReconnected({
|
||||
disconnect_duration_ms: 5000,
|
||||
had_active_jobs: true,
|
||||
active_job_count: 3
|
||||
})
|
||||
|
||||
expect(hoisted.mockCapture).toHaveBeenCalledWith(
|
||||
TelemetryEvents.WEBSOCKET_RECONNECTED,
|
||||
{
|
||||
disconnect_duration_ms: 5000,
|
||||
had_active_jobs: true,
|
||||
active_job_count: 3
|
||||
}
|
||||
)
|
||||
})
|
||||
|
||||
it('captures reconnect event when no jobs were active', async () => {
|
||||
const provider = createProvider()
|
||||
await vi.dynamicImportSettled()
|
||||
|
||||
provider.trackWebSocketReconnected({
|
||||
disconnect_duration_ms: 1200,
|
||||
had_active_jobs: false,
|
||||
active_job_count: 0
|
||||
})
|
||||
|
||||
expect(hoisted.mockCapture).toHaveBeenCalledWith(
|
||||
TelemetryEvents.WEBSOCKET_RECONNECTED,
|
||||
{
|
||||
disconnect_duration_ms: 1200,
|
||||
had_active_jobs: false,
|
||||
active_job_count: 0
|
||||
}
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
describe('page view', () => {
|
||||
it('captures page view with page_name property', async () => {
|
||||
const provider = createProvider()
|
||||
|
||||
@@ -36,7 +36,6 @@ import type {
|
||||
TemplateLibraryMetadata,
|
||||
TemplateMetadata,
|
||||
UiButtonClickMetadata,
|
||||
WebSocketReconnectedMetadata,
|
||||
WorkflowCreatedMetadata,
|
||||
WorkflowImportMetadata,
|
||||
WorkflowSavedMetadata
|
||||
@@ -453,8 +452,4 @@ export class PostHogTelemetryProvider implements TelemetryProvider {
|
||||
...properties
|
||||
})
|
||||
}
|
||||
|
||||
trackWebSocketReconnected(metadata: WebSocketReconnectedMetadata): void {
|
||||
this.trackEvent(TelemetryEvents.WEBSOCKET_RECONNECTED, metadata)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -304,15 +304,6 @@ export interface WorkflowCreatedMetadata {
|
||||
previous_workflow_had_nodes: boolean
|
||||
}
|
||||
|
||||
/**
|
||||
* WebSocket reconnection metadata
|
||||
*/
|
||||
export interface WebSocketReconnectedMetadata {
|
||||
disconnect_duration_ms: number
|
||||
had_active_jobs: boolean
|
||||
active_job_count: number
|
||||
}
|
||||
|
||||
/**
|
||||
* Page view metadata for route tracking
|
||||
*/
|
||||
@@ -436,9 +427,6 @@ export interface TelemetryProvider {
|
||||
|
||||
// Page view tracking
|
||||
trackPageView?(pageName: string, properties?: PageViewMetadata): void
|
||||
|
||||
// WebSocket lifecycle events
|
||||
trackWebSocketReconnected?(metadata: WebSocketReconnectedMetadata): void
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -526,10 +514,7 @@ export const TelemetryEvents = {
|
||||
UI_BUTTON_CLICKED: 'app:ui_button_clicked',
|
||||
|
||||
// Page View
|
||||
PAGE_VIEW: 'app:page_view',
|
||||
|
||||
// WebSocket Lifecycle
|
||||
WEBSOCKET_RECONNECTED: 'app:websocket_reconnected'
|
||||
PAGE_VIEW: 'app:page_view'
|
||||
} as const
|
||||
|
||||
export type TelemetryEventName =
|
||||
@@ -573,4 +558,3 @@ export type TelemetryEventProperties =
|
||||
| WorkflowSavedMetadata
|
||||
| DefaultViewSetMetadata
|
||||
| SubscriptionMetadata
|
||||
| WebSocketReconnectedMetadata
|
||||
|
||||
@@ -66,6 +66,7 @@ import ModelImportProgressDialog from '@/platform/assets/components/ModelImportP
|
||||
import { isCloud, isDesktop } from '@/platform/distribution/types'
|
||||
import { useSettingStore } from '@/platform/settings/settingStore'
|
||||
import { useTelemetry } from '@/platform/telemetry'
|
||||
import { useWebSocketReconnectTracking } from '@/composables/useWebSocketReconnectTracking'
|
||||
import { useFrontendVersionMismatchWarning } from '@/platform/updates/common/useFrontendVersionMismatchWarning'
|
||||
import { useVersionCompatibilityStore } from '@/platform/updates/common/versionCompatibilityStore'
|
||||
import { useCanvasStore } from '@/renderer/core/canvas/canvasStore'
|
||||
@@ -253,17 +254,10 @@ const reconnectingMessage: ToastMessageOptions = {
|
||||
summary: t('g.reconnecting')
|
||||
}
|
||||
|
||||
let disconnectedAt: number | null = null
|
||||
let activeJobCountAtDisconnect = 0
|
||||
const wsReconnectTracking = useWebSocketReconnectTracking()
|
||||
|
||||
const onReconnecting = () => {
|
||||
if (disconnectedAt === null) {
|
||||
disconnectedAt = Date.now()
|
||||
// Includes pending + running tasks. Pending tasks matter because their
|
||||
// pending→running transition is delivered via WebSocket — if the connection
|
||||
// drops while tasks are queued, the user never sees them start.
|
||||
activeJobCountAtDisconnect = queueStore.activeJobsCount
|
||||
}
|
||||
wsReconnectTracking.onDisconnect()
|
||||
if (!settingStore.get('Comfy.Toast.DisableReconnectingToast')) {
|
||||
toast.remove(reconnectingMessage)
|
||||
toast.add(reconnectingMessage)
|
||||
@@ -271,15 +265,7 @@ const onReconnecting = () => {
|
||||
}
|
||||
|
||||
const onReconnected = () => {
|
||||
if (disconnectedAt !== null) {
|
||||
telemetry?.trackWebSocketReconnected({
|
||||
disconnect_duration_ms: Date.now() - disconnectedAt,
|
||||
had_active_jobs: activeJobCountAtDisconnect > 0,
|
||||
active_job_count: activeJobCountAtDisconnect
|
||||
})
|
||||
disconnectedAt = null
|
||||
activeJobCountAtDisconnect = 0
|
||||
}
|
||||
wsReconnectTracking.onReconnect()
|
||||
if (!settingStore.get('Comfy.Toast.DisableReconnectingToast')) {
|
||||
toast.remove(reconnectingMessage)
|
||||
toast.add({
|
||||
@@ -307,8 +293,7 @@ onMounted(() => {
|
||||
})
|
||||
|
||||
onBeforeUnmount(() => {
|
||||
disconnectedAt = null
|
||||
activeJobCountAtDisconnect = 0
|
||||
wsReconnectTracking.reset()
|
||||
executionStore.unbindExecutionEvents()
|
||||
})
|
||||
|
||||
|
||||
Reference in New Issue
Block a user