mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-20 14:30:41 +00:00
fix: prevent cache poisoning on aborted requests in useCachedRequest
This commit is contained in:
@@ -239,4 +239,32 @@ describe('useCachedRequest', () => {
|
||||
await cachedRequest.call(123)
|
||||
expect(mockRequestFn).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('should not cache aborted requests', async () => {
|
||||
vi.unstubAllGlobals()
|
||||
|
||||
let callCount = 0
|
||||
const abortFn = vi.fn(async (_params: unknown, _signal?: AbortSignal) => {
|
||||
callCount++
|
||||
if (callCount === 1) {
|
||||
const error = new DOMException(
|
||||
'The operation was aborted.',
|
||||
'AbortError'
|
||||
)
|
||||
throw error
|
||||
}
|
||||
return { data: 'success' }
|
||||
})
|
||||
|
||||
const cachedRequest = useCachedRequest(abortFn)
|
||||
|
||||
// First call throws AbortError — should NOT be cached
|
||||
const result1 = await cachedRequest.call('key')
|
||||
expect(result1).toBeNull()
|
||||
|
||||
// Second call should retry (not use cached null)
|
||||
const result2 = await cachedRequest.call('key')
|
||||
expect(result2).toEqual({ data: 'success' })
|
||||
expect(abortFn).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -48,6 +48,10 @@ export function useCachedRequest<TParams, TResult>(
|
||||
|
||||
return result
|
||||
} catch (err) {
|
||||
// Don't cache aborted requests — they should be retried
|
||||
if (err instanceof DOMException && err.name === 'AbortError') {
|
||||
return null
|
||||
}
|
||||
// Set cache on error to prevent retrying bad requests
|
||||
cache.set(cacheKey, null)
|
||||
return null
|
||||
|
||||
@@ -11,7 +11,7 @@ export function useUngroupedAssets(
|
||||
assets: Ref<AssetItem[]>,
|
||||
groupByJob: Ref<boolean>
|
||||
) {
|
||||
const { call: cachedResolve, cancel } = useCachedRequest(
|
||||
const { call: cachedResolve } = useCachedRequest(
|
||||
(jobId: string, signal?: AbortSignal) => {
|
||||
const asset = assets.value.find((a) => {
|
||||
const m = getOutputAssetMetadata(a.user_metadata)
|
||||
@@ -29,11 +29,9 @@ export function useUngroupedAssets(
|
||||
const isResolving = ref(false)
|
||||
|
||||
const resolvedAssets = computedAsync(
|
||||
async (onCancel) => {
|
||||
async () => {
|
||||
if (groupByJob.value) return []
|
||||
|
||||
onCancel(() => cancel())
|
||||
|
||||
const entries = assets.value.map((asset) => ({
|
||||
asset,
|
||||
metadata: getOutputAssetMetadata(asset.user_metadata)
|
||||
@@ -41,7 +39,7 @@ export function useUngroupedAssets(
|
||||
|
||||
for (const { metadata } of entries) {
|
||||
if ((metadata?.outputCount ?? 1) > 1 && metadata?.jobId) {
|
||||
void cachedResolve(metadata.jobId)
|
||||
void cachedResolve(metadata.jobId).catch(() => {})
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -227,7 +227,9 @@ describe('fetchJobs', () => {
|
||||
|
||||
const result = await fetchJobDetail(mockFetch, 'job1')
|
||||
|
||||
expect(mockFetch).toHaveBeenCalledWith('/jobs/job1')
|
||||
expect(mockFetch).toHaveBeenCalledWith('/jobs/job1', {
|
||||
signal: undefined
|
||||
})
|
||||
expect(result?.id).toBe('job1')
|
||||
expect(result?.outputs).toBeDefined()
|
||||
})
|
||||
|
||||
@@ -51,7 +51,9 @@ describe('fetchJobDetail', () => {
|
||||
|
||||
await fetchJobDetail(mockFetchApi, 'test-job-id')
|
||||
|
||||
expect(mockFetchApi).toHaveBeenCalledWith('/jobs/test-job-id')
|
||||
expect(mockFetchApi).toHaveBeenCalledWith('/jobs/test-job-id', {
|
||||
signal: undefined
|
||||
})
|
||||
})
|
||||
|
||||
it('should return job detail with workflow and outputs', async () => {
|
||||
|
||||
@@ -323,7 +323,7 @@ describe('jobOutputCache', () => {
|
||||
const result = await getJobDetail(jobId)
|
||||
|
||||
expect(result).toEqual(mockDetail)
|
||||
expect(api.getJobDetail).toHaveBeenCalledWith(jobId)
|
||||
expect(api.getJobDetail).toHaveBeenCalledWith(jobId, undefined)
|
||||
})
|
||||
|
||||
it('returns cached job detail on subsequent calls', async () => {
|
||||
|
||||
Reference in New Issue
Block a user