refactor: encapsulate error extraction in TaskItemImpl getters

Move error extraction logic from standalone extractExecutionError function into TaskItemImpl.errorMessage and TaskItemImpl.executionError getters. This encapsulates the error extraction pattern within the class, preparing for the Jobs API migration where the underlying data format will change but the getter interface will remain stable. Also add ExecutionErrorDialogInput interface to dialogService that accepts both ExecutionErrorWsMessage (WebSocket) and ExecutionError (Jobs API) formats, enabling the error dialog to work with either source.
2026-03-06 13:40:25 +00:00 · 2025-12-19 15:13:54 -08:00
parent 71c6d62c7e
commit 6485f871ba
5 changed files with 168 additions and 110 deletions
--- a/src/components/queue/job/useJobErrorReporting.test.ts
+++ b/src/components/queue/job/useJobErrorReporting.test.ts
@@ -2,13 +2,10 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
 import { computed, ref } from 'vue'
 import type { ComputedRef } from 'vue'

-import type { ExecutionErrorWsMessage } from '@/schemas/apiSchema'
-import type { TaskItemImpl } from '@/stores/queueStore'
-import type {
-  JobErrorDialogService,
-  UseJobErrorReportingOptions
-} from '@/components/queue/job/useJobErrorReporting'
-import * as jobErrorReporting from '@/components/queue/job/useJobErrorReporting'
+import type { ExecutionErrorWsMessage, TaskStatus } from '@/schemas/apiSchema'
+import { TaskItemImpl } from '@/stores/queueStore'
+import type { JobErrorDialogService } from '@/components/queue/job/useJobErrorReporting'
+import { useJobErrorReporting } from '@/components/queue/job/useJobErrorReporting'

 const createExecutionErrorMessage = (
  overrides: Partial<ExecutionErrorWsMessage> = {}
@@ -26,78 +23,33 @@ const createExecutionErrorMessage = (
  ...overrides
 })

-const createTaskWithMessages = (
-  messages: Array<[string, unknown]> | undefined = []
-): TaskItemImpl =>
-  ({
-    status: {
-      status_str: 'error',
-      completed: false,
-      messages
-    }
-  }) as TaskItemImpl
-
-describe('extractExecutionError', () => {
-  it('returns null when task has no execution error messages', () => {
-    expect(jobErrorReporting.extractExecutionError(null)).toBeNull()
-    expect(
-      jobErrorReporting.extractExecutionError({
-        status: undefined
-      } as TaskItemImpl)
-    ).toBeNull()
-    expect(
-      jobErrorReporting.extractExecutionError({
-        status: {
-          status_str: 'error',
-          completed: false,
-          messages: {} as unknown as Array<[string, unknown]>
-        }
-      } as TaskItemImpl)
-    ).toBeNull()
-    expect(
-      jobErrorReporting.extractExecutionError(createTaskWithMessages([]))
-    ).toBeNull()
-    expect(
-      jobErrorReporting.extractExecutionError(
-        createTaskWithMessages([
-          ['execution_start', { prompt_id: 'prompt', timestamp: 1 }]
-        ] as Array<[string, unknown]>)
-      )
-    ).toBeNull()
-  })
-
-  it('returns detail and message for execution_error entries', () => {
-    const detail = createExecutionErrorMessage({ exception_message: 'Kaboom' })
-    const result = jobErrorReporting.extractExecutionError(
-      createTaskWithMessages([
-        ['execution_success', { prompt_id: 'prompt', timestamp: 2 }],
-        ['execution_error', detail]
-      ] as Array<[string, unknown]>)
-    )
-    expect(result).toEqual({
-      detail,
-      message: 'Kaboom'
-    })
-  })
-
-  it('falls back to an empty message when the tuple lacks detail', () => {
-    const result = jobErrorReporting.extractExecutionError(
-      createTaskWithMessages([
-        ['execution_error'] as unknown as [string, ExecutionErrorWsMessage]
-      ])
-    )
-    expect(result).toEqual({ detail: undefined, message: '' })
-  })
-})
+/**
+ * Creates a real TaskItemImpl with the given status messages.
+ * Uses the actual TaskItemImpl class to test the real errorMessage/executionError getters.
+ */
+function createTaskWithMessages(
+  messages: TaskStatus['messages'] = []
+): TaskItemImpl {
+  const status: TaskStatus = {
+    status_str: 'error',
+    completed: false,
+    messages
+  }
+  return new TaskItemImpl(
+    'History',
+    [0, 'test-prompt-id', {}, { client_id: 'test-client' }, []],
+    status
+  )
+}

 describe('useJobErrorReporting', () => {
  let taskState = ref<TaskItemImpl | null>(null)
  let taskForJob: ComputedRef<TaskItemImpl | null>
-  let copyToClipboard: UseJobErrorReportingOptions['copyToClipboard']
-  let showExecutionErrorDialog: JobErrorDialogService['showExecutionErrorDialog']
-  let showErrorDialog: JobErrorDialogService['showErrorDialog']
+  let copyToClipboard: ReturnType<typeof vi.fn>
+  let showExecutionErrorDialog: ReturnType<typeof vi.fn>
+  let showErrorDialog: ReturnType<typeof vi.fn>
  let dialog: JobErrorDialogService
-  let composable: ReturnType<typeof jobErrorReporting.useJobErrorReporting>
+  let composable: ReturnType<typeof useJobErrorReporting>

  beforeEach(() => {
    taskState = ref<TaskItemImpl | null>(null)
@@ -109,7 +61,7 @@ describe('useJobErrorReporting', () => {
      showExecutionErrorDialog,
      showErrorDialog
    }
-    composable = jobErrorReporting.useJobErrorReporting({
+    composable = useJobErrorReporting({
      taskForJob,
      copyToClipboard,
      dialog
@@ -138,6 +90,14 @@ describe('useJobErrorReporting', () => {
    expect(composable.errorMessageValue.value).toBe('Second failure')
  })

+  it('returns empty string when task has no error', () => {
+    taskState.value = null
+    expect(composable.errorMessageValue.value).toBe('')
+
+    taskState.value = createTaskWithMessages([])
+    expect(composable.errorMessageValue.value).toBe('')
+  })
+
  it('only calls the copy handler when a message exists', () => {
    taskState.value = createTaskWithMessages([
      [
@@ -149,7 +109,7 @@ describe('useJobErrorReporting', () => {
    expect(copyToClipboard).toHaveBeenCalledTimes(1)
    expect(copyToClipboard).toHaveBeenCalledWith('Clipboard failure')

-    vi.mocked(copyToClipboard).mockClear()
+    copyToClipboard.mockClear()
    taskState.value = createTaskWithMessages([])
    composable.copyErrorMessage()
    expect(copyToClipboard).not.toHaveBeenCalled()
@@ -177,7 +137,7 @@ describe('useJobErrorReporting', () => {
    composable.reportJobError()
    expect(showExecutionErrorDialog).not.toHaveBeenCalled()
    expect(showErrorDialog).toHaveBeenCalledTimes(1)
-    const [errorArg, optionsArg] = vi.mocked(showErrorDialog).mock.calls[0]
+    const [errorArg, optionsArg] = showErrorDialog.mock.calls[0]
    expect(errorArg).toBeInstanceOf(Error)
    expect(errorArg.message).toBe(message)
    expect(optionsArg).toEqual({ reportType: 'queueJobError' })
--- a/src/components/queue/job/useJobErrorReporting.ts
+++ b/src/components/queue/job/useJobErrorReporting.ts
@@ -1,13 +1,13 @@
 import { computed } from 'vue'
 import type { ComputedRef } from 'vue'

-import type { ExecutionErrorWsMessage } from '@/schemas/apiSchema'
+import type { ExecutionErrorDialogInput } from '@/services/dialogService'
 import type { TaskItemImpl } from '@/stores/queueStore'

 type CopyHandler = (value: string) => void | Promise<void>

 export type JobErrorDialogService = {
-  showExecutionErrorDialog: (error: ExecutionErrorWsMessage) => void
+  showExecutionErrorDialog: (executionError: ExecutionErrorDialogInput) => void
  showErrorDialog: (
    error: Error,
    options?: {
@@ -17,30 +17,7 @@ export type JobErrorDialogService = {
  ) => void
 }

-type JobExecutionError = {
-  detail?: ExecutionErrorWsMessage
-  message: string
-}
-
-export const extractExecutionError = (
-  task: TaskItemImpl | null
-): JobExecutionError | null => {
-  const status = (task as TaskItemImpl | null)?.status
-  const messages = (status as { messages?: unknown[] } | undefined)?.messages
-  if (!Array.isArray(messages) || !messages.length) return null
-  const record = messages.find((entry: unknown) => {
-    return Array.isArray(entry) && entry[0] === 'execution_error'
-  }) as [string, ExecutionErrorWsMessage?] | undefined
-  if (!record) return null
-  const detail = record[1]
-  const message = String(detail?.exception_message ?? '')
-  return {
-    detail,
-    message
-  }
-}
-
-export type UseJobErrorReportingOptions = {
+type UseJobErrorReportingOptions = {
  taskForJob: ComputedRef<TaskItemImpl | null>
  copyToClipboard: CopyHandler
  dialog: JobErrorDialogService
@@ -52,8 +29,7 @@ export const useJobErrorReporting = ({
  dialog
 }: UseJobErrorReportingOptions) => {
  const errorMessageValue = computed(() => {
-    const error = extractExecutionError(taskForJob.value)
-    return error?.message ?? ''
+    return taskForJob.value?.executionError?.exception_message ?? ''
  })

  const copyErrorMessage = () => {
@@ -63,9 +39,9 @@ export const useJobErrorReporting = ({
  }

  const reportJobError = () => {
-    const error = extractExecutionError(taskForJob.value)
-    if (error?.detail) {
-      dialog.showExecutionErrorDialog(error.detail)
+    const executionError = taskForJob.value?.executionError
+    if (executionError) {
+      dialog.showExecutionErrorDialog(executionError)
      return
    }
    if (errorMessageValue.value) {
--- a/src/services/dialogService.ts
+++ b/src/services/dialogService.ts
@@ -19,7 +19,6 @@ import { useTelemetry } from '@/platform/telemetry'
 import { isCloud } from '@/platform/distribution/types'
 import { useSubscription } from '@/platform/cloud/subscription/composables/useSubscription'
 import SettingDialogContent from '@/platform/settings/components/SettingDialogContent.vue'
-import type { ExecutionErrorWsMessage } from '@/schemas/apiSchema'
 import { useDialogStore } from '@/stores/dialogStore'
 import type {
  DialogComponentProps,
@@ -45,6 +44,18 @@ export type ConfirmationDialogType =
  | 'dirtyClose'
  | 'reinstall'

+/**
+ * Minimal interface for execution error dialogs.
+ * Satisfied by both ExecutionErrorWsMessage (WebSocket) and ExecutionError (Jobs API).
+ */
+export interface ExecutionErrorDialogInput {
+  exception_type: string
+  exception_message: string
+  node_id: string | number
+  node_type: string
+  traceback: string[]
+}
+
 export const useDialogService = () => {
  const dialogStore = useDialogStore()

@@ -115,7 +126,7 @@ export const useDialogService = () => {
    })
  }

-  function showExecutionErrorDialog(executionError: ExecutionErrorWsMessage) {
+  function showExecutionErrorDialog(executionError: ExecutionErrorDialogInput) {
    const props: ComponentAttrs<typeof ErrorDialogContent> = {
      error: {
        exceptionType: executionError.exception_type,
--- a/src/stores/queueStore.test.ts
+++ b/src/stores/queueStore.test.ts
@@ -232,6 +232,90 @@ describe('TaskItemImpl', () => {
      })
    })
  })
+
+  describe('error extraction getters', () => {
+    it('errorMessage returns undefined when no execution_error message', () => {
+      const taskItem = new TaskItemImpl(
+        'History',
+        [0, 'prompt-id', {}, { client_id: 'client-id' }, []],
+        { status_str: 'success', messages: [], completed: true }
+      )
+      expect(taskItem.errorMessage).toBeUndefined()
+    })
+
+    it('errorMessage returns undefined when status has no messages', () => {
+      const taskItem = new TaskItemImpl(
+        'History',
+        [0, 'prompt-id', {}, { client_id: 'client-id' }, []],
+        { status_str: 'error', completed: false } as any
+      )
+      expect(taskItem.errorMessage).toBeUndefined()
+    })
+
+    it('errorMessage returns the exception_message from execution_error', () => {
+      const taskItem = new TaskItemImpl(
+        'History',
+        [0, 'prompt-id', {}, { client_id: 'client-id' }, []],
+        {
+          status_str: 'error',
+          completed: false,
+          messages: [
+            ['execution_start', { prompt_id: 'prompt-id', timestamp: 1 }],
+            [
+              'execution_error',
+              {
+                prompt_id: 'prompt-id',
+                timestamp: 2,
+                node_id: 'node-1',
+                node_type: 'KSampler',
+                executed: [],
+                exception_message: 'GPU out of memory',
+                exception_type: 'RuntimeError',
+                traceback: ['line 1', 'line 2'],
+                current_inputs: {},
+                current_outputs: {}
+              }
+            ]
+          ]
+        }
+      )
+      expect(taskItem.errorMessage).toBe('GPU out of memory')
+    })
+
+    it('executionError returns undefined when no execution_error message', () => {
+      const taskItem = new TaskItemImpl(
+        'History',
+        [0, 'prompt-id', {}, { client_id: 'client-id' }, []],
+        { status_str: 'success', messages: [], completed: true }
+      )
+      expect(taskItem.executionError).toBeUndefined()
+    })
+
+    it('executionError returns the full error object from execution_error', () => {
+      const errorDetail = {
+        prompt_id: 'prompt-id',
+        timestamp: 2,
+        node_id: 'node-1',
+        node_type: 'KSampler',
+        executed: ['node-0'],
+        exception_message: 'Invalid dimensions',
+        exception_type: 'ValueError',
+        traceback: ['traceback line'],
+        current_inputs: { input1: 'value' },
+        current_outputs: {}
+      }
+      const taskItem = new TaskItemImpl(
+        'History',
+        [0, 'prompt-id', {}, { client_id: 'client-id' }, []],
+        {
+          status_str: 'error',
+          completed: false,
+          messages: [['execution_error', errorDetail]]
+        }
+      )
+      expect(taskItem.executionError).toEqual(errorDetail)
+    })
+  })
 })

 describe('useQueueStore', () => {
--- a/src/stores/queueStore.ts
+++ b/src/stores/queueStore.ts
@@ -11,6 +11,7 @@ import type {
  NodeId
 } from '@/platform/workflow/validation/schemas/workflowSchema'
 import type {
+  ExecutionErrorWsMessage,
  HistoryTaskItem,
  ResultItem,
  StatusWsMessageStatus,
@@ -320,6 +321,32 @@ export class TaskItemImpl {
    return this.status?.messages || []
  }

+  /**
+   * Extracts the execution error message from status messages.
+   * Used by error reporting UI components.
+   */
+  get errorMessage(): string | undefined {
+    const messages = this.status?.messages
+    if (!Array.isArray(messages) || !messages.length) return undefined
+    const record = messages.find(
+      (entry: unknown) => Array.isArray(entry) && entry[0] === 'execution_error'
+    ) as [string, { exception_message?: string }?] | undefined
+    return record?.[1]?.exception_message
+  }
+
+  /**
+   * Extracts the full execution error from status messages.
+   * Returns the ExecutionErrorWsMessage for detailed error dialogs.
+   */
+  get executionError(): ExecutionErrorWsMessage | undefined {
+    const messages = this.status?.messages
+    if (!Array.isArray(messages) || !messages.length) return undefined
+    const record = messages.find(
+      (entry: unknown) => Array.isArray(entry) && entry[0] === 'execution_error'
+    ) as [string, ExecutionErrorWsMessage?] | undefined
+    return record?.[1]
+  }
+
  /**
   * Server-provided creation time in milliseconds, when available.
   *