From 6a733918a78a685dd4ffc3bb30208dbddbfbc833 Mon Sep 17 00:00:00 2001 From: Jin Yi Date: Thu, 8 Jan 2026 10:54:01 +0900 Subject: [PATCH] Improve Import Failed Error Messages (#7871) --- .../dialog/content/MissingNodesFooter.vue | 2 +- src/locales/en/main.json | 4 ++ src/services/dialogService.ts | 41 +++++++++++ .../manager/ImportFailedNodeContent.vue | 70 +++++++++++++++++++ .../manager/ImportFailedNodeFooter.vue | 43 ++++++++++++ .../manager/ImportFailedNodeHeader.vue | 12 ++++ .../manager/NodeConflictDialogContent.test.ts | 8 ++- .../manager/NodeConflictDialogContent.vue | 13 ++-- .../components/manager/NodeConflictHeader.vue | 2 +- .../manager/button/PackEnableToggle.vue | 46 ++++++++---- .../infoPanel/tabs/WarningTabPanel.vue | 42 +++++------ .../composables/useConflictDetection.test.ts | 4 +- .../composables/useConflictDetection.ts | 20 ++++-- .../useImportFailedDetection.test.ts | 24 +++++-- .../composables/useImportFailedDetection.ts | 41 +++++------ 15 files changed, 284 insertions(+), 88 deletions(-) create mode 100644 src/workbench/extensions/manager/components/manager/ImportFailedNodeContent.vue create mode 100644 src/workbench/extensions/manager/components/manager/ImportFailedNodeFooter.vue create mode 100644 src/workbench/extensions/manager/components/manager/ImportFailedNodeHeader.vue diff --git a/src/components/dialog/content/MissingNodesFooter.vue b/src/components/dialog/content/MissingNodesFooter.vue index 3638e5ee4..ba203660e 100644 --- a/src/components/dialog/content/MissingNodesFooter.vue +++ b/src/components/dialog/content/MissingNodesFooter.vue @@ -22,7 +22,7 @@
- { }) } + function showImportFailedNodeDialog( + options: { + conflictedPackages?: ConflictDetectionResult[] + dialogComponentProps?: DialogComponentProps + } = {} + ) { + const { dialogComponentProps, conflictedPackages } = options + + return dialogStore.showDialog({ + key: 'global-import-failed', + headerComponent: ImportFailedNodeHeader, + footerComponent: ImportFailedNodeFooter, + component: ImportFailedNodeContent, + dialogComponentProps: { + closable: true, + pt: { + root: { class: 'bg-base-background border-border-default' }, + header: { class: '!p-0 !m-0' }, + content: { class: '!p-0 overflow-y-hidden' }, + footer: { class: '!p-0' }, + pcCloseButton: { + root: { + class: '!w-7 !h-7 !border-none !outline-none !p-2 !m-1.5' + } + } + }, + ...dialogComponentProps + }, + props: { + conflictedPackages: conflictedPackages ?? [] + }, + footerProps: { + conflictedPackages: conflictedPackages ?? [] + } + }) + } + function showNodeConflictDialog( options: { showAfterWhatsNew?: boolean @@ -561,6 +601,7 @@ export const useDialogService = () => { toggleManagerDialog, toggleManagerProgressDialog, showLayoutDialog, + showImportFailedNodeDialog, showNodeConflictDialog } } diff --git a/src/workbench/extensions/manager/components/manager/ImportFailedNodeContent.vue b/src/workbench/extensions/manager/components/manager/ImportFailedNodeContent.vue new file mode 100644 index 000000000..0edb63f5e --- /dev/null +++ b/src/workbench/extensions/manager/components/manager/ImportFailedNodeContent.vue @@ -0,0 +1,70 @@ + + + diff --git a/src/workbench/extensions/manager/components/manager/ImportFailedNodeFooter.vue b/src/workbench/extensions/manager/components/manager/ImportFailedNodeFooter.vue new file mode 100644 index 000000000..84b4dd6e4 --- /dev/null +++ b/src/workbench/extensions/manager/components/manager/ImportFailedNodeFooter.vue @@ -0,0 +1,43 @@ + + + diff --git a/src/workbench/extensions/manager/components/manager/ImportFailedNodeHeader.vue b/src/workbench/extensions/manager/components/manager/ImportFailedNodeHeader.vue new file mode 100644 index 000000000..0588e8c5f --- /dev/null +++ b/src/workbench/extensions/manager/components/manager/ImportFailedNodeHeader.vue @@ -0,0 +1,12 @@ + + + diff --git a/src/workbench/extensions/manager/components/manager/NodeConflictDialogContent.test.ts b/src/workbench/extensions/manager/components/manager/NodeConflictDialogContent.test.ts index b5b072482..e169ec9f0 100644 --- a/src/workbench/extensions/manager/components/manager/NodeConflictDialogContent.test.ts +++ b/src/workbench/extensions/manager/components/manager/NodeConflictDialogContent.test.ts @@ -245,7 +245,7 @@ describe('NodeConflictDialogContent', () => { await conflictsHeader.trigger('click') // Should be expanded now - const conflictItems = wrapper.findAll('.conflict-list-item') + const conflictItems = wrapper.findAll('[aria-label*="Conflict:"]') expect(conflictItems.length).toBeGreaterThan(0) }) @@ -324,7 +324,7 @@ describe('NodeConflictDialogContent', () => { await conflictsHeader.trigger('click') // Should display conflict messages (excluding import_failed) - const conflictItems = wrapper.findAll('.conflict-list-item') + const conflictItems = wrapper.findAll('[aria-label*="Conflict:"]') expect(conflictItems).toHaveLength(3) // 2 from Package1 + 1 from Package2 }) @@ -338,7 +338,9 @@ describe('NodeConflictDialogContent', () => { await importFailedHeader.trigger('click') // Should display only import failed package - const importFailedItems = wrapper.findAll('.conflict-list-item') + const importFailedItems = wrapper.findAll( + '[aria-label*="Import failed package:"]' + ) expect(importFailedItems).toHaveLength(1) expect(importFailedItems[0].text()).toContain('Test Package 3') }) diff --git a/src/workbench/extensions/manager/components/manager/NodeConflictDialogContent.vue b/src/workbench/extensions/manager/components/manager/NodeConflictDialogContent.vue index c7eda7f09..9cf091ffc 100644 --- a/src/workbench/extensions/manager/components/manager/NodeConflictDialogContent.vue +++ b/src/workbench/extensions/manager/components/manager/NodeConflictDialogContent.vue @@ -50,7 +50,8 @@
{{ packageName }} @@ -98,7 +99,8 @@
{{ getConflictMessage(conflict, t) @@ -146,7 +148,7 @@
{{ conflictResult.package_name }} @@ -236,8 +238,3 @@ const toggleExtensionsPanel = () => { importFailedExpanded.value = false } - diff --git a/src/workbench/extensions/manager/components/manager/NodeConflictHeader.vue b/src/workbench/extensions/manager/components/manager/NodeConflictHeader.vue index 368a1229f..24a1820d3 100644 --- a/src/workbench/extensions/manager/components/manager/NodeConflictHeader.vue +++ b/src/workbench/extensions/manager/components/manager/NodeConflictHeader.vue @@ -2,7 +2,7 @@
- +

{{ $t('manager.conflicts.title') }} diff --git a/src/workbench/extensions/manager/components/manager/button/PackEnableToggle.vue b/src/workbench/extensions/manager/components/manager/button/PackEnableToggle.vue index 82805e6c2..2317810cf 100644 --- a/src/workbench/extensions/manager/components/manager/button/PackEnableToggle.vue +++ b/src/workbench/extensions/manager/components/manager/button/PackEnableToggle.vue @@ -41,6 +41,8 @@ import { useComfyManagerStore } from '@/workbench/extensions/manager/stores/comf import { useConflictDetectionStore } from '@/workbench/extensions/manager/stores/conflictDetectionStore' import type { components as ManagerComponents } from '@/workbench/extensions/manager/types/generatedManagerTypes' +import { useImportFailedDetection } from '../../../composables/useImportFailedDetection' + const TOGGLE_DEBOUNCE_MS = 256 const { nodePack } = defineProps<{ @@ -53,6 +55,7 @@ const { isPackEnabled, enablePack, disablePack, installedPacks } = const { getConflictsForPackageByID } = useConflictDetectionStore() const { showNodeConflictDialog } = useDialogService() const { acknowledgmentState, markConflictsAsSeen } = useConflictAcknowledgment() +const { showImportFailedDialog } = useImportFailedDetection(nodePack.id || '') const isLoading = ref(false) @@ -81,23 +84,36 @@ const canToggleDirectly = computed(() => { const showConflictModal = (skipModalDismissed: boolean) => { let modal_dismissed = acknowledgmentState.value.modal_dismissed if (skipModalDismissed) modal_dismissed = false + if (packageConflict.value && !modal_dismissed) { - showNodeConflictDialog({ - conflictedPackages: [packageConflict.value], - buttonText: !isEnabled.value - ? t('manager.conflicts.enableAnyway') - : t('manager.conflicts.understood'), - onButtonClick: async () => { - if (!isEnabled.value) { - await handleEnable() + // Check if there's an import failed conflict first + const hasImportFailed = packageConflict.value.conflicts.some( + (conflict) => conflict.type === 'import_failed' + ) + if (hasImportFailed) { + // Show import failed dialog instead of general conflict dialog + showImportFailedDialog(() => { + markConflictsAsSeen() + }) + } else { + // Show general conflict dialog for other types of conflicts + showNodeConflictDialog({ + conflictedPackages: [packageConflict.value], + buttonText: !isEnabled.value + ? t('manager.conflicts.enableAnyway') + : t('manager.conflicts.understood'), + onButtonClick: async () => { + if (!isEnabled.value) { + await handleEnable() + } + }, + dialogComponentProps: { + onClose: () => { + markConflictsAsSeen() + } } - }, - dialogComponentProps: { - onClose: () => { - markConflictsAsSeen() - } - } - }) + }) + } } } diff --git a/src/workbench/extensions/manager/components/manager/infoPanel/tabs/WarningTabPanel.vue b/src/workbench/extensions/manager/components/manager/infoPanel/tabs/WarningTabPanel.vue index f673318da..472a60e1d 100644 --- a/src/workbench/extensions/manager/components/manager/infoPanel/tabs/WarningTabPanel.vue +++ b/src/workbench/extensions/manager/components/manager/infoPanel/tabs/WarningTabPanel.vue @@ -1,43 +1,37 @@ diff --git a/src/workbench/extensions/manager/composables/useConflictDetection.test.ts b/src/workbench/extensions/manager/composables/useConflictDetection.test.ts index 70b512bdf..28fa8302c 100644 --- a/src/workbench/extensions/manager/composables/useConflictDetection.test.ts +++ b/src/workbench/extensions/manager/composables/useConflictDetection.test.ts @@ -410,7 +410,7 @@ describe('useConflictDetection', () => { mockComfyManagerService.getImportFailInfoBulk ).mockResolvedValue({ 'fail-pack': { - msg: 'Import error', + error: 'Import error', name: 'fail-pack', path: '/path/to/pack' } as any // The actual API returns different structure than types @@ -428,7 +428,7 @@ describe('useConflictDetection', () => { // Import failure should match the actual implementation expect(result.results[0].conflicts).toContainEqual({ type: 'import_failed', - current_value: 'installed', + current_value: 'Import error', required_value: 'Import error' }) }) diff --git a/src/workbench/extensions/manager/composables/useConflictDetection.ts b/src/workbench/extensions/manager/composables/useConflictDetection.ts index 43a6fd3d2..4483e25d9 100644 --- a/src/workbench/extensions/manager/composables/useConflictDetection.ts +++ b/src/workbench/extensions/manager/composables/useConflictDetection.ts @@ -389,7 +389,10 @@ export function useConflictDetection() { * @returns Array of conflict detection results for failed imports */ function detectImportFailConflicts( - importFailInfo: Record + importFailInfo: Record< + string, + { error?: string; traceback?: string } | null + > ): ConflictDetectionResult[] { const results: ConflictDetectionResult[] = [] if (!importFailInfo || typeof importFailInfo !== 'object') { @@ -400,8 +403,11 @@ export function useConflictDetection() { for (const [packageId, failureInfo] of Object.entries(importFailInfo)) { if (failureInfo && typeof failureInfo === 'object') { // Extract error information from Manager API response - const errorMsg = failureInfo.msg || 'Unknown import error' - const modulePath = failureInfo.path || '' + const errorMsg = failureInfo.error || 'Unknown import error' + const traceback = failureInfo.traceback || '' + + // Combine error and traceback for display + const fullErrorInfo = traceback || errorMsg results.push({ package_id: packageId, @@ -410,8 +416,8 @@ export function useConflictDetection() { conflicts: [ { type: 'import_failed', - current_value: 'installed', - required_value: failureInfo.msg + current_value: errorMsg, + required_value: fullErrorInfo } ], is_compatible: false @@ -420,8 +426,8 @@ export function useConflictDetection() { console.warn( `[ConflictDetection] Python import failure detected for ${packageId}:`, { - path: modulePath, - error: errorMsg + error: errorMsg, + hasTraceback: !!traceback } ) } diff --git a/src/workbench/extensions/manager/composables/useImportFailedDetection.test.ts b/src/workbench/extensions/manager/composables/useImportFailedDetection.test.ts index 39619d5ed..d548af8da 100644 --- a/src/workbench/extensions/manager/composables/useImportFailedDetection.test.ts +++ b/src/workbench/extensions/manager/composables/useImportFailedDetection.test.ts @@ -44,7 +44,8 @@ describe('useImportFailedDetection', () => { > mockDialogService = { - showErrorDialog: vi.fn() + showErrorDialog: vi.fn(), + showImportFailedNodeDialog: vi.fn() } as unknown as ReturnType vi.mocked(comfyManagerStore.useComfyManagerStore).mockReturnValue( @@ -226,13 +227,22 @@ describe('useImportFailedDetection', () => { showImportFailedDialog() - expect(mockDialogService.showErrorDialog).toHaveBeenCalledWith( - expect.any(Error), - { - title: 'manager.failedToInstall', - reportType: 'importFailedError' + expect(mockDialogService.showImportFailedNodeDialog).toHaveBeenCalledWith({ + conflictedPackages: expect.arrayContaining([ + expect.objectContaining({ + package_id: 'test-package', + package_name: 'Test Package', + conflicts: expect.arrayContaining([ + expect.objectContaining({ + type: 'import_failed' + }) + ]) + }) + ]), + dialogComponentProps: { + onClose: undefined } - ) + }) }) it('should handle null packageId', () => { diff --git a/src/workbench/extensions/manager/composables/useImportFailedDetection.ts b/src/workbench/extensions/manager/composables/useImportFailedDetection.ts index affd47c3a..1edc65006 100644 --- a/src/workbench/extensions/manager/composables/useImportFailedDetection.ts +++ b/src/workbench/extensions/manager/composables/useImportFailedDetection.ts @@ -1,11 +1,13 @@ import { computed, unref } from 'vue' import type { ComputedRef } from 'vue' -import { useI18n } from 'vue-i18n' import { useDialogService } from '@/services/dialogService' import { useComfyManagerStore } from '@/workbench/extensions/manager/stores/comfyManagerStore' import { useConflictDetectionStore } from '@/workbench/extensions/manager/stores/conflictDetectionStore' -import type { ConflictDetail } from '@/workbench/extensions/manager/types/conflictDetectionTypes' +import type { + ConflictDetail, + ConflictDetectionResult +} from '@/workbench/extensions/manager/types/conflictDetectionTypes' /** * Extracting import failed conflicts from conflict list @@ -24,22 +26,18 @@ function extractImportFailedConflicts(conflicts?: ConflictDetail[] | null) { * Creating import failed dialog */ function createImportFailedDialog() { - const { t } = useI18n() - const { showErrorDialog } = useDialogService() + const { showImportFailedNodeDialog } = useDialogService() - return (importFailedInfo: ConflictDetail[] | null) => { - if (importFailedInfo) { - const errorMessage = - importFailedInfo - .map((conflict) => conflict.required_value) - .filter(Boolean) - .join('\n') || t('manager.importFailedGenericError') - - const error = new Error(errorMessage) - - showErrorDialog(error, { - title: t('manager.failedToInstall'), - reportType: 'importFailedError' + return ( + conflictedPackages: ConflictDetectionResult[] | null, + onClose?: () => void + ) => { + if (conflictedPackages && conflictedPackages.length > 0) { + showImportFailedNodeDialog({ + conflictedPackages, + dialogComponentProps: { + onClose + } }) } } @@ -74,13 +72,16 @@ export function useImportFailedDetection( return importFailedInfo.value !== null }) - const showImportFailedDialog = createImportFailedDialog() + const openDialog = createImportFailedDialog() return { importFailedInfo, importFailed, - showImportFailedDialog: () => - showImportFailedDialog(importFailedInfo.value), + showImportFailedDialog: (onClose?: () => void) => { + if (conflicts.value) { + openDialog([conflicts.value], onClose) + } + }, isInstalled } }