scriptoriumadmin/open-webui
1
0
Fork 0
mirror of https://github.com/open-webui/open-webui.git synced 2026-09-13 23:14:11 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

fix: close storage leaks, surface per-file sync failures, drop unused imports

Browse source 
- upload_and_replace_file now routes both the old-file removal and the
  failure-path cleanup through the files router delete handler, so the
  object-storage blob is removed alongside the DB row, vector entries,
  and per-file vector collection. The KB-scoped remove_file helper left
  storage objects behind and was accumulating orphans across syncs.
- On processing-status failure (Step 2), the newly uploaded artifact is
  deleted instead of being left as an orphaned file row/blob.
- uploadFileHandler and addFileHandler now return explicit
  success/failure signals so the directory sync loop can track real
  outcomes. The sync summary reports succeeded counts (not planned
  counts) and switches to a warning toast with a failure tally when any
  upload or replace fails, instead of always reporting "Sync complete".
- Replace loop wraps uploadAndReplaceFile in try/catch so a single
  failed replacement no longer aborts the whole sync, and shows a per-
  file error toast with the server detail.
- Drop the unused has_access imports in files.py and knowledge.py
  added by this PR (AccessGrants.has_access and has_access_to_file are
  the actual call sites).
This commit is contained in:
DrMelone 2026-04-12 23:46:52 +02:00
parent a7cdc4d118
commit 86e6e239a4
3 changed files with 105 additions and 39 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
backend/open_webui/routers/files.py
View file

@ -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

26
backend/open_webui/routers/knowledge.py
View file

@ -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(

117
src/lib/components/workspace/Knowledge/KnowledgeBase.svelte
View file

@ -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<boolean> => {
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;
}
};