From 504b71757581cc95cfa1519d8ccabb33d5bebd76 Mon Sep 17 00:00:00 2001
From: Chenlei Hu <huchenlei@proton.me>
Date: Fri, 28 Mar 2025 11:53:29 -0400
Subject: [PATCH] [Refactor] Unify error dialog component (#3265)

Co-authored-by: github-actions <github-actions@github.com>
---
 browser_tests/tests/dialog.spec.ts            |   2 +-
 .../dialog/content/ErrorDialogContent.vue     | 175 +++++++++++++-----
 .../content/ExecutionErrorDialogContent.vue   | 155 ----------------
 src/locales/en/main.json                      |   3 +-
 src/locales/es/main.json                      |   1 +
 src/locales/fr/main.json                      |   1 +
 src/locales/ja/main.json                      |   1 +
 src/locales/ko/main.json                      |   1 +
 src/locales/ru/main.json                      |   1 +
 src/locales/zh/main.json                      |   1 +
 src/scripts/app.ts                            |   4 +-
 src/services/dialogService.ts                 |  41 ++--
 src/utils/errorReportUtil.ts                  |  20 +-
 13 files changed, 178 insertions(+), 228 deletions(-)
 delete mode 100644 src/components/dialog/content/ExecutionErrorDialogContent.vue

diff --git a/browser_tests/tests/dialog.spec.ts b/browser_tests/tests/dialog.spec.ts
index bd8daec17..d309b2b1e 100644
--- a/browser_tests/tests/dialog.spec.ts
+++ b/browser_tests/tests/dialog.spec.ts
@@ -323,7 +323,7 @@ test.describe('Error dialog', () => {
 
     await comfyPage.loadWorkflow('default')
 
-    const errorDialog = comfyPage.page.locator('.error-dialog-content')
+    const errorDialog = comfyPage.page.locator('.comfy-error-report')
     await expect(errorDialog).toBeVisible()
   })
 })
diff --git a/src/components/dialog/content/ErrorDialogContent.vue b/src/components/dialog/content/ErrorDialogContent.vue
index a1cb4113f..361b23cb6 100644
--- a/src/components/dialog/content/ErrorDialogContent.vue
+++ b/src/components/dialog/content/ErrorDialogContent.vue
@@ -1,80 +1,159 @@
 <template>
-  <div class="error-dialog-content flex flex-col gap-4">
+  <div class="comfy-error-report flex flex-col gap-4">
     <NoResultsPlaceholder
       class="pb-0"
       icon="pi pi-exclamation-circle"
       :title="title"
-      :message="errorMessage"
+      :message="error.exceptionMessage"
     />
-    <pre
-      class="stack-trace p-5 text-neutral-400 text-xs max-h-[50vh] overflow-auto bg-black/20"
-    >
-      {{ stackTrace }}
-    </pre>
-
-    <template v-if="extensionFile">
+    <template v-if="error.extensionFile">
       <span>{{ t('errorDialog.extensionFileHint') }}:</span>
       <br />
-      <span class="font-bold">{{ extensionFile }}</span>
+      <span class="font-bold">{{ error.extensionFile }}</span>
     </template>
 
-    <Button
-      v-show="!sendReportOpen"
-      text
-      fluid
-      :label="$t('issueReport.helpFix')"
-      @click="showSendReport"
-    />
-
+    <div class="flex gap-2 justify-center">
+      <Button
+        v-show="!reportOpen"
+        text
+        :label="$t('g.showReport')"
+        @click="showReport"
+      />
+      <Button
+        v-show="!sendReportOpen"
+        text
+        :label="$t('issueReport.helpFix')"
+        @click="showSendReport"
+      />
+    </div>
+    <template v-if="reportOpen">
+      <Divider />
+      <ScrollPanel class="w-full h-[400px] max-w-[80vw]">
+        <pre class="whitespace-pre-wrap break-words">{{ reportContent }}</pre>
+      </ScrollPanel>
+      <Divider />
+    </template>
     <ReportIssuePanel
       v-if="sendReportOpen"
-      :error-type="errorType"
-      :extra-fields="[
-        {
-          label: t('issueReport.stackTrace'),
-          value: 'StackTrace',
-          optIn: true,
-          getData: () => stackTrace
-        }
-      ]"
+      :title="$t('issueReport.submitErrorReport')"
+      :error-type="error.reportType ?? 'unknownError'"
+      :extra-fields="[stackTraceField]"
       :tags="{
-        exceptionMessage: errorMessage,
-        extensionFile: extensionFile ?? 'UNKNOWN'
+        exceptionMessage: error.exceptionMessage,
+        nodeType: error.nodeType ?? 'UNKNOWN'
       }"
-      :title="t('issueReport.submitErrorReport')"
     />
+    <div class="flex gap-4 justify-end">
+      <FindIssueButton
+        :errorMessage="error.exceptionMessage"
+        :repoOwner="repoOwner"
+        :repoName="repoName"
+      />
+      <Button
+        v-if="reportOpen"
+        :label="$t('g.copyToClipboard')"
+        icon="pi pi-copy"
+        @click="copyReportToClipboard"
+      />
+    </div>
   </div>
 </template>
 
 <script setup lang="ts">
 import Button from 'primevue/button'
-import { computed, ref } from 'vue'
+import Divider from 'primevue/divider'
+import ScrollPanel from 'primevue/scrollpanel'
+import { useToast } from 'primevue/usetoast'
+import { computed, onMounted, ref } from 'vue'
 import { useI18n } from 'vue-i18n'
 
 import NoResultsPlaceholder from '@/components/common/NoResultsPlaceholder.vue'
+import FindIssueButton from '@/components/dialog/content/error/FindIssueButton.vue'
+import { useCopyToClipboard } from '@/composables/useCopyToClipboard'
+import { api } from '@/scripts/api'
+import { app } from '@/scripts/app'
+import { useSystemStatsStore } from '@/stores/systemStatsStore'
+import type { ReportField } from '@/types/issueReportTypes'
+import {
+  type ErrorReportData,
+  generateErrorReport
+} from '@/utils/errorReportUtil'
 
 import ReportIssuePanel from './error/ReportIssuePanel.vue'
 
-const { t } = useI18n()
-const {
-  title: _title,
-  errorMessage,
-  stackTrace: _stackTrace,
-  extensionFile,
-  errorType = 'frontendError'
-} = defineProps<{
-  title?: string
-  errorMessage: string
-  stackTrace?: string
-  extensionFile?: string
-  errorType?: string
+const { error } = defineProps<{
+  error: Omit<ErrorReportData, 'workflow' | 'systemStats' | 'serverLogs'> & {
+    /**
+     * The type of error report to submit.
+     * @default 'unknownError'
+     */
+    reportType?: string
+    /**
+     * The file name of the extension that caused the error.
+     */
+    extensionFile?: string
+  }
 }>()
 
-const title = computed(() => _title ?? t('errorDialog.defaultTitle'))
-const stackTrace = computed(() => _stackTrace ?? t('errorDialog.noStackTrace'))
-
+const repoOwner = 'comfyanonymous'
+const repoName = 'ComfyUI'
+const reportContent = ref('')
+const reportOpen = ref(false)
+const showReport = () => {
+  reportOpen.value = true
+}
 const sendReportOpen = ref(false)
-function showSendReport() {
+const showSendReport = () => {
   sendReportOpen.value = true
 }
+const toast = useToast()
+const { t } = useI18n()
+const systemStatsStore = useSystemStatsStore()
+
+const title = computed<string>(
+  () => error.nodeType ?? error.exceptionType ?? t('errorDialog.defaultTitle')
+)
+
+const stackTraceField = computed<ReportField>(() => {
+  return {
+    label: t('issueReport.stackTrace'),
+    value: 'StackTrace',
+    optIn: true,
+    getData: () => error.traceback
+  }
+})
+
+onMounted(async () => {
+  if (!systemStatsStore.systemStats) {
+    await systemStatsStore.fetchSystemStats()
+  }
+
+  try {
+    const [logs] = await Promise.all([api.getLogs()])
+
+    reportContent.value = generateErrorReport({
+      systemStats: systemStatsStore.systemStats!,
+      serverLogs: logs,
+      workflow: app.graph.serialize(),
+      exceptionType: error.exceptionType,
+      exceptionMessage: error.exceptionMessage,
+      traceback: error.traceback,
+      nodeId: error.nodeId,
+      nodeType: error.nodeType
+    })
+  } catch (error) {
+    console.error('Error fetching logs:', error)
+    toast.add({
+      severity: 'error',
+      summary: t('g.error'),
+      detail: t('toastMessages.failedToFetchLogs'),
+      life: 5000
+    })
+  }
+})
+
+const { copyToClipboard } = useCopyToClipboard()
+const copyReportToClipboard = async () => {
+  await copyToClipboard(reportContent.value)
+}
 </script>
diff --git a/src/components/dialog/content/ExecutionErrorDialogContent.vue b/src/components/dialog/content/ExecutionErrorDialogContent.vue
deleted file mode 100644
index 383384017..000000000
--- a/src/components/dialog/content/ExecutionErrorDialogContent.vue
+++ /dev/null
@@ -1,155 +0,0 @@
-<template>
-  <NoResultsPlaceholder
-    icon="pi pi-exclamation-circle"
-    :title="error.node_type"
-    :message="error.exception_message"
-  />
-  <div class="comfy-error-report">
-    <div class="flex gap-2 justify-center">
-      <Button
-        v-show="!reportOpen"
-        text
-        :label="$t('g.showReport')"
-        @click="showReport"
-      />
-      <Button
-        v-show="!sendReportOpen"
-        text
-        :label="$t('issueReport.helpFix')"
-        @click="showSendReport"
-      />
-    </div>
-    <template v-if="reportOpen">
-      <Divider />
-      <ScrollPanel style="width: 100%; height: 400px; max-width: 80vw">
-        <pre class="wrapper-pre">{{ reportContent }}</pre>
-      </ScrollPanel>
-      <Divider />
-    </template>
-    <ReportIssuePanel
-      v-if="sendReportOpen"
-      :title="$t('issueReport.submitErrorReport')"
-      error-type="graphExecutionError"
-      :extra-fields="[stackTraceField]"
-      :tags="{
-        exceptionMessage: error.exception_message,
-        nodeType: error.node_type
-      }"
-    />
-    <div class="action-container">
-      <FindIssueButton
-        :errorMessage="error.exception_message"
-        :repoOwner="repoOwner"
-        :repoName="repoName"
-      />
-      <Button
-        v-if="reportOpen"
-        :label="$t('g.copyToClipboard')"
-        icon="pi pi-copy"
-        @click="copyReportToClipboard"
-      />
-    </div>
-  </div>
-</template>
-
-<script setup lang="ts">
-import Button from 'primevue/button'
-import Divider from 'primevue/divider'
-import ScrollPanel from 'primevue/scrollpanel'
-import { useToast } from 'primevue/usetoast'
-import { computed, onMounted, ref } from 'vue'
-import { useI18n } from 'vue-i18n'
-
-import NoResultsPlaceholder from '@/components/common/NoResultsPlaceholder.vue'
-import FindIssueButton from '@/components/dialog/content/error/FindIssueButton.vue'
-import { useCopyToClipboard } from '@/composables/useCopyToClipboard'
-import type { ExecutionErrorWsMessage, SystemStats } from '@/schemas/apiSchema'
-import { api } from '@/scripts/api'
-import { app } from '@/scripts/app'
-import type { ReportField } from '@/types/issueReportTypes'
-import { generateErrorReport } from '@/utils/errorReportUtil'
-
-import ReportIssuePanel from './error/ReportIssuePanel.vue'
-
-const { error } = defineProps<{
-  error: ExecutionErrorWsMessage
-}>()
-
-const repoOwner = 'comfyanonymous'
-const repoName = 'ComfyUI'
-const reportContent = ref('')
-const reportOpen = ref(false)
-const showReport = () => {
-  reportOpen.value = true
-}
-const sendReportOpen = ref(false)
-const showSendReport = () => {
-  sendReportOpen.value = true
-}
-const toast = useToast()
-const { t } = useI18n()
-
-const stackTraceField = computed<ReportField>(() => {
-  return {
-    label: t('issueReport.stackTrace'),
-    value: 'StackTrace',
-    optIn: true,
-    getData: () => error.traceback?.join('\n')
-  }
-})
-
-onMounted(async () => {
-  try {
-    const [systemStats, logs] = await Promise.all([
-      api.getSystemStats(),
-      api.getLogs()
-    ])
-    generateReport(systemStats, logs)
-  } catch (error) {
-    console.error('Error fetching system stats or logs:', error)
-    toast.add({
-      severity: 'error',
-      summary: 'Error',
-      detail: 'Failed to fetch system information',
-      life: 5000
-    })
-  }
-})
-
-const generateReport = (systemStats: SystemStats, logs: string) => {
-  reportContent.value = generateErrorReport({
-    systemStats,
-    serverLogs: logs,
-    workflow: app.graph.serialize(),
-    exception_type: error.exception_type,
-    exception_message: error.exception_message,
-    traceback: error.traceback,
-    node_id: error.node_id,
-    node_type: error.node_type
-  })
-}
-
-const { copyToClipboard } = useCopyToClipboard()
-const copyReportToClipboard = async () => {
-  await copyToClipboard(reportContent.value)
-}
-</script>
-
-<style scoped>
-.comfy-error-report {
-  display: flex;
-  flex-direction: column;
-  gap: 1rem;
-}
-
-.action-container {
-  display: flex;
-  gap: 1rem;
-  justify-content: flex-end;
-}
-
-.wrapper-pre {
-  white-space: pre-wrap;
-  word-wrap: break-word;
-}
-</style>
diff --git a/src/locales/en/main.json b/src/locales/en/main.json
index 906193ca3..51815e4fe 100644
--- a/src/locales/en/main.json
+++ b/src/locales/en/main.json
@@ -1022,6 +1022,7 @@
     "nodeDefinitionsUpdated": "Node definitions updated",
     "errorSaveSetting": "Error saving setting {id}: {err}",
     "errorCopyImage": "Error copying image: {error}",
-    "noTemplatesToExport": "No templates to export"
+    "noTemplatesToExport": "No templates to export",
+    "failedToFetchLogs": "Failed to fetch server logs"
   }
 }
\ No newline at end of file
diff --git a/src/locales/es/main.json b/src/locales/es/main.json
index 8cffadbea..8849f0a42 100644
--- a/src/locales/es/main.json
+++ b/src/locales/es/main.json
@@ -995,6 +995,7 @@
     "failedToApplyTexture": "Error al aplicar textura",
     "failedToDownloadFile": "Error al descargar el archivo",
     "failedToExportModel": "Error al exportar modelo como {format}",
+    "failedToFetchLogs": "Error al obtener los registros del servidor",
     "fileLoadError": "No se puede encontrar el flujo de trabajo en {fileName}",
     "fileUploadFailed": "Error al subir el archivo",
     "interrupted": "La ejecución ha sido interrumpida",
diff --git a/src/locales/fr/main.json b/src/locales/fr/main.json
index 51c43da75..e16624ace 100644
--- a/src/locales/fr/main.json
+++ b/src/locales/fr/main.json
@@ -995,6 +995,7 @@
     "failedToApplyTexture": "Échec de l'application de la texture",
     "failedToDownloadFile": "Échec du téléchargement du fichier",
     "failedToExportModel": "Échec de l'exportation du modèle en {format}",
+    "failedToFetchLogs": "Échec de la récupération des journaux du serveur",
     "fileLoadError": "Impossible de trouver le flux de travail dans {fileName}",
     "fileUploadFailed": "Échec du téléchargement du fichier",
     "interrupted": "L'exécution a été interrompue",
diff --git a/src/locales/ja/main.json b/src/locales/ja/main.json
index 914c87574..fb9c9810f 100644
--- a/src/locales/ja/main.json
+++ b/src/locales/ja/main.json
@@ -995,6 +995,7 @@
     "failedToApplyTexture": "テクスチャの適用に失敗しました",
     "failedToDownloadFile": "ファイルのダウンロードに失敗しました",
     "failedToExportModel": "{format}としてモデルのエクスポートに失敗しました",
+    "failedToFetchLogs": "サーバーログの取得に失敗しました",
     "fileLoadError": "{fileName}でワークフローが見つかりません",
     "fileUploadFailed": "ファイルのアップロードに失敗しました",
     "interrupted": "実行が中断されました",
diff --git a/src/locales/ko/main.json b/src/locales/ko/main.json
index e0455f8a5..f060a967a 100644
--- a/src/locales/ko/main.json
+++ b/src/locales/ko/main.json
@@ -995,6 +995,7 @@
     "failedToApplyTexture": "텍스처 적용에 실패했습니다",
     "failedToDownloadFile": "파일 다운로드에 실패했습니다",
     "failedToExportModel": "{format} 형식으로 모델 내보내기에 실패했습니다",
+    "failedToFetchLogs": "서버 로그를 가져오는 데 실패했습니다",
     "fileLoadError": "{fileName}에서 워크플로우를 찾을 수 없습니다",
     "fileUploadFailed": "파일 업로드에 실패했습니다",
     "interrupted": "실행이 중단되었습니다",
diff --git a/src/locales/ru/main.json b/src/locales/ru/main.json
index b5fc38377..960cb57fb 100644
--- a/src/locales/ru/main.json
+++ b/src/locales/ru/main.json
@@ -995,6 +995,7 @@
     "failedToApplyTexture": "Не удалось применить текстуру",
     "failedToDownloadFile": "Не удалось скачать файл",
     "failedToExportModel": "Не удалось экспортировать модель как {format}",
+    "failedToFetchLogs": "Не удалось получить серверные логи",
     "fileLoadError": "Не удалось найти рабочий процесс в {fileName}",
     "fileUploadFailed": "Не удалось загрузить файл",
     "interrupted": "Выполнение было прервано",
diff --git a/src/locales/zh/main.json b/src/locales/zh/main.json
index 84b0219ee..e251f68dc 100644
--- a/src/locales/zh/main.json
+++ b/src/locales/zh/main.json
@@ -995,6 +995,7 @@
     "failedToApplyTexture": "应用纹理失败",
     "failedToDownloadFile": "文件下载失败",
     "failedToExportModel": "无法将模型导出为 {format}",
+    "failedToFetchLogs": "无法获取服务器日志",
     "fileLoadError": "无法在 {fileName} 中找到工作流",
     "fileUploadFailed": "文件上传失败",
     "interrupted": "执行已被中断",
diff --git a/src/scripts/app.ts b/src/scripts/app.ts
index 33e76bd7b..78c47a605 100644
--- a/src/scripts/app.ts
+++ b/src/scripts/app.ts
@@ -741,7 +741,7 @@ export class ComfyApp {
 
     api.addEventListener('execution_error', ({ detail }) => {
       this.lastExecutionError = detail
-      useDialogService().showExecutionErrorDialog({ error: detail })
+      useDialogService().showExecutionErrorDialog(detail)
       this.canvas.draw(true, true)
     })
 
@@ -1129,7 +1129,7 @@ export class ComfyApp {
     } catch (error) {
       useDialogService().showErrorDialog(error, {
         title: t('errorDialog.loadWorkflowTitle'),
-        errorType: 'loadWorkflowError'
+        reportType: 'loadWorkflowError'
       })
       return
     }
diff --git a/src/services/dialogService.ts b/src/services/dialogService.ts
index d9a5d3732..0b4e655eb 100644
--- a/src/services/dialogService.ts
+++ b/src/services/dialogService.ts
@@ -1,6 +1,5 @@
 import ConfirmationDialogContent from '@/components/dialog/content/ConfirmationDialogContent.vue'
 import ErrorDialogContent from '@/components/dialog/content/ErrorDialogContent.vue'
-import ExecutionErrorDialogContent from '@/components/dialog/content/ExecutionErrorDialogContent.vue'
 import IssueReportDialogContent from '@/components/dialog/content/IssueReportDialogContent.vue'
 import LoadWorkflowWarning from '@/components/dialog/content/LoadWorkflowWarning.vue'
 import ManagerProgressDialogContent from '@/components/dialog/content/ManagerProgressDialogContent.vue'
@@ -15,6 +14,7 @@ import SettingDialogHeader from '@/components/dialog/header/SettingDialogHeader.
 import TemplateWorkflowsContent from '@/components/templates/TemplateWorkflowsContent.vue'
 import TemplateWorkflowsDialogHeader from '@/components/templates/TemplateWorkflowsDialogHeader.vue'
 import { t } from '@/i18n'
+import type { ExecutionErrorWsMessage } from '@/schemas/apiSchema'
 import { type ShowDialogOptions, useDialogStore } from '@/stores/dialogStore'
 
 export type ConfirmationDialogType =
@@ -70,12 +70,21 @@ export const useDialogService = () => {
     })
   }
 
-  function showExecutionErrorDialog(
-    props: InstanceType<typeof ExecutionErrorDialogContent>['$props']
-  ) {
+  function showExecutionErrorDialog(executionError: ExecutionErrorWsMessage) {
+    const props: InstanceType<typeof ErrorDialogContent>['$props'] = {
+      error: {
+        exceptionType: executionError.exception_type,
+        exceptionMessage: executionError.exception_message,
+        nodeId: executionError.node_id,
+        nodeType: executionError.node_type,
+        traceback: executionError.traceback.join('\n'),
+        reportType: 'graphExecutionError'
+      }
+    }
+
     dialogStore.showDialog({
       key: 'global-execution-error',
-      component: ExecutionErrorDialogContent,
+      component: ErrorDialogContent,
       props
     })
   }
@@ -174,23 +183,33 @@ export const useDialogService = () => {
     error: unknown,
     options: {
       title?: string
-      errorType?: string
+      reportType?: string
     } = {}
   ) {
-    const props =
+    const errorProps: {
+      errorMessage: string
+      stackTrace?: string
+      extensionFile?: string
+    } =
       error instanceof Error
         ? parseError(error)
         : {
             errorMessage: String(error)
           }
 
+    const props: InstanceType<typeof ErrorDialogContent>['$props'] = {
+      error: {
+        exceptionType: options.title ?? 'Unknown Error',
+        exceptionMessage: errorProps.errorMessage,
+        traceback: errorProps.stackTrace ?? t('errorDialog.noStackTrace'),
+        reportType: options.reportType
+      }
+    }
+
     dialogStore.showDialog({
       key: 'global-error',
       component: ErrorDialogContent,
-      props: {
-        ...props,
-        ...options
-      }
+      props
     })
   }
 
diff --git a/src/utils/errorReportUtil.ts b/src/utils/errorReportUtil.ts
index 5f7692caa..643ec6d0f 100644
--- a/src/utils/errorReportUtil.ts
+++ b/src/utils/errorReportUtil.ts
@@ -4,15 +4,15 @@ import type { SystemStats } from '@/schemas/apiSchema'
 import type { NodeId } from '@/schemas/comfyWorkflowSchema'
 
 export interface ErrorReportData {
-  exception_type: string
-  exception_message: string
+  exceptionType: string
+  exceptionMessage: string
   systemStats: SystemStats
   serverLogs: string
   workflow: ISerialisedGraph
 
-  traceback?: string[]
-  node_id?: NodeId
-  node_type?: string
+  traceback?: string
+  nodeId?: NodeId
+  nodeType?: string
 }
 
 /**
@@ -35,13 +35,13 @@ export function generateErrorReport(error: ErrorReportData): string {
 ${
   error
     ? `## Error Details
-- **Node ID:** ${error.node_id || 'N/A'}
-- **Node Type:** ${error.node_type || 'N/A'}
-- **Exception Type:** ${error.exception_type || 'N/A'}
-- **Exception Message:** ${error.exception_message || 'N/A'}
+- **Node ID:** ${error.nodeId || 'N/A'}
+- **Node Type:** ${error.nodeType || 'N/A'}
+- **Exception Type:** ${error.exceptionType || 'N/A'}
+- **Exception Message:** ${error.exceptionMessage || 'N/A'}
 ## Stack Trace
 \`\`\`
-${error.traceback ? error.traceback.join('\n') : 'No stack trace available'}
+${error.traceback || 'No stack trace available'}
 \`\`\``
     : ''
 }