diff --git a/src/base/assert.test.ts b/src/base/assert.test.ts new file mode 100644 index 0000000000..1448198385 --- /dev/null +++ b/src/base/assert.test.ts @@ -0,0 +1,91 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { assert, setAssertReporter } from '@/base/assert' + +describe('assert', () => { + let consoleErrorSpy: ReturnType + + beforeEach(() => { + consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + }) + + afterEach(() => { + vi.restoreAllMocks() + vi.unstubAllEnvs() + setAssertReporter(null) + }) + + it('does nothing when condition is true', () => { + expect(() => assert(true, 'should not throw')).not.toThrow() + expect(consoleErrorSpy).not.toHaveBeenCalled() + }) + + it('logs console.error when condition is false', () => { + vi.stubEnv('DEV', false) + assert(false, 'test message') + expect(consoleErrorSpy).toHaveBeenCalledWith( + '[Assertion failed]: test message' + ) + }) + + it('throws in DEV mode when condition is false', () => { + vi.stubEnv('DEV', true) + const reporter = vi.fn() + setAssertReporter(reporter) + expect(() => assert(false, 'dev error')).toThrow( + '[Assertion failed]: dev error' + ) + expect(consoleErrorSpy).toHaveBeenCalledWith( + '[Assertion failed]: dev error' + ) + expect(reporter).not.toHaveBeenCalled() + }) + + it('does not throw in non-DEV mode when condition is false', () => { + vi.stubEnv('DEV', false) + expect(() => assert(false, 'non-dev error')).not.toThrow() + }) + + it('calls registered reporter in non-DEV mode with formatted message', () => { + vi.stubEnv('DEV', false) + const reporter = vi.fn() + setAssertReporter(reporter) + assert(false, 'reporter message') + expect(reporter).toHaveBeenCalledWith( + '[Assertion failed]: reporter message' + ) + }) + + it('does not call reporter when condition is true', () => { + vi.stubEnv('DEV', false) + const reporter = vi.fn() + setAssertReporter(reporter) + assert(true, 'no call') + expect(reporter).not.toHaveBeenCalled() + }) + + it('handles null reporter gracefully in non-DEV mode', () => { + vi.stubEnv('DEV', false) + setAssertReporter(null) + expect(() => assert(false, 'null reporter')).not.toThrow() + expect(consoleErrorSpy).toHaveBeenCalledWith( + '[Assertion failed]: null reporter' + ) + }) + + it('swallows reporter exceptions in non-DEV mode', () => { + vi.stubEnv('DEV', false) + const reporter = vi.fn(() => { + throw new Error('reporter blew up') + }) + setAssertReporter(reporter) + expect(() => assert(false, 'safe under reporter failure')).not.toThrow() + expect(consoleErrorSpy).toHaveBeenCalledWith( + '[Assertion failed]: safe under reporter failure' + ) + expect(consoleErrorSpy).toHaveBeenCalledWith( + '[Assertion reporter failed]', + expect.any(Error) + ) + }) +}) diff --git a/src/base/assert.ts b/src/base/assert.ts new file mode 100644 index 0000000000..829285ffde --- /dev/null +++ b/src/base/assert.ts @@ -0,0 +1,36 @@ +type AssertReporter = (message: string) => void + +let reporter: AssertReporter | null = null + +/** + * Register a reporter for assertion failures in non-DEV environments. + * Called once at app startup by platform/ or higher layers to wire in + * Sentry, toast notifications, etc. + */ +export function setAssertReporter(fn: AssertReporter | null): void { + reporter = fn +} + +/** + * Centralized invariant assertion. + * + * - Always: console.error + * - DEV: throws (surfaces bugs immediately) + * - Otherwise: delegates to registered reporter (Sentry, toast, etc.) + */ +export function assert(condition: unknown, message: string): asserts condition { + if (condition) return + + const formatted = `[Assertion failed]: ${message}` + console.error(formatted) + + if (import.meta.env.DEV) { + throw new Error(formatted) + } + + try { + reporter?.(formatted) + } catch (error) { + console.error('[Assertion reporter failed]', error) + } +} diff --git a/src/main.ts b/src/main.ts index a32242468c..7898779cf5 100644 --- a/src/main.ts +++ b/src/main.ts @@ -11,6 +11,7 @@ import Tooltip from 'primevue/tooltip' import { createApp } from 'vue' import { VueFire, VueFireAuth } from 'vuefire' +import { setAssertReporter } from '@/base/assert' import { getFirebaseConfig } from '@/config/firebase' import { configValueOrDefault, @@ -18,6 +19,8 @@ import { } from '@/platform/remoteConfig/remoteConfig' import '@/lib/litegraph/public/css/litegraph.css' import router from '@/router' +import { isDesktop, isNightly } from '@/platform/distribution/types' +import { useToastStore } from '@/platform/updates/common/toastStore' import { useBootstrapStore } from '@/stores/bootstrapStore' import App from './App.vue' @@ -81,6 +84,22 @@ Sentry.init({ defaultIntegrations: false }) }) +// Assertion reporter receives pre-formatted messages (with "[Assertion failed]: " prefix). +// Strings here are intentionally not i18n'd: they're developer/nightly diagnostics, +// not user-facing in stable releases. +setAssertReporter((message) => { + if (isDesktop) { + Sentry.captureMessage(message, { level: 'warning' }) + } + if (isNightly) { + useToastStore(pinia).add({ + severity: 'warn', + summary: 'Assertion failed', + detail: message + }) + } +}) + app.directive('tooltip', Tooltip) app .use(router) diff --git a/src/scripts/changeTracker.test.ts b/src/scripts/changeTracker.test.ts index 5513d83915..53724fb52d 100644 --- a/src/scripts/changeTracker.test.ts +++ b/src/scripts/changeTracker.test.ts @@ -2,6 +2,12 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import type { ComfyWorkflowJSON } from '@/platform/workflow/validation/schemas/workflowSchema' +const mockAssert = vi.hoisted(() => vi.fn()) + +vi.mock('@/base/assert', () => ({ + assert: mockAssert +})) + const mockNodeOutputStore = vi.hoisted(() => ({ snapshotOutputs: vi.fn(() => ({})), restoreOutputs: vi.fn() @@ -151,13 +157,17 @@ describe('ChangeTracker', () => { expect(app.rootGraph.serialize).not.toHaveBeenCalled() }) - it('is a no-op and logs error when called on inactive tracker', () => { + it('is a no-op and calls assert when called on inactive tracker', () => { const tracker = createTracker() mockWorkflowStore.activeWorkflow = { changeTracker: {} } tracker.captureCanvasState() expect(app.rootGraph.serialize).not.toHaveBeenCalled() + expect(mockAssert).toHaveBeenCalledWith( + false, + expect.stringContaining('captureCanvasState') + ) }) }) @@ -254,7 +264,7 @@ describe('ChangeTracker', () => { expect(mockNodeOutputStore.snapshotOutputs).toHaveBeenCalled() }) - it('is a full no-op when called on inactive tracker', () => { + it('is a full no-op and calls assert when called on inactive tracker', () => { const tracker = createTracker() mockWorkflowStore.activeWorkflow = { changeTracker: {} } @@ -262,6 +272,10 @@ describe('ChangeTracker', () => { expect(app.rootGraph.serialize).not.toHaveBeenCalled() expect(mockNodeOutputStore.snapshotOutputs).not.toHaveBeenCalled() + expect(mockAssert).toHaveBeenCalledWith( + false, + expect.stringContaining('deactivate') + ) }) }) diff --git a/src/scripts/changeTracker.ts b/src/scripts/changeTracker.ts index da7d861405..54312ea239 100644 --- a/src/scripts/changeTracker.ts +++ b/src/scripts/changeTracker.ts @@ -1,9 +1,8 @@ -import * as Sentry from '@sentry/vue' import _ from 'es-toolkit/compat' +import { assert } from '@/base/assert' import type { CanvasPointerEvent } from '@/lib/litegraph/src/litegraph' import { LGraphCanvas, LiteGraph } from '@/lib/litegraph/src/litegraph' -import { isDesktop } from '@/platform/distribution/types' import type { ComfyWorkflow } from '@/platform/workflow/management/stores/workflowStore' import { useWorkflowStore } from '@/platform/workflow/management/stores/workflowStore' import type { ComfyWorkflowJSON } from '@/platform/workflow/validation/schemas/workflowSchema' @@ -24,23 +23,20 @@ function isActiveTracker(tracker: ChangeTracker): boolean { return useWorkflowStore().activeWorkflow?.changeTracker === tracker } +const reportedInactiveCalls = new Set() + /** - * Report a ChangeTracker method being called on an inactive tracker — - * a lifecycle violation that usually indicates stale extension state or - * an incorrect call ordering. + * Report a ChangeTracker method being called on an inactive tracker. + * Deduplicates per method+workflow per session to avoid signal noise on hot paths. */ function reportInactiveTrackerCall(method: string, workflowPath: string) { - console.warn(`${method}() called on inactive tracker for: ${workflowPath}`) - - if (isDesktop) { - Sentry.captureMessage( - `ChangeTracker.${method}() called on inactive tracker`, - { - level: 'warning', - tags: { workflow: workflowPath } - } - ) - } + const key = `${method}:${workflowPath}` + if (reportedInactiveCalls.has(key)) return + reportedInactiveCalls.add(key) + assert( + false, + `ChangeTracker.${method}() called on inactive tracker for: ${workflowPath}` + ) } export class ChangeTracker {