mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-06-10 16:29:26 +00:00
Compare commits
2 Commits
v1.46.12
...
fix/manage
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
c7ccafc75b | ||
|
|
d349677767 |
@@ -453,6 +453,7 @@
|
||||
"totalNodes": "Total Nodes",
|
||||
"discoverCommunityContent": "Discover community-made Node Packs, Extensions, and more...",
|
||||
"errorConnecting": "Error connecting to the Comfy Node Registry.",
|
||||
"retry": "Try Again",
|
||||
"noResultsFound": "No results found matching your search.",
|
||||
"tryDifferentSearch": "Please try a different search query.",
|
||||
"emptyState": {
|
||||
|
||||
@@ -7,8 +7,13 @@ import { isAbortError } from '@/utils/typeGuardUtil'
|
||||
|
||||
const API_BASE_URL = 'https://api.comfy.org'
|
||||
|
||||
// Without a timeout a hung socket (e.g. no internet, captive portal) never
|
||||
// rejects, leaving callers stuck in their loading state indefinitely.
|
||||
const REQUEST_TIMEOUT_MS = 10_000
|
||||
|
||||
const registryApiClient = axios.create({
|
||||
baseURL: API_BASE_URL,
|
||||
timeout: REQUEST_TIMEOUT_MS,
|
||||
headers: {
|
||||
'Content-Type': 'application/json'
|
||||
},
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
<script setup lang="ts">
|
||||
import { cn } from '@comfyorg/tailwind-utils'
|
||||
import { useScroll, whenever } from '@vueuse/core'
|
||||
import Panel from 'primevue/panel'
|
||||
import TabMenu from 'primevue/tabmenu'
|
||||
@@ -40,10 +41,7 @@ const isInProgress = computed(
|
||||
() => comfyManagerStore.isProcessingTasks || isRestarting.value
|
||||
)
|
||||
|
||||
const isTaskInProgress = (index: number) => {
|
||||
const log = focusedLogs.value[index]
|
||||
if (!log) return false
|
||||
|
||||
const isTaskInProgress = (taskId: string) => {
|
||||
const taskQueue = comfyManagerStore.taskQueue
|
||||
if (!taskQueue) return false
|
||||
|
||||
@@ -52,7 +50,13 @@ const isTaskInProgress = (index: number) => {
|
||||
...(taskQueue.pending_queue || [])
|
||||
]
|
||||
|
||||
return allQueueTasks.some((task) => task.ui_id === log.taskId)
|
||||
return allQueueTasks.some((task) => task.ui_id === taskId)
|
||||
}
|
||||
|
||||
function taskStatusLabel(taskId: string): string {
|
||||
if (isTaskInProgress(taskId)) return t('g.inProgress')
|
||||
if (comfyManagerStore.isTaskFailed(taskId)) return t('manager.failed')
|
||||
return t('g.completedWithCheckmark')
|
||||
}
|
||||
|
||||
const completedTasksCount = computed(() => {
|
||||
@@ -190,12 +194,16 @@ onBeforeUnmount(() => {
|
||||
<div class="flex w-full items-center justify-between py-2">
|
||||
<div class="flex flex-col text-sm/normal font-medium">
|
||||
<span>{{ log.taskName }}</span>
|
||||
<span class="text-muted">
|
||||
{{
|
||||
isTaskInProgress(index)
|
||||
? t('g.inProgress')
|
||||
: t('g.completedWithCheckmark')
|
||||
}}
|
||||
<span
|
||||
:class="
|
||||
cn(
|
||||
'text-muted',
|
||||
comfyManagerStore.isTaskFailed(log.taskId) &&
|
||||
'text-danger'
|
||||
)
|
||||
"
|
||||
>
|
||||
{{ taskStatusLabel(log.taskId) }}
|
||||
</span>
|
||||
</div>
|
||||
</div>
|
||||
@@ -229,6 +237,17 @@ onBeforeUnmount(() => {
|
||||
@scroll="handleScroll"
|
||||
>
|
||||
<div class="h-full">
|
||||
<div
|
||||
v-for="(
|
||||
errorMessage, errorIndex
|
||||
) in comfyManagerStore.getTaskErrorMessages(log.taskId)"
|
||||
:key="`error-${errorIndex}`"
|
||||
class="text-danger"
|
||||
>
|
||||
<pre class="wrap-break-word whitespace-pre-wrap">{{
|
||||
errorMessage
|
||||
}}</pre>
|
||||
</div>
|
||||
<div
|
||||
v-for="(logLine, logIndex) in log.logs"
|
||||
:key="logIndex"
|
||||
|
||||
@@ -114,6 +114,8 @@
|
||||
v-else-if="displayPacks.length === 0"
|
||||
:title="emptyStateTitle"
|
||||
:message="emptyStateMessage"
|
||||
:button-label="searchError ? $t('manager.retry') : undefined"
|
||||
@action="() => void retrySearch()"
|
||||
/>
|
||||
<div v-else class="size-full" @click="handleGridContainerClick">
|
||||
<VirtualGrid
|
||||
@@ -344,6 +346,8 @@ const {
|
||||
searchQuery,
|
||||
pageNumber,
|
||||
isLoading: isSearchLoading,
|
||||
error: searchError,
|
||||
retry: retrySearch,
|
||||
searchResults,
|
||||
searchMode,
|
||||
sortField,
|
||||
@@ -434,9 +438,15 @@ const isManagerErrorRelevant = computed(() => {
|
||||
)
|
||||
})
|
||||
|
||||
// The registry search failing (e.g. offline) is also a connection error worth
|
||||
// surfacing, and unlike the manager-store error it can be retried in place.
|
||||
const hasConnectionError = computed(
|
||||
() => isManagerErrorRelevant.value || !!searchError.value
|
||||
)
|
||||
|
||||
// Empty state messages based on current tab and search state
|
||||
const emptyStateTitle = computed(() => {
|
||||
if (isManagerErrorRelevant.value) return t('manager.errorConnecting')
|
||||
if (hasConnectionError.value) return t('manager.errorConnecting')
|
||||
if (searchQuery.value) return t('manager.noResultsFound')
|
||||
|
||||
const tabId = selectedTab.value?.id
|
||||
@@ -448,7 +458,7 @@ const emptyStateTitle = computed(() => {
|
||||
})
|
||||
|
||||
const emptyStateMessage = computed(() => {
|
||||
if (isManagerErrorRelevant.value) return t('manager.tryAgainLater')
|
||||
if (hasConnectionError.value) return t('manager.tryAgainLater')
|
||||
if (searchQuery.value) {
|
||||
const baseMessage = t('manager.tryDifferentSearch')
|
||||
if (isLegacyManagerSearch.value) {
|
||||
@@ -475,6 +485,9 @@ const onClickWarningLink = () => {
|
||||
}
|
||||
|
||||
const isLoading = computed(() => {
|
||||
// A failed search must not read as "still loading" -- otherwise the spinner
|
||||
// runs forever (e.g. offline) instead of showing the error placeholder.
|
||||
if (searchError.value) return false
|
||||
if (isSearchLoading.value) return searchResults.value.length === 0
|
||||
if (isTabLoading.value) return true
|
||||
return isInitialLoad.value
|
||||
|
||||
@@ -0,0 +1,61 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import { useRegistrySearchGateway } from '@/services/gateway/registrySearchGateway'
|
||||
import type { NodePackSearchProvider } from '@/types/searchServiceTypes'
|
||||
import { useRegistrySearch } from '@/workbench/extensions/manager/composables/useRegistrySearch'
|
||||
|
||||
vi.mock('@/services/gateway/registrySearchGateway')
|
||||
|
||||
function mockGateway(searchPacks: NodePackSearchProvider['searchPacks']) {
|
||||
vi.mocked(useRegistrySearchGateway).mockReturnValue({
|
||||
searchPacks,
|
||||
clearSearchCache: vi.fn(),
|
||||
getSortValue: vi.fn(),
|
||||
getSortableFields: vi.fn().mockReturnValue([])
|
||||
})
|
||||
}
|
||||
|
||||
describe('useRegistrySearch', () => {
|
||||
beforeEach(() => {
|
||||
// Suppress the immediate debounced search so each test drives the search
|
||||
// explicitly via retry(); pending timers stay queued and never fire.
|
||||
vi.useFakeTimers()
|
||||
vi.clearAllMocks()
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers()
|
||||
})
|
||||
|
||||
it('clears loading and records the error when the search fails', async () => {
|
||||
const searchPacks = vi
|
||||
.fn()
|
||||
.mockRejectedValue(new Error('All search providers failed'))
|
||||
mockGateway(searchPacks)
|
||||
|
||||
const { isLoading, error, retry } = useRegistrySearch()
|
||||
await retry()
|
||||
|
||||
// The bug: without try/finally, isLoading stayed true forever -> infinite spinner.
|
||||
expect(isLoading.value).toBe(false)
|
||||
expect(error.value).toBe('All search providers failed')
|
||||
})
|
||||
|
||||
it('recovers and clears the error on a successful retry', async () => {
|
||||
const searchPacks = vi.fn().mockRejectedValue(new Error('offline'))
|
||||
mockGateway(searchPacks)
|
||||
|
||||
const { error, searchResults, retry } = useRegistrySearch()
|
||||
await retry()
|
||||
expect(error.value).toBe('offline')
|
||||
|
||||
searchPacks.mockResolvedValue({
|
||||
nodePacks: [{ id: 'a', name: 'Pack A' }],
|
||||
querySuggestions: []
|
||||
})
|
||||
await retry()
|
||||
|
||||
expect(error.value).toBeNull()
|
||||
expect(searchResults.value).toHaveLength(1)
|
||||
})
|
||||
})
|
||||
@@ -33,6 +33,7 @@ export function useRegistrySearch(
|
||||
} = options
|
||||
|
||||
const isLoading = ref(false)
|
||||
const error = ref<string | null>(null)
|
||||
const sortField = ref<string>(initialSortField)
|
||||
const searchMode = ref<SearchMode>(initialSearchMode)
|
||||
const pageSize = ref(DEFAULT_PAGE_SIZE)
|
||||
@@ -52,43 +53,51 @@ export function useRegistrySearch(
|
||||
|
||||
const updateSearchResults = async (options: { append?: boolean }) => {
|
||||
isLoading.value = true
|
||||
error.value = null
|
||||
if (!options.append) {
|
||||
pageNumber.value = 0
|
||||
}
|
||||
const { nodePacks, querySuggestions } = await searchPacks(
|
||||
searchQuery.value,
|
||||
{
|
||||
pageSize: pageSize.value,
|
||||
pageNumber: pageNumber.value,
|
||||
restrictSearchableAttributes: searchAttributes.value
|
||||
}
|
||||
)
|
||||
|
||||
let sortedPacks = nodePacks
|
||||
|
||||
// Results are sorted by the default field to begin with -- so don't manually sort again
|
||||
if (sortField.value && sortField.value !== DEFAULT_SORT_FIELD) {
|
||||
// Get the sort direction from the provider's sortable fields
|
||||
const sortableFields = getSortableFields()
|
||||
const fieldConfig = sortableFields.find((f) => f.id === sortField.value)
|
||||
const direction = fieldConfig?.direction || 'desc'
|
||||
|
||||
sortedPacks = orderBy(
|
||||
nodePacks,
|
||||
[(pack) => getSortValue(pack, sortField.value)],
|
||||
[direction]
|
||||
try {
|
||||
const { nodePacks, querySuggestions } = await searchPacks(
|
||||
searchQuery.value,
|
||||
{
|
||||
pageSize: pageSize.value,
|
||||
pageNumber: pageNumber.value,
|
||||
restrictSearchableAttributes: searchAttributes.value
|
||||
}
|
||||
)
|
||||
}
|
||||
|
||||
if (options.append && searchResults.value?.length) {
|
||||
searchResults.value = searchResults.value.concat(sortedPacks)
|
||||
} else {
|
||||
searchResults.value = sortedPacks
|
||||
let sortedPacks = nodePacks
|
||||
|
||||
// Results are sorted by the default field to begin with -- so don't manually sort again
|
||||
if (sortField.value && sortField.value !== DEFAULT_SORT_FIELD) {
|
||||
// Get the sort direction from the provider's sortable fields
|
||||
const sortableFields = getSortableFields()
|
||||
const fieldConfig = sortableFields.find((f) => f.id === sortField.value)
|
||||
const direction = fieldConfig?.direction || 'desc'
|
||||
|
||||
sortedPacks = orderBy(
|
||||
nodePacks,
|
||||
[(pack) => getSortValue(pack, sortField.value)],
|
||||
[direction]
|
||||
)
|
||||
}
|
||||
|
||||
if (options.append && searchResults.value?.length) {
|
||||
searchResults.value = searchResults.value.concat(sortedPacks)
|
||||
} else {
|
||||
searchResults.value = sortedPacks
|
||||
}
|
||||
suggestions.value = querySuggestions
|
||||
} catch (e) {
|
||||
error.value = e instanceof Error ? e.message : String(e)
|
||||
} finally {
|
||||
isLoading.value = false
|
||||
}
|
||||
suggestions.value = querySuggestions
|
||||
isLoading.value = false
|
||||
}
|
||||
|
||||
const retry = () => updateSearchResults({ append: false })
|
||||
|
||||
const onQueryChange = () => void updateSearchResults({ append: false })
|
||||
const onPageChange = () => {
|
||||
if (pageNumber.value === 0) return
|
||||
@@ -108,6 +117,8 @@ export function useRegistrySearch(
|
||||
|
||||
return {
|
||||
isLoading,
|
||||
error,
|
||||
retry,
|
||||
pageNumber,
|
||||
pageSize,
|
||||
sortField,
|
||||
|
||||
@@ -0,0 +1,74 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { ref } from 'vue'
|
||||
|
||||
import { useComfyManagerService } from '@/workbench/extensions/manager/services/comfyManagerService'
|
||||
|
||||
const { mockClient } = vi.hoisted(() => ({
|
||||
mockClient: { get: vi.fn(), post: vi.fn() }
|
||||
}))
|
||||
|
||||
vi.mock('axios', () => ({
|
||||
default: {
|
||||
create: () => mockClient,
|
||||
isAxiosError: (e: unknown): boolean =>
|
||||
!!e &&
|
||||
typeof e === 'object' &&
|
||||
(e as { isAxiosError?: boolean }).isAxiosError === true
|
||||
}
|
||||
}))
|
||||
|
||||
vi.mock('@/scripts/api', () => ({
|
||||
api: {
|
||||
apiURL: (p: string) => p,
|
||||
clientId: 'test-client',
|
||||
initialClientId: 'test-client'
|
||||
}
|
||||
}))
|
||||
|
||||
vi.mock('@/workbench/extensions/manager/composables/useManagerState', () => ({
|
||||
useManagerState: () => ({ isNewManagerUI: ref(true) })
|
||||
}))
|
||||
|
||||
function axiosError(status: number, data?: { message: string }) {
|
||||
return { isAxiosError: true, response: { status, data } }
|
||||
}
|
||||
|
||||
describe('useComfyManagerService error messages', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
})
|
||||
|
||||
it('surfaces the backend security message on a 403 instead of the generic fallback', async () => {
|
||||
const backendMessage =
|
||||
"ERROR: To use this action, '--listen' must be set to a local IP and security_level must be 'normal-' or lower."
|
||||
mockClient.post.mockRejectedValue(
|
||||
axiosError(403, { message: backendMessage })
|
||||
)
|
||||
|
||||
const service = useComfyManagerService()
|
||||
await service.installPack({
|
||||
id: 'some-pack',
|
||||
version: '1.0.0',
|
||||
selected_version: '1.0.0',
|
||||
mode: 'remote',
|
||||
channel: 'default'
|
||||
})
|
||||
|
||||
expect(service.error.value).toBe(backendMessage)
|
||||
})
|
||||
|
||||
it('falls back to the generic security message when the 403 has no body', async () => {
|
||||
mockClient.post.mockRejectedValue(axiosError(403))
|
||||
|
||||
const service = useComfyManagerService()
|
||||
await service.installPack({
|
||||
id: 'some-pack',
|
||||
version: '1.0.0',
|
||||
selected_version: '1.0.0',
|
||||
mode: 'remote',
|
||||
channel: 'default'
|
||||
})
|
||||
|
||||
expect(service.error.value).toContain('security error has occurred')
|
||||
})
|
||||
})
|
||||
@@ -38,8 +38,13 @@ enum ManagerRoute {
|
||||
QUEUE_TASK = 'manager/queue/task'
|
||||
}
|
||||
|
||||
// Without a timeout a hung socket (e.g. no internet, captive portal) never
|
||||
// rejects, leaving callers stuck in their loading state indefinitely.
|
||||
const REQUEST_TIMEOUT_MS = 10_000
|
||||
|
||||
const managerApiClient = axios.create({
|
||||
baseURL: api.apiURL('/v2/'),
|
||||
timeout: REQUEST_TIMEOUT_MS,
|
||||
headers: {
|
||||
'Content-Type': 'application/json'
|
||||
}
|
||||
@@ -74,14 +79,18 @@ export const useComfyManagerService = () => {
|
||||
} else {
|
||||
const axiosError = err as AxiosError<{ message: string }>
|
||||
const status = axiosError.response?.status
|
||||
if (status && routeSpecificErrors?.[status]) {
|
||||
const backendMessage = axiosError.response?.data?.message
|
||||
// Prefer the backend's message: ComfyUI-Manager returns actionable,
|
||||
// security-aware text (e.g. which security_level/--listen is required)
|
||||
// that is far more useful than our generic per-status fallbacks.
|
||||
if (backendMessage) {
|
||||
message = backendMessage
|
||||
} else if (status && routeSpecificErrors?.[status]) {
|
||||
message = routeSpecificErrors[status]
|
||||
} else if (status === 404) {
|
||||
message = 'Could not connect to ComfyUI-Manager'
|
||||
} else {
|
||||
message =
|
||||
axiosError.response?.data?.message ??
|
||||
`${context} failed with status ${status}`
|
||||
message = `${context} failed with status ${status}`
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -409,6 +409,51 @@ describe('useComfyManagerStore', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('task failure surfacing', () => {
|
||||
type TaskHistoryItem = ManagerComponents['schemas']['TaskHistoryItem']
|
||||
|
||||
const historyItem = (
|
||||
uiId: string,
|
||||
statusStr: 'success' | 'error' | 'skip',
|
||||
messages: string[]
|
||||
): TaskHistoryItem => ({
|
||||
ui_id: uiId,
|
||||
client_id: 'client',
|
||||
kind: 'install',
|
||||
result: statusStr === 'success' ? 'success' : 'failed',
|
||||
timestamp: '2024-01-01T00:00:00Z',
|
||||
status: {
|
||||
status_str: statusStr,
|
||||
completed: statusStr === 'success',
|
||||
messages
|
||||
}
|
||||
})
|
||||
|
||||
it('flags an errored task as failed and surfaces its messages', async () => {
|
||||
const store = useComfyManagerStore()
|
||||
const reason =
|
||||
"ERROR: To use this action, '--listen' must be set and security_level must be 'normal-' or lower."
|
||||
|
||||
store.taskHistory = { 'task-1': historyItem('task-1', 'error', [reason]) }
|
||||
await nextTick()
|
||||
|
||||
expect(store.isTaskFailed('task-1')).toBe(true)
|
||||
expect(store.getTaskErrorMessages('task-1')).toEqual([reason])
|
||||
})
|
||||
|
||||
it('does not surface messages for a successful task', async () => {
|
||||
const store = useComfyManagerStore()
|
||||
|
||||
store.taskHistory = {
|
||||
'task-2': historyItem('task-2', 'success', ['Installed successfully'])
|
||||
}
|
||||
await nextTick()
|
||||
|
||||
expect(store.isTaskFailed('task-2')).toBe(false)
|
||||
expect(store.getTaskErrorMessages('task-2')).toEqual([])
|
||||
})
|
||||
})
|
||||
|
||||
describe('refreshInstalledList with pack ID normalization', () => {
|
||||
it('normalizes pack IDs by removing version suffixes', async () => {
|
||||
const mockPacks = {
|
||||
|
||||
@@ -115,6 +115,18 @@ export const useComfyManagerStore = defineStore('comfyManager', () => {
|
||||
{ deep: true }
|
||||
)
|
||||
|
||||
const isTaskFailed = (taskId: string): boolean =>
|
||||
failedTasksIds.value.includes(taskId)
|
||||
|
||||
// The actionable reason a task failed (e.g. the security_level/--listen
|
||||
// restriction ComfyUI-Manager reports on a blocked install) lives in task
|
||||
// history, not the streamed server logs -- which stay empty when the request
|
||||
// is rejected before the task ever runs. Surface it so the failure isn't silent.
|
||||
const getTaskErrorMessages = (taskId: string): string[] =>
|
||||
isTaskFailed(taskId)
|
||||
? (taskHistory.value[taskId]?.status?.messages ?? [])
|
||||
: []
|
||||
|
||||
const getPackId = (pack: ManagerPackInstalled) => pack.cnr_id || pack.aux_id
|
||||
|
||||
const isInstalledPackId = (packName: NodePackId | undefined): boolean =>
|
||||
@@ -384,6 +396,8 @@ export const useComfyManagerStore = defineStore('comfyManager', () => {
|
||||
failedTasksIds,
|
||||
succeededTasksLogs,
|
||||
failedTasksLogs,
|
||||
isTaskFailed,
|
||||
getTaskErrorMessages,
|
||||
managerQueue,
|
||||
|
||||
// Pack actions
|
||||
|
||||
Reference in New Issue
Block a user