diff --git a/backend/open_webui/routers/files.py b/backend/open_webui/routers/files.py index 7900baa495..825565e48d 100644 --- a/backend/open_webui/routers/files.py +++ b/backend/open_webui/routers/files.py @@ -51,7 +51,6 @@ from open_webui.storage.provider import Storage from open_webui.config import BYPASS_ADMIN_ACCESS_CONTROL from open_webui.utils.auth import get_admin_user, get_verified_user -from open_webui.utils.access_control import has_access from open_webui.utils.misc import calculate_sha256_bytes from open_webui.utils.misc import strict_match_mime_type from pydantic import BaseModel diff --git a/backend/open_webui/routers/knowledge.py b/backend/open_webui/routers/knowledge.py index a96b0c7bfd..47cf8df317 100644 --- a/backend/open_webui/routers/knowledge.py +++ b/backend/open_webui/routers/knowledge.py @@ -31,7 +31,7 @@ from open_webui.routers.files import upload_file_handler, delete_file_by_id as d from open_webui.constants import ERROR_MESSAGES from open_webui.utils.auth import get_verified_user, get_admin_user -from open_webui.utils.access_control import has_access, has_permission, filter_allowed_access_grants +from open_webui.utils.access_control import has_permission, filter_allowed_access_grants from open_webui.models.access_grants import AccessGrants @@ -775,6 +775,12 @@ async def upload_and_replace_file( new_file = await Files.get_file_by_id(new_file_id, db=db) if not new_file or not new_file.data or new_file.data.get('status') != 'completed': error_detail = (new_file.data or {}).get('error') if new_file else None + # Clean up the newly uploaded artifact so failed replacements don't + # accumulate orphaned file rows and storage blobs over repeated syncs. + try: + await delete_file_by_id_route(id=new_file_id, user=user, db=db) + except Exception as cleanup_err: + log.warning(f'Failed to clean up new file {new_file_id} after processing failure: {cleanup_err}') raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, detail=error_detail or ERROR_MESSAGES.FILE_NOT_PROCESSED, @@ -806,17 +812,15 @@ async def upload_and_replace_file( detail=f'Failed to add file to knowledge base: {str(e)}', ) - # Step 4: Remove old file. Any failure here means "replace" semantics - # were violated (new file added but old file still present); surface the - # error so callers can reconcile instead of silently accumulating dupes. + # Step 4: Remove old file. Use the files router delete handler so the + # object storage blob is removed too — the KB-scoped remove_file helper + # drops the DB row and vector entries but not the Storage blob, which + # would accumulate orphans in S3/GCS/local storage over repeated syncs. + # Any failure here means "replace" semantics were violated (new file + # added but old file still present); surface the error so callers can + # reconcile instead of silently accumulating duplicates. try: - await remove_file_from_knowledge_by_id( - id=id, - form_data=KnowledgeFileIdForm(file_id=old_file_id), - delete_file=True, - user=user, - db=db, - ) + await delete_file_by_id_route(id=old_file_id, user=user, db=db) except Exception as e: log.error(f'Failed to remove old file after replacement upload: {e}') raise HTTPException( diff --git a/src/lib/components/workspace/Knowledge/KnowledgeBase.svelte b/src/lib/components/workspace/Knowledge/KnowledgeBase.svelte index d61f47e6d1..6f390888f9 100644 --- a/src/lib/components/workspace/Knowledge/KnowledgeBase.svelte +++ b/src/lib/components/workspace/Knowledge/KnowledgeBase.svelte @@ -264,7 +264,12 @@ } }; - const uploadFileHandler = async (file) => { + // Returns true only if the file was uploaded AND added to the KB without + // any error surfaced. Callers (notably the directory sync flow) rely on + // this boolean to distinguish genuine successes from empty-file / size- + // limit / upload / add failures that this handler otherwise swallows via + // toasts. + const uploadFileHandler = async (file): Promise => { console.log(file); const fileItem = { @@ -281,7 +286,7 @@ if (fileItem.size == 0) { toast.error($i18n.t('You cannot upload an empty file.')); - return null; + return false; } if ( @@ -297,7 +302,7 @@ maxSize: $config?.file?.max_size }) ); - return; + return false; } fileItems = [fileItem, ...(fileItems ?? [])]; @@ -331,14 +336,18 @@ console.warn('File upload warning:', uploadedFile.error); toast.warning(uploadedFile.error); fileItems = fileItems.filter((file) => file.id !== uploadedFile.id); + return false; } else { - await addFileHandler(uploadedFile.id); + const added = await addFileHandler(uploadedFile.id); + return Boolean(added); } } else { toast.error($i18n.t('Failed to upload file.')); + return false; } } catch (e) { toast.error(`${e}`); + return false; } }; @@ -567,11 +576,20 @@ } } - // STEP 2: Upload new files + // STEP 2: Upload new files. uploadFileHandler swallows per-file + // errors via toasts and returns a boolean; track successes so the + // final summary doesn't over-report completion when uploads fail. + let newSucceeded = 0; + let newFailed = 0; for (const filePath of new_files) { const fileData = directoryFiles.find((f) => f.path === filePath); if (fileData) { - await uploadFileHandler(fileData.file); + const ok = await uploadFileHandler(fileData.file); + if (ok) { + newSucceeded++; + } else { + newFailed++; + } processedCount++; toast.info( $i18n.t('Uploading new: {{current}}/{{total}}', { @@ -581,17 +599,38 @@ ); } } - - // STEP 3: Upload changed files using atomic upload_and_replace endpoint + + // STEP 3: Upload changed files using atomic upload_and_replace + // endpoint. The API wrapper throws on failure, so catch per file + // to keep the sync going and report an accurate changed/failed + // split at the end. + let changedSucceeded = 0; + let changedFailed = 0; for (const changedFile of changed_files) { const fileData = directoryFiles.find((f) => f.path === changedFile.file_path); if (fileData) { - await uploadAndReplaceFile( - localStorage.token, - id, - fileData.file, - changedFile.old_file_id - ); + try { + await uploadAndReplaceFile( + localStorage.token, + id, + fileData.file, + changedFile.old_file_id + ); + changedSucceeded++; + } catch (replaceErr) { + changedFailed++; + console.error('Replace failed for', changedFile.file_path, replaceErr); + const detail = + typeof replaceErr === 'string' + ? replaceErr + : (replaceErr?.detail ?? replaceErr?.message ?? 'unknown error'); + toast.error( + $i18n.t('Failed to update {{path}}: {{detail}}', { + path: changedFile.file_path, + detail + }) + ); + } processedCount++; toast.info( $i18n.t('Updating: {{current}}/{{total}}', { @@ -601,19 +640,37 @@ ); } } - - // Show summary - toast.success( - $i18n.t( - 'Sync complete: {{newCount}} new, {{changedCount}} updated, {{removedCount}} removed, {{unchangedCount}} unchanged', - { - newCount: new_files.length, - changedCount: changed_files.length, - removedCount: removed_file_ids.length, - unchangedCount: unchanged.length - } - ) - ); + + // Show summary. Use succeeded counts — not the planned counts — + // so the user sees real outcomes. Include a failure tally only + // when something went wrong. + const totalFailed = newFailed + changedFailed; + if (totalFailed > 0) { + toast.warning( + $i18n.t( + 'Sync finished with issues: {{newCount}} new, {{changedCount}} updated, {{removedCount}} removed, {{unchangedCount}} unchanged, {{failedCount}} failed', + { + newCount: newSucceeded, + changedCount: changedSucceeded, + removedCount: removed_file_ids.length, + unchangedCount: unchanged.length, + failedCount: totalFailed + } + ) + ); + } else { + toast.success( + $i18n.t( + 'Sync complete: {{newCount}} new, {{changedCount}} updated, {{removedCount}} removed, {{unchangedCount}} unchanged', + { + newCount: newSucceeded, + changedCount: changedSucceeded, + removedCount: removed_file_ids.length, + unchangedCount: unchanged.length + } + ) + ); + } // Refresh the file list await init(); @@ -627,6 +684,10 @@ } }; + // Returns a truthy value only when the file actually made it into the KB, + // so upstream flows (uploadFileHandler → directory sync) can distinguish + // real successes from add failures this handler otherwise absorbs via + // toasts. const addFileHandler = async (fileId) => { const res = await addFileToKnowledgeById(localStorage.token, id, fileId).catch((e) => { toast.error(`${e}`); @@ -636,9 +697,11 @@ if (res) { toast.success($i18n.t('File added successfully.')); init(); + return res; } else { toast.error($i18n.t('Failed to add file.')); fileItems = fileItems.filter((file) => file.id !== fileId); + return null; } };