Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 101 additions & 6 deletions src/platform/remote/comfyui/jobs/fetchJobs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ describe('fetchJobs', () => {
const result = await fetchHistory(mockFetch)

expect(mockFetch).toHaveBeenCalledWith(
'/jobs?status=completed,failed,cancelled&limit=200&offset=0'
'/jobs?status=completed,failed,cancelled&limit=200&offset=0',
{ signal: expect.any(AbortSignal) }
)
expect(result).toHaveLength(2)
expect(result[0].id).toBe('job1')
Expand Down Expand Up @@ -112,7 +113,8 @@ describe('fetchJobs', () => {
const result = await fetchHistory(mockFetch, 200, 5)

expect(mockFetch).toHaveBeenCalledWith(
'/jobs?status=completed,failed,cancelled&limit=200&offset=5'
'/jobs?status=completed,failed,cancelled&limit=200&offset=5',
{ signal: expect.any(AbortSignal) }
)
// Priority base is total - offset = 10 - 5 = 5
expect(result[0].priority).toBe(5) // (total - offset) - 0
Expand All @@ -135,15 +137,22 @@ describe('fetchJobs', () => {
expect(result[0].priority).toBe(999)
})

it('returns empty array on error', async () => {
it('reports a network error and returns an empty array', async () => {
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
const mockFetch = vi.fn().mockRejectedValue(new Error('Network error'))

const result = await fetchHistory(mockFetch)

expect(result).toEqual([])
expect(errorSpy).toHaveBeenCalledWith(
'[Jobs API] Error fetching jobs:',
expect.any(Error)
)
errorSpy.mockRestore()
})

it('returns empty array on non-ok response', async () => {
it('reports a server error and returns an empty array', async () => {
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
const mockFetch = vi.fn().mockResolvedValue({
ok: false,
status: 500
Expand All @@ -152,6 +161,90 @@ describe('fetchJobs', () => {
const result = await fetchHistory(mockFetch)

expect(result).toEqual([])
expect(errorSpy).toHaveBeenCalledWith(
'[Jobs API] Failed to fetch jobs: 500'
)
errorSpy.mockRestore()
})

it('reports a malformed response body as an error', async () => {
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
const mockFetch = vi.fn().mockResolvedValue({
ok: true,
json: () => Promise.resolve({ jobs: 'not-an-array' })
})

const result = await fetchHistory(mockFetch)

expect(result).toEqual([])
expect(errorSpy).toHaveBeenCalledWith(
'[Jobs API] Error fetching jobs:',
expect.anything()
)
errorSpy.mockRestore()
})

it('warns instead of erroring on an unauthenticated response', async () => {
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
const mockFetch = vi.fn().mockResolvedValue({
ok: false,
status: 401
})

const result = await fetchHistory(mockFetch)

expect(result).toEqual([])
expect(warnSpy).toHaveBeenCalledWith(
'[Jobs API] Failed to fetch jobs: 401'
)
expect(errorSpy).not.toHaveBeenCalled()
errorSpy.mockRestore()
warnSpy.mockRestore()
})

it('does not report an aborted request', async () => {
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
const mockFetch = vi
.fn()
.mockRejectedValue(new DOMException('Aborted', 'AbortError'))

const result = await fetchHistory(mockFetch)

expect(result).toEqual([])
expect(errorSpy).not.toHaveBeenCalled()
errorSpy.mockRestore()
})

it('does not report a request cancelled by page teardown', async () => {
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
const mockFetch = vi.fn().mockImplementation(() => {
window.dispatchEvent(new Event('pagehide'))
return Promise.reject(new TypeError('Failed to fetch'))
})

const result = await fetchHistory(mockFetch)

expect(result).toEqual([])
expect(errorSpy).not.toHaveBeenCalled()
errorSpy.mockRestore()
})

it('still reports network errors after the page is restored', async () => {
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
window.dispatchEvent(new Event('pagehide'))
const mockFetch = vi
.fn()
.mockRejectedValue(new TypeError('Failed to fetch'))

const result = await fetchHistory(mockFetch)

expect(result).toEqual([])
expect(errorSpy).toHaveBeenCalledWith(
'[Jobs API] Error fetching jobs:',
expect.any(TypeError)
)
errorSpy.mockRestore()
})

it('parses batch containing text-only preview outputs', async () => {
Expand Down Expand Up @@ -208,7 +301,8 @@ describe('fetchJobs', () => {
const result = await fetchHistoryPage(mockFetch, 2, 5)

expect(mockFetch).toHaveBeenCalledWith(
'/jobs?status=completed,failed,cancelled&limit=2&offset=5'
'/jobs?status=completed,failed,cancelled&limit=2&offset=5',
{ signal: expect.any(AbortSignal) }
)
expect(result.jobs).toHaveLength(2)
expect(result.offset).toBe(5)
Expand Down Expand Up @@ -237,7 +331,8 @@ describe('fetchJobs', () => {
const result = await fetchQueue(mockFetch)

expect(mockFetch).toHaveBeenCalledWith(
'/jobs?status=in_progress,pending&limit=200&offset=0'
'/jobs?status=in_progress,pending&limit=200&offset=0',
{ signal: expect.any(AbortSignal) }
)
expect(result.Running).toHaveLength(1)
expect(result.Pending).toHaveLength(2)
Expand Down
43 changes: 28 additions & 15 deletions src/platform/remote/comfyui/jobs/fetchJobs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
import type { ComfyWorkflowJSON } from '@/platform/workflow/validation/schemas/workflowSchema'
import { validateComfyWorkflow } from '@/platform/workflow/validation/schemas/workflowSchema'
import type { JobId } from '@/schemas/apiSchema'
import { pageTeardownSignal } from '@/utils/pageTeardownUtil'
import { isAbortError } from '@/utils/typeGuardUtil'

import type {
JobDetail,
Expand All @@ -18,6 +20,8 @@ import type {
} from './jobTypes'
import { zJobDetail, zJobsListResponse, zWorkflowContainer } from './jobTypes'

type JobsApiFetcher = (url: string, options?: RequestInit) => Promise<Response>

interface FetchJobsRawResult {
jobs: RawJobListItem[]
total: number
Expand All @@ -39,24 +43,30 @@ export interface FetchHistoryPageResult {
* @internal
*/
async function fetchJobsRaw(
fetchApi: (url: string) => Promise<Response>,
fetchApi: JobsApiFetcher,
statuses: JobStatus[],
maxItems: number = 200,
offset: number = 0
): Promise<FetchJobsRawResult> {
const statusParam = statuses.join(',')
const url = `/jobs?status=${statusParam}&limit=${maxItems}&offset=${offset}`
const noJobs: FetchJobsRawResult = {
jobs: [],
total: 0,
offset,
limit: maxItems,
hasMore: false
}
const signal = pageTeardownSignal()
try {
const res = await fetchApi(url)
const res = await fetchApi(url, { signal })
Comment thread
mattmillerai marked this conversation as resolved.
if (!res.ok) {
console.error(`[Jobs API] Failed to fetch jobs: ${res.status}`)
return {
jobs: [],
total: 0,
offset,
limit: maxItems,
hasMore: false
}
const message = `[Jobs API] Failed to fetch jobs: ${res.status}`
// A poll that outran the session is a lifecycle state, not a fault: the
// request seam already waits for auth to resolve and re-mints once on 401.
if (res.status === 401) console.warn(message)
else console.error(message)
return noJobs
}
const data = zJobsListResponse.parse(await res.json())
return {
Expand All @@ -67,8 +77,11 @@ async function fetchJobsRaw(
hasMore: data.pagination.has_more
}
} catch (error) {
// `signal.aborted` rather than the error shape: a request cancelled while
// the page unloads can surface as a bare `TypeError: Failed to fetch`.
if (signal.aborted || isAbortError(error)) return noJobs
Comment thread
mattmillerai marked this conversation as resolved.
console.error('[Jobs API] Error fetching jobs:', error)
return { jobs: [], total: 0, offset, limit: maxItems, hasMore: false }
return noJobs
}
}

Expand All @@ -94,7 +107,7 @@ function assignPriority(
* Assigns synthetic priority starting from total (lower than queue jobs).
*/
export async function fetchHistory(
fetchApi: (url: string) => Promise<Response>,
fetchApi: JobsApiFetcher,
maxItems: number = 200,
offset: number = 0
): Promise<JobListItem[]> {
Expand All @@ -106,7 +119,7 @@ export async function fetchHistory(
* Fetches one page of history with server-provided pagination metadata.
*/
export async function fetchHistoryPage(
fetchApi: (url: string) => Promise<Response>,
fetchApi: JobsApiFetcher,
maxItems: number = 200,
offset: number = 0
): Promise<FetchHistoryPageResult> {
Expand All @@ -132,7 +145,7 @@ export async function fetchHistoryPage(
* Pending jobs get highest priority, then running jobs.
*/
export async function fetchQueue(
fetchApi: (url: string) => Promise<Response>
fetchApi: JobsApiFetcher
): Promise<{ Running: JobListItem[]; Pending: JobListItem[] }> {
const { jobs } = await fetchJobsRaw(
fetchApi,
Expand All @@ -159,7 +172,7 @@ export async function fetchQueue(
* Fetches full job details from /jobs/{job_id}
*/
export async function fetchJobDetail(
fetchApi: (url: string) => Promise<Response>,
fetchApi: JobsApiFetcher,
Comment thread
mattmillerai marked this conversation as resolved.
jobId: JobId
): Promise<JobDetail | undefined> {
try {
Expand Down
24 changes: 24 additions & 0 deletions src/utils/pageTeardownUtil.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/**
* A signal that aborts when the page goes away — navigation, tab close, or a
* mobile discard. Requests the browser cancels at teardown reject with a
* generic `TypeError: Failed to fetch`, indistinguishable from a real network
* fault; aborting them as the page hides lets a caller recognise the ordinary
* page exit instead.
*
* `pagehide` rather than `beforeunload`, which also fires for navigations the
* user then cancels — nothing should be aborted while the page keeps running.
* A hidden page can still come back from the back/forward cache, so the
* controller is replaced after each abort rather than latching.
*/
let controller = new AbortController()

function abortInFlightRequests() {
controller.abort()
controller = new AbortController()
Comment thread
mattmillerai marked this conversation as resolved.
Outdated
}

window.addEventListener('pagehide', abortInFlightRequests)
Comment thread
mattmillerai marked this conversation as resolved.
Outdated

export function pageTeardownSignal(): AbortSignal {
return controller.signal
}
Loading