Improve Import Failed Error Messages (#7871)

This commit is contained in:
Jin Yi
2026-01-08 10:54:01 +09:00
committed by GitHub
parent a87d2cf1bd
commit 6a733918a7
15 changed files with 284 additions and 88 deletions

View File

@@ -22,7 +22,7 @@
<!-- OSS mode: Open Manager + Install All buttons --> <!-- OSS mode: Open Manager + Install All buttons -->
<div v-else-if="showManagerButtons" class="flex justify-end gap-1 py-2 px-4"> <div v-else-if="showManagerButtons" class="flex justify-end gap-1 py-2 px-4">
<Button variant="textonly" size="sm" @click="openManager">{{ <Button variant="textonly" @click="openManager">{{
$t('g.openManager') $t('g.openManager')
}}</Button> }}</Button>
<PackInstallButton <PackInstallButton

View File

@@ -378,6 +378,10 @@
"warningTooltip": "This package may have compatibility issues with your current environment" "warningTooltip": "This package may have compatibility issues with your current environment"
} }
}, },
"importFailed": {
"title": "Import Failed",
"copyError": "Copy Error"
},
"issueReport": { "issueReport": {
"helpFix": "Help Fix This" "helpFix": "Help Fix This"
}, },

View File

@@ -30,6 +30,9 @@ import ManagerProgressFooter from '@/workbench/extensions/manager/components/Man
import ManagerProgressHeader from '@/workbench/extensions/manager/components/ManagerProgressHeader.vue' import ManagerProgressHeader from '@/workbench/extensions/manager/components/ManagerProgressHeader.vue'
import ManagerDialogContent from '@/workbench/extensions/manager/components/manager/ManagerDialogContent.vue' import ManagerDialogContent from '@/workbench/extensions/manager/components/manager/ManagerDialogContent.vue'
import ManagerHeader from '@/workbench/extensions/manager/components/manager/ManagerHeader.vue' import ManagerHeader from '@/workbench/extensions/manager/components/manager/ManagerHeader.vue'
import ImportFailedNodeContent from '@/workbench/extensions/manager/components/manager/ImportFailedNodeContent.vue'
import ImportFailedNodeFooter from '@/workbench/extensions/manager/components/manager/ImportFailedNodeFooter.vue'
import ImportFailedNodeHeader from '@/workbench/extensions/manager/components/manager/ImportFailedNodeHeader.vue'
import NodeConflictDialogContent from '@/workbench/extensions/manager/components/manager/NodeConflictDialogContent.vue' import NodeConflictDialogContent from '@/workbench/extensions/manager/components/manager/NodeConflictDialogContent.vue'
import NodeConflictFooter from '@/workbench/extensions/manager/components/manager/NodeConflictFooter.vue' import NodeConflictFooter from '@/workbench/extensions/manager/components/manager/NodeConflictFooter.vue'
import NodeConflictHeader from '@/workbench/extensions/manager/components/manager/NodeConflictHeader.vue' import NodeConflictHeader from '@/workbench/extensions/manager/components/manager/NodeConflictHeader.vue'
@@ -482,6 +485,43 @@ export const useDialogService = () => {
}) })
} }
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( function showNodeConflictDialog(
options: { options: {
showAfterWhatsNew?: boolean showAfterWhatsNew?: boolean
@@ -561,6 +601,7 @@ export const useDialogService = () => {
toggleManagerDialog, toggleManagerDialog,
toggleManagerProgressDialog, toggleManagerProgressDialog,
showLayoutDialog, showLayoutDialog,
showImportFailedNodeDialog,
showNodeConflictDialog showNodeConflictDialog
} }
} }

View File

@@ -0,0 +1,70 @@
<template>
<div class="flex w-[490px] flex-col border-t-1 border-border-default">
<div class="flex h-full w-full flex-col gap-4 p-4">
<!-- Error Details -->
<div v-if="importFailedPackages.length > 0" class="flex flex-col gap-3">
<div
v-for="pkg in importFailedPackages"
:key="pkg.packageId"
class="flex flex-col gap-2 max-h-60 overflow-x-hidden overflow-y-auto scrollbar-custom"
role="region"
:aria-label="`Error traceback for ${pkg.packageId}`"
tabindex="0"
>
<!-- Error Message -->
<div
v-if="pkg.traceback || pkg.errorMessage"
class="text-xs p-4 rounded-md bg-secondary-background font-mono"
>
{{ pkg.traceback || pkg.errorMessage }}
</div>
</div>
</div>
</div>
</div>
</template>
<script setup lang="ts">
import { computed } from 'vue'
import type { ConflictDetectionResult } from '@/workbench/extensions/manager/types/conflictDetectionTypes'
const { conflictedPackages } = defineProps<{
conflictedPackages: ConflictDetectionResult[]
}>()
interface ImportFailedPackage {
packageId: string
packageName: string
errorMessage: string
traceback: string
}
const importFailedPackages = computed((): ImportFailedPackage[] => {
return conflictedPackages
.filter((pkg) =>
pkg.conflicts.some((conflict) => conflict.type === 'import_failed')
)
.map((pkg) => {
const importFailedConflict = pkg.conflicts.find(
(conflict) => conflict.type === 'import_failed'
)
if (!importFailedConflict) {
return {
packageId: pkg.package_id,
packageName: pkg.package_name,
errorMessage: 'Unknown import error',
traceback: ''
}
}
return {
packageId: pkg.package_id,
packageName: pkg.package_name,
errorMessage:
importFailedConflict.current_value || 'Unknown import error',
traceback: importFailedConflict.required_value || ''
}
})
})
</script>

View File

@@ -0,0 +1,43 @@
<template>
<div class="flex w-full items-center justify-between px-3 pb-4">
<div class="flex w-full items-start justify-end gap-2 pr-1">
<Button variant="secondary" @click="handleCopyError">
{{ $t('importFailed.copyError') }}
</Button>
</div>
</div>
</template>
<script setup lang="ts">
import { computed } from 'vue'
import Button from '@/components/ui/button/Button.vue'
import { useCopyToClipboard } from '@/composables/useCopyToClipboard'
import type { ConflictDetectionResult } from '@/workbench/extensions/manager/types/conflictDetectionTypes'
const { conflictedPackages = [] } = defineProps<{
conflictedPackages?: ConflictDetectionResult[]
}>()
const { copyToClipboard } = useCopyToClipboard()
const formatErrorText = computed(() => {
const errorParts: string[] = []
conflictedPackages.forEach((pkg) => {
const importFailedConflict = pkg.conflicts.find(
(conflict) => conflict.type === 'import_failed'
)
if (importFailedConflict?.required_value) {
errorParts.push(importFailedConflict.required_value)
}
})
return errorParts.join('\n\n')
})
const handleCopyError = () => {
copyToClipboard(formatErrorText.value)
}
</script>

View File

@@ -0,0 +1,12 @@
<template>
<div class="flex w-full items-center justify-between p-4">
<div class="flex items-center gap-2">
<i class="icon-[lucide--triangle-alert] text-gold-600"></i>
<p class="m-0 text-sm">
{{ $t('importFailed.title') }}
</p>
</div>
</div>
</template>
<script setup lang="ts"></script>

View File

@@ -245,7 +245,7 @@ describe('NodeConflictDialogContent', () => {
await conflictsHeader.trigger('click') await conflictsHeader.trigger('click')
// Should be expanded now // Should be expanded now
const conflictItems = wrapper.findAll('.conflict-list-item') const conflictItems = wrapper.findAll('[aria-label*="Conflict:"]')
expect(conflictItems.length).toBeGreaterThan(0) expect(conflictItems.length).toBeGreaterThan(0)
}) })
@@ -324,7 +324,7 @@ describe('NodeConflictDialogContent', () => {
await conflictsHeader.trigger('click') await conflictsHeader.trigger('click')
// Should display conflict messages (excluding import_failed) // 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 expect(conflictItems).toHaveLength(3) // 2 from Package1 + 1 from Package2
}) })
@@ -338,7 +338,9 @@ describe('NodeConflictDialogContent', () => {
await importFailedHeader.trigger('click') await importFailedHeader.trigger('click')
// Should display only import failed package // 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).toHaveLength(1)
expect(importFailedItems[0].text()).toContain('Test Package 3') expect(importFailedItems[0].text()).toContain('Test Package 3')
}) })

View File

@@ -50,7 +50,8 @@
<div <div
v-for="(packageName, i) in importFailedConflicts" v-for="(packageName, i) in importFailedConflicts"
:key="i" :key="i"
class="conflict-list-item flex h-6 shrink-0 items-center justify-between px-4" :aria-label="`Import failed package: ${packageName}`"
class="flex min-h-6 shrink-0 hover:bg-node-component-surface-hovered items-center justify-between px-4 py-1"
> >
<span class="text-xs text-muted"> <span class="text-xs text-muted">
{{ packageName }} {{ packageName }}
@@ -98,7 +99,8 @@
<div <div
v-for="(conflict, i) in allConflictDetails" v-for="(conflict, i) in allConflictDetails"
:key="i" :key="i"
class="conflict-list-item flex h-6 shrink-0 items-center justify-between px-4" :aria-label="`Conflict: ${getConflictMessage(conflict, t)}`"
class="flex min-h-6 shrink-0 hover:bg-node-component-surface-hovered items-center justify-between px-4 py-1"
> >
<span class="text-xs text-muted">{{ <span class="text-xs text-muted">{{
getConflictMessage(conflict, t) getConflictMessage(conflict, t)
@@ -146,7 +148,7 @@
<div <div
v-for="conflictResult in conflictData" v-for="conflictResult in conflictData"
:key="conflictResult.package_id" :key="conflictResult.package_id"
class="conflict-list-item flex h-6 shrink-0 items-center justify-between px-4" class="flex min-h-6 shrink-0 hover:bg-node-component-surface-hovered items-center justify-between px-4 py-1"
> >
<span class="text-xs text-muted"> <span class="text-xs text-muted">
{{ conflictResult.package_name }} {{ conflictResult.package_name }}
@@ -236,8 +238,3 @@ const toggleExtensionsPanel = () => {
importFailedExpanded.value = false importFailedExpanded.value = false
} }
</script> </script>
<style scoped>
.conflict-list-item:hover {
background-color: rgb(0 122 255 / 0.2);
}
</style>

View File

@@ -2,7 +2,7 @@
<div class="flex h-12 w-full items-center justify-between pl-6"> <div class="flex h-12 w-full items-center justify-between pl-6">
<div class="flex items-center gap-2"> <div class="flex items-center gap-2">
<!-- Warning Icon --> <!-- Warning Icon -->
<i class="pi pi-exclamation-triangle text-lg"></i> <i class="icon-[lucide--triangle-alert] text-gold-600"></i>
<!-- Title --> <!-- Title -->
<p class="text-base font-bold"> <p class="text-base font-bold">
{{ $t('manager.conflicts.title') }} {{ $t('manager.conflicts.title') }}

View File

@@ -41,6 +41,8 @@ import { useComfyManagerStore } from '@/workbench/extensions/manager/stores/comf
import { useConflictDetectionStore } from '@/workbench/extensions/manager/stores/conflictDetectionStore' import { useConflictDetectionStore } from '@/workbench/extensions/manager/stores/conflictDetectionStore'
import type { components as ManagerComponents } from '@/workbench/extensions/manager/types/generatedManagerTypes' import type { components as ManagerComponents } from '@/workbench/extensions/manager/types/generatedManagerTypes'
import { useImportFailedDetection } from '../../../composables/useImportFailedDetection'
const TOGGLE_DEBOUNCE_MS = 256 const TOGGLE_DEBOUNCE_MS = 256
const { nodePack } = defineProps<{ const { nodePack } = defineProps<{
@@ -53,6 +55,7 @@ const { isPackEnabled, enablePack, disablePack, installedPacks } =
const { getConflictsForPackageByID } = useConflictDetectionStore() const { getConflictsForPackageByID } = useConflictDetectionStore()
const { showNodeConflictDialog } = useDialogService() const { showNodeConflictDialog } = useDialogService()
const { acknowledgmentState, markConflictsAsSeen } = useConflictAcknowledgment() const { acknowledgmentState, markConflictsAsSeen } = useConflictAcknowledgment()
const { showImportFailedDialog } = useImportFailedDetection(nodePack.id || '')
const isLoading = ref(false) const isLoading = ref(false)
@@ -81,23 +84,36 @@ const canToggleDirectly = computed(() => {
const showConflictModal = (skipModalDismissed: boolean) => { const showConflictModal = (skipModalDismissed: boolean) => {
let modal_dismissed = acknowledgmentState.value.modal_dismissed let modal_dismissed = acknowledgmentState.value.modal_dismissed
if (skipModalDismissed) modal_dismissed = false if (skipModalDismissed) modal_dismissed = false
if (packageConflict.value && !modal_dismissed) { if (packageConflict.value && !modal_dismissed) {
showNodeConflictDialog({ // Check if there's an import failed conflict first
conflictedPackages: [packageConflict.value], const hasImportFailed = packageConflict.value.conflicts.some(
buttonText: !isEnabled.value (conflict) => conflict.type === 'import_failed'
? t('manager.conflicts.enableAnyway') )
: t('manager.conflicts.understood'), if (hasImportFailed) {
onButtonClick: async () => { // Show import failed dialog instead of general conflict dialog
if (!isEnabled.value) { showImportFailedDialog(() => {
await handleEnable() 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()
}
}
})
} }
} }

View File

@@ -1,43 +1,37 @@
<template> <template>
<div class="flex flex-col gap-3"> <div class="flex flex-col gap-3">
<button
v-if="importFailedInfo"
class="inline-flex cursor-pointer items-center justify-end gap-1 border-none bg-transparent outline-none"
@click="showImportFailedDialog"
>
<i class="pi pi-code text-base"></i>
<span class="text-sm text-base-foreground">{{
t('serverStart.openLogs')
}}</span>
</button>
<div <div
v-for="(conflict, index) in conflictResult?.conflicts || []" v-for="(conflict, index) in conflictResult?.conflicts || []"
:key="index" :key="index"
class="rounded-md bg-yellow-800/20 p-3" class="rounded-md bg-secondary-background/60 px-2 py-1"
> >
<div class="flex items-center justify-between"> <!-- Import failed conflicts show detailed error message -->
<div class="flex-1 text-sm break-words"> <template v-if="conflict.type === 'import_failed'">
<div
v-if="conflict.required_value"
class="max-h-64 overflow-x-hidden scrollbar-custom overflow-y-auto rounded px-2"
>
<p class="text-xs text-muted-foreground break-all font-mono">
{{ conflict.required_value }}
</p>
</div>
</template>
<!-- Other conflict types use standard message -->
<template v-else>
<div class="text-sm break-words">
{{ getConflictMessage(conflict, $t) }} {{ getConflictMessage(conflict, $t) }}
</div> </div>
</div> </template>
</div> </div>
</div> </div>
</template> </template>
<script setup lang="ts"> <script setup lang="ts">
import { computed } from 'vue'
import { t } from '@/i18n'
import type { components } from '@/types/comfyRegistryTypes'
import { useImportFailedDetection } from '@/workbench/extensions/manager/composables/useImportFailedDetection'
import type { ConflictDetectionResult } from '@/workbench/extensions/manager/types/conflictDetectionTypes' import type { ConflictDetectionResult } from '@/workbench/extensions/manager/types/conflictDetectionTypes'
import { getConflictMessage } from '@/workbench/extensions/manager/utils/conflictMessageUtil' import { getConflictMessage } from '@/workbench/extensions/manager/utils/conflictMessageUtil'
const { nodePack, conflictResult } = defineProps<{ const { conflictResult } = defineProps<{
nodePack: components['schemas']['Node']
conflictResult: ConflictDetectionResult | null | undefined conflictResult: ConflictDetectionResult | null | undefined
}>() }>()
const packageId = computed(() => nodePack?.id || '')
const { importFailedInfo, showImportFailedDialog } =
useImportFailedDetection(packageId)
</script> </script>

View File

@@ -410,7 +410,7 @@ describe('useConflictDetection', () => {
mockComfyManagerService.getImportFailInfoBulk mockComfyManagerService.getImportFailInfoBulk
).mockResolvedValue({ ).mockResolvedValue({
'fail-pack': { 'fail-pack': {
msg: 'Import error', error: 'Import error',
name: 'fail-pack', name: 'fail-pack',
path: '/path/to/pack' path: '/path/to/pack'
} as any // The actual API returns different structure than types } as any // The actual API returns different structure than types
@@ -428,7 +428,7 @@ describe('useConflictDetection', () => {
// Import failure should match the actual implementation // Import failure should match the actual implementation
expect(result.results[0].conflicts).toContainEqual({ expect(result.results[0].conflicts).toContainEqual({
type: 'import_failed', type: 'import_failed',
current_value: 'installed', current_value: 'Import error',
required_value: 'Import error' required_value: 'Import error'
}) })
}) })

View File

@@ -389,7 +389,10 @@ export function useConflictDetection() {
* @returns Array of conflict detection results for failed imports * @returns Array of conflict detection results for failed imports
*/ */
function detectImportFailConflicts( function detectImportFailConflicts(
importFailInfo: Record<string, { msg: string; name: string; path: string }> importFailInfo: Record<
string,
{ error?: string; traceback?: string } | null
>
): ConflictDetectionResult[] { ): ConflictDetectionResult[] {
const results: ConflictDetectionResult[] = [] const results: ConflictDetectionResult[] = []
if (!importFailInfo || typeof importFailInfo !== 'object') { if (!importFailInfo || typeof importFailInfo !== 'object') {
@@ -400,8 +403,11 @@ export function useConflictDetection() {
for (const [packageId, failureInfo] of Object.entries(importFailInfo)) { for (const [packageId, failureInfo] of Object.entries(importFailInfo)) {
if (failureInfo && typeof failureInfo === 'object') { if (failureInfo && typeof failureInfo === 'object') {
// Extract error information from Manager API response // Extract error information from Manager API response
const errorMsg = failureInfo.msg || 'Unknown import error' const errorMsg = failureInfo.error || 'Unknown import error'
const modulePath = failureInfo.path || '' const traceback = failureInfo.traceback || ''
// Combine error and traceback for display
const fullErrorInfo = traceback || errorMsg
results.push({ results.push({
package_id: packageId, package_id: packageId,
@@ -410,8 +416,8 @@ export function useConflictDetection() {
conflicts: [ conflicts: [
{ {
type: 'import_failed', type: 'import_failed',
current_value: 'installed', current_value: errorMsg,
required_value: failureInfo.msg required_value: fullErrorInfo
} }
], ],
is_compatible: false is_compatible: false
@@ -420,8 +426,8 @@ export function useConflictDetection() {
console.warn( console.warn(
`[ConflictDetection] Python import failure detected for ${packageId}:`, `[ConflictDetection] Python import failure detected for ${packageId}:`,
{ {
path: modulePath, error: errorMsg,
error: errorMsg hasTraceback: !!traceback
} }
) )
} }

View File

@@ -44,7 +44,8 @@ describe('useImportFailedDetection', () => {
> >
mockDialogService = { mockDialogService = {
showErrorDialog: vi.fn() showErrorDialog: vi.fn(),
showImportFailedNodeDialog: vi.fn()
} as unknown as ReturnType<typeof dialogService.useDialogService> } as unknown as ReturnType<typeof dialogService.useDialogService>
vi.mocked(comfyManagerStore.useComfyManagerStore).mockReturnValue( vi.mocked(comfyManagerStore.useComfyManagerStore).mockReturnValue(
@@ -226,13 +227,22 @@ describe('useImportFailedDetection', () => {
showImportFailedDialog() showImportFailedDialog()
expect(mockDialogService.showErrorDialog).toHaveBeenCalledWith( expect(mockDialogService.showImportFailedNodeDialog).toHaveBeenCalledWith({
expect.any(Error), conflictedPackages: expect.arrayContaining([
{ expect.objectContaining({
title: 'manager.failedToInstall', package_id: 'test-package',
reportType: 'importFailedError' package_name: 'Test Package',
conflicts: expect.arrayContaining([
expect.objectContaining({
type: 'import_failed'
})
])
})
]),
dialogComponentProps: {
onClose: undefined
} }
) })
}) })
it('should handle null packageId', () => { it('should handle null packageId', () => {

View File

@@ -1,11 +1,13 @@
import { computed, unref } from 'vue' import { computed, unref } from 'vue'
import type { ComputedRef } from 'vue' import type { ComputedRef } from 'vue'
import { useI18n } from 'vue-i18n'
import { useDialogService } from '@/services/dialogService' import { useDialogService } from '@/services/dialogService'
import { useComfyManagerStore } from '@/workbench/extensions/manager/stores/comfyManagerStore' import { useComfyManagerStore } from '@/workbench/extensions/manager/stores/comfyManagerStore'
import { useConflictDetectionStore } from '@/workbench/extensions/manager/stores/conflictDetectionStore' 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 * Extracting import failed conflicts from conflict list
@@ -24,22 +26,18 @@ function extractImportFailedConflicts(conflicts?: ConflictDetail[] | null) {
* Creating import failed dialog * Creating import failed dialog
*/ */
function createImportFailedDialog() { function createImportFailedDialog() {
const { t } = useI18n() const { showImportFailedNodeDialog } = useDialogService()
const { showErrorDialog } = useDialogService()
return (importFailedInfo: ConflictDetail[] | null) => { return (
if (importFailedInfo) { conflictedPackages: ConflictDetectionResult[] | null,
const errorMessage = onClose?: () => void
importFailedInfo ) => {
.map((conflict) => conflict.required_value) if (conflictedPackages && conflictedPackages.length > 0) {
.filter(Boolean) showImportFailedNodeDialog({
.join('\n') || t('manager.importFailedGenericError') conflictedPackages,
dialogComponentProps: {
const error = new Error(errorMessage) onClose
}
showErrorDialog(error, {
title: t('manager.failedToInstall'),
reportType: 'importFailedError'
}) })
} }
} }
@@ -74,13 +72,16 @@ export function useImportFailedDetection(
return importFailedInfo.value !== null return importFailedInfo.value !== null
}) })
const showImportFailedDialog = createImportFailedDialog() const openDialog = createImportFailedDialog()
return { return {
importFailedInfo, importFailedInfo,
importFailed, importFailed,
showImportFailedDialog: () => showImportFailedDialog: (onClose?: () => void) => {
showImportFailedDialog(importFailedInfo.value), if (conflicts.value) {
openDialog([conflicts.value], onClose)
}
},
isInstalled isInstalled
} }
} }