Skip to content

Commit caaf056

Browse files
fix(knowledge): fail upload completion on dispatch errors
1 parent 8d58a11 commit caaf056

2 files changed

Lines changed: 132 additions & 19 deletions

File tree

apps/sim/lib/knowledge/application/upload-sessions.test.ts

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,9 +390,85 @@ describe('knowledge-document upload application lifecycle', () => {
390390
expect(result.value.created).toBe(false)
391391
expect(mocks.resolveBilling).not.toHaveBeenCalled()
392392
expect(mocks.createDocument).not.toHaveBeenCalled()
393+
expect(mocks.processQueue).not.toHaveBeenCalled()
393394
expect(mocks.recordAudit).not.toHaveBeenCalled()
394395
})
395396

397+
it('fails completion when document processing cannot be dispatched', async () => {
398+
const failure = new Error('queue unavailable')
399+
mocks.processQueue.mockRejectedValue(failure)
400+
mocks.completeUpload.mockImplementation(
401+
async (params: {
402+
session: UploadSessionRecord
403+
finalize: (session: UploadSessionRecord) => Promise<unknown>
404+
}) => params.finalize(params.session)
405+
)
406+
407+
await expect(
408+
completeKnowledgeDocumentUpload.execute({
409+
principal: PRINCIPAL,
410+
input: {
411+
knowledgeBaseId: 'knowledge-1',
412+
assertedWorkspaceId: 'workspace-1',
413+
uploadId: 'upload-1',
414+
uploadToken: 'token',
415+
source: 'api',
416+
},
417+
request: REQUEST,
418+
})
419+
).rejects.toMatchObject({
420+
name: 'KnowledgeDocumentProcessingDispatchError',
421+
message: 'Knowledge document processing dispatch failed',
422+
cause: failure,
423+
})
424+
expect(mocks.createDocument).toHaveBeenCalledTimes(1)
425+
expect(mocks.recordAudit).not.toHaveBeenCalled()
426+
})
427+
428+
it('retries a failed processing dispatch before completing a bound registration', async () => {
429+
const recoveringSession = {
430+
...SESSION,
431+
status: 'finalizing' as const,
432+
completedFileId: null,
433+
error: 'Knowledge document processing dispatch failed',
434+
}
435+
mocks.getUpload.mockResolvedValue(recoveringSession)
436+
mocks.findBound.mockResolvedValue({
437+
status: 'bound',
438+
document: { ...DOCUMENT, processingStatus: 'pending' },
439+
})
440+
mocks.completeUpload.mockImplementation(
441+
async (params: {
442+
session: UploadSessionRecord
443+
finalize: (session: UploadSessionRecord) => Promise<{
444+
value: { document: typeof DOCUMENT; created: boolean; knowledgeBaseName: string | null }
445+
}>
446+
}) => ({
447+
session: { ...params.session, status: 'completed' as const },
448+
value: (await params.finalize(params.session)).value,
449+
alreadyCompleted: true,
450+
})
451+
)
452+
453+
const result = await completeKnowledgeDocumentUpload.execute({
454+
principal: PRINCIPAL,
455+
input: {
456+
knowledgeBaseId: 'knowledge-1',
457+
assertedWorkspaceId: 'workspace-1',
458+
uploadId: 'upload-1',
459+
uploadToken: 'token',
460+
source: 'api',
461+
},
462+
request: REQUEST,
463+
})
464+
465+
expect(result.value.created).toBe(true)
466+
expect(mocks.resolveBilling).toHaveBeenCalledTimes(1)
467+
expect(mocks.processQueue).toHaveBeenCalledTimes(1)
468+
expect(mocks.createDocument).not.toHaveBeenCalled()
469+
expect(mocks.recordAudit).toHaveBeenCalledTimes(1)
470+
})
471+
396472
it('converges a finalization retry after durable bind without duplicate document or audit', async () => {
397473
const recoveringSession = {
398474
...SESSION,
@@ -454,6 +530,7 @@ describe('knowledge-document upload application lifecycle', () => {
454530
expect(recovered.value.created).toBe(true)
455531
expect(retry.value.created).toBe(false)
456532
expect(mocks.createDocument).not.toHaveBeenCalled()
533+
expect(mocks.processQueue).not.toHaveBeenCalled()
457534
expect(mocks.recordAudit).toHaveBeenCalledTimes(1)
458535
})
459536

apps/sim/lib/knowledge/application/upload-sessions.ts

Lines changed: 55 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import { AuditAction, AuditResourceType } from '@sim/audit'
22
import type { Principal } from '@sim/auth/principal'
3-
import { createLogger } from '@sim/logger'
43
import type { V2KnowledgeDocumentUploadMetadata } from '@/lib/api/contracts/v2/knowledge'
54
import { v2KnowledgeDocumentUploadMetadataSchema } from '@/lib/api/contracts/v2/knowledge'
65
import { checkAttributedUsageLimits } from '@/lib/billing/core/billing-attribution'
@@ -39,7 +38,14 @@ import {
3938
} from '@/lib/uploads/upload-session/service'
4039
import { validateFileType } from '@/lib/uploads/utils/validation'
4140

42-
const logger = createLogger('KnowledgeUploadApplication')
41+
const PROCESSING_DISPATCH_FAILURE_MESSAGE = 'Knowledge document processing dispatch failed'
42+
43+
class KnowledgeDocumentProcessingDispatchError extends Error {
44+
constructor(cause: unknown) {
45+
super(PROCESSING_DISPATCH_FAILURE_MESSAGE, { cause })
46+
this.name = 'KnowledgeDocumentProcessingDispatchError'
47+
}
48+
}
4349

4450
export class KnowledgeDocumentUnsupportedMediaTypeError extends Error {
4551
constructor(message: string) {
@@ -253,6 +259,22 @@ export const completeKnowledgeDocumentUpload = defineAuthorizedKnowledgeUseCase(
253259
)
254260
}
255261
if (bound.status === 'bound') {
262+
if (
263+
claimed.error === PROCESSING_DISPATCH_FAILURE_MESSAGE &&
264+
bound.document.processingStatus === 'pending'
265+
) {
266+
const billingAttribution = await resolveKnowledgeBillingAttribution(
267+
principal,
268+
freshContext
269+
)
270+
await dispatchKnowledgeDocumentProcessing(
271+
bound.document,
272+
freshContext.knowledgeBaseId,
273+
processingOptions,
274+
requestId,
275+
billingAttribution
276+
)
277+
}
256278
return {
257279
value: {
258280
document: bound.document,
@@ -315,26 +337,13 @@ export const completeKnowledgeDocumentUpload = defineAuthorizedKnowledgeUseCase(
315337
throw error
316338
}
317339

318-
const processingDocument: DocumentData = {
319-
documentId: created.id,
320-
filename: created.filename,
321-
fileUrl: created.fileUrl,
322-
fileSize: created.fileSize,
323-
mimeType: created.mimeType,
324-
}
325-
processDocumentsWithQueue(
326-
[processingDocument],
340+
await dispatchKnowledgeDocumentProcessing(
341+
created,
327342
registrationContext.knowledgeBaseId,
328-
processingOptions ?? {},
343+
processingOptions,
329344
requestId,
330345
billingAttribution
331-
).catch((error: unknown) => {
332-
logger.error('Knowledge document processing pipeline failed', {
333-
knowledgeBaseId: registrationContext.knowledgeBaseId,
334-
documentId: created.id,
335-
error,
336-
})
337-
})
346+
)
338347
return {
339348
value: {
340349
document: created,
@@ -372,6 +381,33 @@ export const completeKnowledgeDocumentUpload = defineAuthorizedKnowledgeUseCase(
372381
},
373382
})
374383

384+
async function dispatchKnowledgeDocumentProcessing(
385+
document: CreatedKnowledgeDocument,
386+
knowledgeBaseId: string,
387+
processingOptions: V2KnowledgeDocumentUploadMetadata['processingOptions'],
388+
requestId: string,
389+
billingAttribution: Awaited<ReturnType<typeof resolveKnowledgeBillingAttribution>>
390+
): Promise<void> {
391+
const processingDocument: DocumentData = {
392+
documentId: document.id,
393+
filename: document.filename,
394+
fileUrl: document.fileUrl,
395+
fileSize: document.fileSize,
396+
mimeType: document.mimeType,
397+
}
398+
try {
399+
await processDocumentsWithQueue(
400+
[processingDocument],
401+
knowledgeBaseId,
402+
processingOptions ?? {},
403+
requestId,
404+
billingAttribution
405+
)
406+
} catch (error) {
407+
throw new KnowledgeDocumentProcessingDispatchError(error)
408+
}
409+
}
410+
375411
async function loadBoundKnowledgeDocumentUpload(
376412
principal: Principal,
377413
input: KnowledgeDocumentUploadControlInput,

0 commit comments

Comments
 (0)