diff --git a/package.json b/package.json index 315d90d1b..a636bdd51 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,7 @@ "prepare-resources": "node scripts/download-python-embed.js", "test": "npm run test:py && npm run test:node", "test:py": "node scripts/run-pytests.mjs", - "test:node": "node --test --experimental-strip-types --experimental-loader ./scripts/node-ts-extensionless-loader.mjs src/shared/types/assetLibrary.test.ts src/areas/generate/assetLibraryProjection.test.ts src/areas/generate/assetLibraryService.test.ts src/areas/generate/assetLibraryUi.test.ts src/areas/generate/orcaSlicerLink.test.ts src/shared/utils/agentStream.test.ts src/shared/utils/ollamaHealth.test.ts electron/main/artifact-registry-service.test.ts electron/main/extension-path-guard.test.ts electron/main/storage-directory-guard.test.ts electron/main/storage-ownership.test.ts electron/main/startup-error.test.ts electron/main/api-port.test.ts electron/preload/artifact-registry-preload.test.ts && node --test electron/main/*.test.mjs src/**/*.test.mjs", + "test:node": "node --test --experimental-strip-types --experimental-loader ./scripts/node-ts-extensionless-loader.mjs src/shared/types/assetLibrary.test.ts src/areas/generate/assetLibraryProjection.test.ts src/areas/generate/assetLibraryService.test.ts src/areas/generate/assetLibraryUi.test.ts src/areas/generate/orcaSlicerLink.test.ts src/shared/utils/agentStream.test.ts src/shared/utils/ollamaHealth.test.ts src/shared/utils/notification.test.ts electron/main/artifact-registry-service.test.ts electron/main/extension-path-guard.test.ts electron/main/storage-directory-guard.test.ts electron/main/storage-ownership.test.ts electron/main/startup-error.test.ts electron/main/api-port.test.ts electron/preload/artifact-registry-preload.test.ts && node --test electron/main/*.test.mjs src/**/*.test.mjs", "package": "cross-env CSC_IDENTITY_AUTO_DISCOVERY=false npm run build && npm run prepare-resources && electron-builder", "package:mac": "cross-env CSC_IDENTITY_AUTO_DISCOVERY=false npm run build && npm run prepare-resources && electron-builder --mac --arm64", "lint": "eslint ." diff --git a/src/areas/workflows/workflowRunStore.ts b/src/areas/workflows/workflowRunStore.ts index 1c64c0c58..383ce0c69 100644 --- a/src/areas/workflows/workflowRunStore.ts +++ b/src/areas/workflows/workflowRunStore.ts @@ -3,7 +3,7 @@ import axios, { AxiosInstance } from 'axios' import { useAppStore } from '@shared/stores/appStore' import { getWorkflowExtension } from './mockExtensions' import { hasUnsupportedSceneShape } from './sceneShape' -import { showCompletionNotification, showErrorNotification } from '@shared/utils/notification' +import { showCompletionNotification, showRunErrorNotification } from '@shared/utils/notification' import type { WorkflowExtension } from './mockExtensions' import type { Workflow, WFNode, WFEdge } from '@shared/types/electron.d' import { isBranchStarter, isSceneOutput, resolveDataSource, reachesSceneOutput, nearestUpstreamWaits } from './nodeBehaviors' @@ -973,7 +973,7 @@ export const useWorkflowRunStore = create((set, get) => { if (!_cancel.current) { set((s) => ({ runState: { ...s.runState, status: 'error', error: String(err) }, activeNodeId: null })) useAppStore.getState().updateCurrentJob({ status: 'error', error: String(err) }) - void showErrorNotification(String(err), 'Workflow run failed') + void showRunErrorNotification({ title: 'Workflow run failed', name: workflow.name, error: err }) } } }, @@ -1032,6 +1032,7 @@ export const useWorkflowRunStore = create((set, get) => { for (const d of descendantWaits(waitId, ctx)) { if (newWaitStates[d] === 'blocked') newWaitStates[d] = 'error' } + void showRunErrorNotification({ title: 'Workflow branch failed', name: ctx.workflow.name, error: err }) } const allFinished = ctx.waitIds.every((id) => newWaitStates[id] === 'done' || newWaitStates[id] === 'error') const anyError = ctx.waitIds.some((id) => newWaitStates[id] === 'error') diff --git a/src/areas/workflows/workflowSceneRun.test.mjs b/src/areas/workflows/workflowSceneRun.test.mjs index cb43b9d55..86600a841 100644 --- a/src/areas/workflows/workflowSceneRun.test.mjs +++ b/src/areas/workflows/workflowSceneRun.test.mjs @@ -17,7 +17,7 @@ useAppStore.getState = () => appState `) const axiosStub = stub('axios.ts', `const axios: any = { create: () => (globalThis as any).__sceneClient }; export default axios; export type AxiosInstance = any`) const extStub = stub('ext.ts', `export const getWorkflowExtension = (id: string, all: any[]) => all.find((value) => value.id === id); export type WorkflowExtension = any`) -const notifyStub = stub('notify.ts', `export const showCompletionNotification = async () => {}; export const showErrorNotification = async () => {}`) +const notifyStub = stub('notify.ts', `export const showCompletionNotification = async () => {}; export const showRunErrorNotification = async () => {}`) const aliases = new Map([ ['axios', axiosStub], ['@shared/stores/appStore', appStoreStub], ['./mockExtensions', extStub], ['@shared/utils/notification', notifyStub], diff --git a/src/shared/hooks/useGeneration.ts b/src/shared/hooks/useGeneration.ts index 6f3d3a375..a2b0f9cf3 100644 --- a/src/shared/hooks/useGeneration.ts +++ b/src/shared/hooks/useGeneration.ts @@ -1,7 +1,7 @@ import { useCallback, useRef } from 'react' import { useAppStore } from '@shared/stores/appStore' import { useApi } from './useApi' -import { showCompletionNotification, showErrorNotification } from '@shared/utils/notification' +import { showCompletionNotification, showRunErrorNotification } from '@shared/utils/notification' export function useGeneration() { const { currentJob, setCurrentJob, updateCurrentJob, generationOptions, selectedImageData, pushMeshUrl, clearMeshHistory } = useAppStore() @@ -36,7 +36,7 @@ export function useGeneration() { updateCurrentJob({ status: 'generating', progress: 0 }) - await pollUntilDone(jobId) + await pollUntilDone(jobId, job.modelId) } catch (err) { if (cancelledRef.current) { setCurrentJob(null) @@ -53,14 +53,14 @@ export function useGeneration() { status: 'error', error: errorMessage }) - void showErrorNotification(errorMessage, 'Generation failed') + void showRunErrorNotification({ title: 'Generation failed', name: job.modelId, error: errorMessage }) } }, // eslint-disable-next-line react-hooks/exhaustive-deps -- useApi re-creates its fns each render, so this re-memoizes anyway (values stay fresh) [generateFromImage, pollJobStatus, cancelJob, setCurrentJob, updateCurrentJob] ) - const pollUntilDone = async (jobId: string) => { + const pollUntilDone = async (jobId: string, modelId: string) => { while (true) { await new Promise((r) => setTimeout(r, 1000)) @@ -86,7 +86,7 @@ export function useGeneration() { if (result.status === 'error') { updateCurrentJob({ status: 'error', error: result.error }) - void showErrorNotification(result.error ?? 'Unknown error', 'Generation failed') + void showRunErrorNotification({ title: 'Generation failed', name: modelId, error: result.error, cancelled: cancelledRef.current }) break } diff --git a/src/shared/utils/notification.test.ts b/src/shared/utils/notification.test.ts new file mode 100644 index 000000000..adb954d50 --- /dev/null +++ b/src/shared/utils/notification.test.ts @@ -0,0 +1,59 @@ +import test from 'node:test' +import assert from 'node:assert/strict' + +import { buildRunErrorNotification } from './notification.ts' + +test('a cancelled run never notifies', () => { + assert.equal( + buildRunErrorNotification({ title: 'Generation failed', error: 'aborted', cancelled: true }), + null, + ) +}) + +test('body is the run name followed by the error message', () => { + assert.deepEqual( + buildRunErrorNotification({ title: 'Workflow run failed', name: 'Bake textures', error: 'CUDA out of memory' }), + { title: 'Workflow run failed', body: 'Bake textures: CUDA out of memory' }, + ) +}) + +test('without a name the body is just the error message', () => { + assert.deepEqual( + buildRunErrorNotification({ title: 'Generation failed', error: 'boom' }), + { title: 'Generation failed', body: 'boom' }, + ) +}) + +test('keeps only the first non-empty line of a multi-line error (e.g. a traceback)', () => { + const n = buildRunErrorNotification({ + title: 'Generation failed', + error: '\n RuntimeError: weights missing \nTraceback (most recent call last):\n File "x.py"', + }) + assert.equal(n?.body, 'RuntimeError: weights missing') +}) + +test('strips the generic "Error: " prefix String(err) adds, but keeps specific error types', () => { + assert.equal( + buildRunErrorNotification({ title: 't', error: new Error('Node failed') })?.body, + 'Node failed', + ) + assert.equal( + buildRunErrorNotification({ title: 't', error: String(new Error('Error: nested')) })?.body, + 'nested', + ) + assert.equal( + buildRunErrorNotification({ title: 't', error: String(new TypeError('bad input')) })?.body, + 'TypeError: bad input', + ) +}) + +test('truncates long messages with an ellipsis', () => { + const body = buildRunErrorNotification({ title: 't', error: 'x'.repeat(500) })?.body ?? '' + assert.equal(body.length, 120) + assert.ok(body.endsWith('…')) +}) + +test('falls back to "Unknown error" for an empty or missing error', () => { + assert.equal(buildRunErrorNotification({ title: 't', error: undefined })?.body, 'Unknown error') + assert.equal(buildRunErrorNotification({ title: 't', name: 'triposr', error: ' ' })?.body, 'triposr: Unknown error') +}) diff --git a/src/shared/utils/notification.ts b/src/shared/utils/notification.ts index e51799740..0338dd66c 100644 --- a/src/shared/utils/notification.ts +++ b/src/shared/utils/notification.ts @@ -20,6 +20,34 @@ export function showCompletionNotification(body: string, title = 'Modly'): Promi return notifyIfUnfocused(title, body) } -export function showErrorNotification(body: string, title = 'Modly'): Promise { - return notifyIfUnfocused(title, body) +const MAX_ERROR_BODY = 120 + +export interface RunErrorInput { + /** Notification title, e.g. "Generation failed". */ + title: string + /** What failed — the model id or workflow name — prefixed to the message. */ + name?: string + error: unknown + /** The user stopped the run themselves: nothing to tell them. */ + cancelled?: boolean +} + +/** + * Decide whether a failed run deserves a notification and with what text: none + * for a user cancellation, otherwise the run's name plus the first line of the + * error (tracebacks and `Error: ` prefixes trimmed), short enough for a toast. + */ +export function buildRunErrorNotification(input: RunErrorInput): { title: string; body: string } | null { + if (input.cancelled) return null + const raw = input.error instanceof Error ? input.error.message : String(input.error ?? '') + const firstLine = raw.split(/\r?\n/).map((l) => l.trim()).find(Boolean) ?? '' + const message = firstLine.replace(/^(Error:\s*)+/, '') || 'Unknown error' + const full = input.name ? `${input.name}: ${message}` : message + const body = full.length > MAX_ERROR_BODY ? `${full.slice(0, MAX_ERROR_BODY - 1)}…` : full + return { title: input.title, body } +} + +export function showRunErrorNotification(input: RunErrorInput): Promise { + const notification = buildRunErrorNotification(input) + return notification ? notifyIfUnfocused(notification.title, notification.body) : Promise.resolve() }