Skip to content

Commit 6a73391

Browse files
authored
Improve Import Failed Error Messages (#7871)
1 parent a87d2cf commit 6a73391

15 files changed

+284
-88
lines changed

src/components/dialog/content/MissingNodesFooter.vue

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222

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

src/locales/en/main.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,10 @@
378378
"warningTooltip": "This package may have compatibility issues with your current environment"
379379
}
380380
},
381+
"importFailed": {
382+
"title": "Import Failed",
383+
"copyError": "Copy Error"
384+
},
381385
"issueReport": {
382386
"helpFix": "Help Fix This"
383387
},

src/services/dialogService.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ import ManagerProgressFooter from '@/workbench/extensions/manager/components/Man
3030
import ManagerProgressHeader from '@/workbench/extensions/manager/components/ManagerProgressHeader.vue'
3131
import ManagerDialogContent from '@/workbench/extensions/manager/components/manager/ManagerDialogContent.vue'
3232
import ManagerHeader from '@/workbench/extensions/manager/components/manager/ManagerHeader.vue'
33+
import ImportFailedNodeContent from '@/workbench/extensions/manager/components/manager/ImportFailedNodeContent.vue'
34+
import ImportFailedNodeFooter from '@/workbench/extensions/manager/components/manager/ImportFailedNodeFooter.vue'
35+
import ImportFailedNodeHeader from '@/workbench/extensions/manager/components/manager/ImportFailedNodeHeader.vue'
3336
import NodeConflictDialogContent from '@/workbench/extensions/manager/components/manager/NodeConflictDialogContent.vue'
3437
import NodeConflictFooter from '@/workbench/extensions/manager/components/manager/NodeConflictFooter.vue'
3538
import NodeConflictHeader from '@/workbench/extensions/manager/components/manager/NodeConflictHeader.vue'
@@ -482,6 +485,43 @@ export const useDialogService = () => {
482485
})
483486
}
484487

488+
function showImportFailedNodeDialog(
489+
options: {
490+
conflictedPackages?: ConflictDetectionResult[]
491+
dialogComponentProps?: DialogComponentProps
492+
} = {}
493+
) {
494+
const { dialogComponentProps, conflictedPackages } = options
495+
496+
return dialogStore.showDialog({
497+
key: 'global-import-failed',
498+
headerComponent: ImportFailedNodeHeader,
499+
footerComponent: ImportFailedNodeFooter,
500+
component: ImportFailedNodeContent,
501+
dialogComponentProps: {
502+
closable: true,
503+
pt: {
504+
root: { class: 'bg-base-background border-border-default' },
505+
header: { class: '!p-0 !m-0' },
506+
content: { class: '!p-0 overflow-y-hidden' },
507+
footer: { class: '!p-0' },
508+
pcCloseButton: {
509+
root: {
510+
class: '!w-7 !h-7 !border-none !outline-none !p-2 !m-1.5'
511+
}
512+
}
513+
},
514+
...dialogComponentProps
515+
},
516+
props: {
517+
conflictedPackages: conflictedPackages ?? []
518+
},
519+
footerProps: {
520+
conflictedPackages: conflictedPackages ?? []
521+
}
522+
})
523+
}
524+
485525
function showNodeConflictDialog(
486526
options: {
487527
showAfterWhatsNew?: boolean
@@ -561,6 +601,7 @@ export const useDialogService = () => {
561601
toggleManagerDialog,
562602
toggleManagerProgressDialog,
563603
showLayoutDialog,
604+
showImportFailedNodeDialog,
564605
showNodeConflictDialog
565606
}
566607
}
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
<template>
2+
<div class="flex w-[490px] flex-col border-t-1 border-border-default">
3+
<div class="flex h-full w-full flex-col gap-4 p-4">
4+
<!-- Error Details -->
5+
<div v-if="importFailedPackages.length > 0" class="flex flex-col gap-3">
6+
<div
7+
v-for="pkg in importFailedPackages"
8+
:key="pkg.packageId"
9+
class="flex flex-col gap-2 max-h-60 overflow-x-hidden overflow-y-auto scrollbar-custom"
10+
role="region"
11+
:aria-label="`Error traceback for ${pkg.packageId}`"
12+
tabindex="0"
13+
>
14+
<!-- Error Message -->
15+
<div
16+
v-if="pkg.traceback || pkg.errorMessage"
17+
class="text-xs p-4 rounded-md bg-secondary-background font-mono"
18+
>
19+
{{ pkg.traceback || pkg.errorMessage }}
20+
</div>
21+
</div>
22+
</div>
23+
</div>
24+
</div>
25+
</template>
26+
27+
<script setup lang="ts">
28+
import { computed } from 'vue'
29+
30+
import type { ConflictDetectionResult } from '@/workbench/extensions/manager/types/conflictDetectionTypes'
31+
32+
const { conflictedPackages } = defineProps<{
33+
conflictedPackages: ConflictDetectionResult[]
34+
}>()
35+
36+
interface ImportFailedPackage {
37+
packageId: string
38+
packageName: string
39+
errorMessage: string
40+
traceback: string
41+
}
42+
43+
const importFailedPackages = computed((): ImportFailedPackage[] => {
44+
return conflictedPackages
45+
.filter((pkg) =>
46+
pkg.conflicts.some((conflict) => conflict.type === 'import_failed')
47+
)
48+
.map((pkg) => {
49+
const importFailedConflict = pkg.conflicts.find(
50+
(conflict) => conflict.type === 'import_failed'
51+
)
52+
if (!importFailedConflict) {
53+
return {
54+
packageId: pkg.package_id,
55+
packageName: pkg.package_name,
56+
errorMessage: 'Unknown import error',
57+
traceback: ''
58+
}
59+
}
60+
61+
return {
62+
packageId: pkg.package_id,
63+
packageName: pkg.package_name,
64+
errorMessage:
65+
importFailedConflict.current_value || 'Unknown import error',
66+
traceback: importFailedConflict.required_value || ''
67+
}
68+
})
69+
})
70+
</script>
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
<template>
2+
<div class="flex w-full items-center justify-between px-3 pb-4">
3+
<div class="flex w-full items-start justify-end gap-2 pr-1">
4+
<Button variant="secondary" @click="handleCopyError">
5+
{{ $t('importFailed.copyError') }}
6+
</Button>
7+
</div>
8+
</div>
9+
</template>
10+
11+
<script setup lang="ts">
12+
import { computed } from 'vue'
13+
14+
import Button from '@/components/ui/button/Button.vue'
15+
import { useCopyToClipboard } from '@/composables/useCopyToClipboard'
16+
import type { ConflictDetectionResult } from '@/workbench/extensions/manager/types/conflictDetectionTypes'
17+
18+
const { conflictedPackages = [] } = defineProps<{
19+
conflictedPackages?: ConflictDetectionResult[]
20+
}>()
21+
22+
const { copyToClipboard } = useCopyToClipboard()
23+
24+
const formatErrorText = computed(() => {
25+
const errorParts: string[] = []
26+
27+
conflictedPackages.forEach((pkg) => {
28+
const importFailedConflict = pkg.conflicts.find(
29+
(conflict) => conflict.type === 'import_failed'
30+
)
31+
32+
if (importFailedConflict?.required_value) {
33+
errorParts.push(importFailedConflict.required_value)
34+
}
35+
})
36+
37+
return errorParts.join('\n\n')
38+
})
39+
40+
const handleCopyError = () => {
41+
copyToClipboard(formatErrorText.value)
42+
}
43+
</script>
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<template>
2+
<div class="flex w-full items-center justify-between p-4">
3+
<div class="flex items-center gap-2">
4+
<i class="icon-[lucide--triangle-alert] text-gold-600"></i>
5+
<p class="m-0 text-sm">
6+
{{ $t('importFailed.title') }}
7+
</p>
8+
</div>
9+
</div>
10+
</template>
11+
12+
<script setup lang="ts"></script>

src/workbench/extensions/manager/components/manager/NodeConflictDialogContent.test.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@ describe('NodeConflictDialogContent', () => {
245245
await conflictsHeader.trigger('click')
246246

247247
// Should be expanded now
248-
const conflictItems = wrapper.findAll('.conflict-list-item')
248+
const conflictItems = wrapper.findAll('[aria-label*="Conflict:"]')
249249
expect(conflictItems.length).toBeGreaterThan(0)
250250
})
251251

@@ -324,7 +324,7 @@ describe('NodeConflictDialogContent', () => {
324324
await conflictsHeader.trigger('click')
325325

326326
// Should display conflict messages (excluding import_failed)
327-
const conflictItems = wrapper.findAll('.conflict-list-item')
327+
const conflictItems = wrapper.findAll('[aria-label*="Conflict:"]')
328328
expect(conflictItems).toHaveLength(3) // 2 from Package1 + 1 from Package2
329329
})
330330

@@ -338,7 +338,9 @@ describe('NodeConflictDialogContent', () => {
338338
await importFailedHeader.trigger('click')
339339

340340
// Should display only import failed package
341-
const importFailedItems = wrapper.findAll('.conflict-list-item')
341+
const importFailedItems = wrapper.findAll(
342+
'[aria-label*="Import failed package:"]'
343+
)
342344
expect(importFailedItems).toHaveLength(1)
343345
expect(importFailedItems[0].text()).toContain('Test Package 3')
344346
})

src/workbench/extensions/manager/components/manager/NodeConflictDialogContent.vue

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,8 @@
5050
<div
5151
v-for="(packageName, i) in importFailedConflicts"
5252
:key="i"
53-
class="conflict-list-item flex h-6 shrink-0 items-center justify-between px-4"
53+
:aria-label="`Import failed package: ${packageName}`"
54+
class="flex min-h-6 shrink-0 hover:bg-node-component-surface-hovered items-center justify-between px-4 py-1"
5455
>
5556
<span class="text-xs text-muted">
5657
{{ packageName }}
@@ -98,7 +99,8 @@
9899
<div
99100
v-for="(conflict, i) in allConflictDetails"
100101
:key="i"
101-
class="conflict-list-item flex h-6 shrink-0 items-center justify-between px-4"
102+
:aria-label="`Conflict: ${getConflictMessage(conflict, t)}`"
103+
class="flex min-h-6 shrink-0 hover:bg-node-component-surface-hovered items-center justify-between px-4 py-1"
102104
>
103105
<span class="text-xs text-muted">{{
104106
getConflictMessage(conflict, t)
@@ -146,7 +148,7 @@
146148
<div
147149
v-for="conflictResult in conflictData"
148150
:key="conflictResult.package_id"
149-
class="conflict-list-item flex h-6 shrink-0 items-center justify-between px-4"
151+
class="flex min-h-6 shrink-0 hover:bg-node-component-surface-hovered items-center justify-between px-4 py-1"
150152
>
151153
<span class="text-xs text-muted">
152154
{{ conflictResult.package_name }}
@@ -236,8 +238,3 @@ const toggleExtensionsPanel = () => {
236238
importFailedExpanded.value = false
237239
}
238240
</script>
239-
<style scoped>
240-
.conflict-list-item:hover {
241-
background-color: rgb(0 122 255 / 0.2);
242-
}
243-
</style>

src/workbench/extensions/manager/components/manager/NodeConflictHeader.vue

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
<div class="flex h-12 w-full items-center justify-between pl-6">
33
<div class="flex items-center gap-2">
44
<!-- Warning Icon -->
5-
<i class="pi pi-exclamation-triangle text-lg"></i>
5+
<i class="icon-[lucide--triangle-alert] text-gold-600"></i>
66
<!-- Title -->
77
<p class="text-base font-bold">
88
{{ $t('manager.conflicts.title') }}

src/workbench/extensions/manager/components/manager/button/PackEnableToggle.vue

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ import { useComfyManagerStore } from '@/workbench/extensions/manager/stores/comf
4141
import { useConflictDetectionStore } from '@/workbench/extensions/manager/stores/conflictDetectionStore'
4242
import type { components as ManagerComponents } from '@/workbench/extensions/manager/types/generatedManagerTypes'
4343
44+
import { useImportFailedDetection } from '../../../composables/useImportFailedDetection'
45+
4446
const TOGGLE_DEBOUNCE_MS = 256
4547
4648
const { nodePack } = defineProps<{
@@ -53,6 +55,7 @@ const { isPackEnabled, enablePack, disablePack, installedPacks } =
5355
const { getConflictsForPackageByID } = useConflictDetectionStore()
5456
const { showNodeConflictDialog } = useDialogService()
5557
const { acknowledgmentState, markConflictsAsSeen } = useConflictAcknowledgment()
58+
const { showImportFailedDialog } = useImportFailedDetection(nodePack.id || '')
5659
5760
const isLoading = ref(false)
5861
@@ -81,23 +84,36 @@ const canToggleDirectly = computed(() => {
8184
const showConflictModal = (skipModalDismissed: boolean) => {
8285
let modal_dismissed = acknowledgmentState.value.modal_dismissed
8386
if (skipModalDismissed) modal_dismissed = false
87+
8488
if (packageConflict.value && !modal_dismissed) {
85-
showNodeConflictDialog({
86-
conflictedPackages: [packageConflict.value],
87-
buttonText: !isEnabled.value
88-
? t('manager.conflicts.enableAnyway')
89-
: t('manager.conflicts.understood'),
90-
onButtonClick: async () => {
91-
if (!isEnabled.value) {
92-
await handleEnable()
93-
}
94-
},
95-
dialogComponentProps: {
96-
onClose: () => {
97-
markConflictsAsSeen()
89+
// Check if there's an import failed conflict first
90+
const hasImportFailed = packageConflict.value.conflicts.some(
91+
(conflict) => conflict.type === 'import_failed'
92+
)
93+
if (hasImportFailed) {
94+
// Show import failed dialog instead of general conflict dialog
95+
showImportFailedDialog(() => {
96+
markConflictsAsSeen()
97+
})
98+
} else {
99+
// Show general conflict dialog for other types of conflicts
100+
showNodeConflictDialog({
101+
conflictedPackages: [packageConflict.value],
102+
buttonText: !isEnabled.value
103+
? t('manager.conflicts.enableAnyway')
104+
: t('manager.conflicts.understood'),
105+
onButtonClick: async () => {
106+
if (!isEnabled.value) {
107+
await handleEnable()
108+
}
109+
},
110+
dialogComponentProps: {
111+
onClose: () => {
112+
markConflictsAsSeen()
113+
}
98114
}
99-
}
100-
})
115+
})
116+
}
101117
}
102118
}
103119

0 commit comments

Comments
 (0)