From 8450cee02a1916155630653c26547883043c25b1 Mon Sep 17 00:00:00 2001 From: lekt8 Date: Wed, 3 Jun 2026 03:12:23 +0800 Subject: [PATCH] Surface upload failures instead of silently dropping the files (#1425) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit uploadPending() read `data.files` from /api/upload without checking `res.ok`, so a non-OK response (429 rate limit, 413 too large, …) was swallowed: the pending files vanished and the chat sent with no attachments and no feedback — part of why the model "didn't even see them" in #1346. Check res.ok; on failure show the server's reason via a toast and keep the pending files so the attach strip re-renders for a retry (matching the existing "restored on error" comment that the code never actually honored). Co-authored-by: Claude Opus 4.8 (1M context) --- static/js/fileHandler.js | 11 ++++++++++ tests/test_upload_error_surfaced.py | 31 +++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) create mode 100644 tests/test_upload_error_surfaced.py diff --git a/static/js/fileHandler.js b/static/js/fileHandler.js index e62382d..88f995c 100644 --- a/static/js/fileHandler.js +++ b/static/js/fileHandler.js @@ -172,6 +172,17 @@ export async function uploadPending() { method: 'POST', body: fd }); + if (!res.ok) { + // Surface the failure instead of swallowing it. Previously a non-OK + // response (e.g. 429 rate limit, 413 too large) was ignored: the files + // silently vanished and the chat sent with no attachments, so the model + // "didn't even see them" (issue #1346). Show the server's reason and keep + // pendingFiles so the strip re-renders for a retry (see finally below). + let detail = ''; + try { const e = await res.json(); detail = e.detail || e.error || ''; } catch (_) {} + _showToast('Upload failed' + (detail ? ': ' + detail : ` (HTTP ${res.status})`)); + return []; + } const data = await res.json(); uploaded = (data.files || []); pendingFiles = []; // clear only on success diff --git a/tests/test_upload_error_surfaced.py b/tests/test_upload_error_surfaced.py new file mode 100644 index 0000000..1eb2679 --- /dev/null +++ b/tests/test_upload_error_surfaced.py @@ -0,0 +1,31 @@ +"""Regression guard for the frontend error-surfacing follow-up to #1346. + +`uploadPending()` in static/js/fileHandler.js used to read `data.files` from the +`/api/upload` response without checking `res.ok`, so a non-OK response (429 rate +limit, 413 too large, …) was swallowed: the files silently vanished and the chat +sent with no attachments, with no feedback to the user. It now checks `res.ok` +and shows a toast on failure, keeping the pending files for a retry. + +fileHandler.js pulls in browser globals so it can't run under node; guard the +fix at the source level. +""" +import re +from pathlib import Path + +SRC = Path(__file__).resolve().parent.parent / "static/js/fileHandler.js" + + +def _upload_pending_body() -> str: + text = SRC.read_text(encoding="utf-8") + start = text.index("export async function uploadPending()") + rest = text[start:] + m = re.search(r"\n(export |function )", rest[1:]) + return rest[: m.start() + 1] if m else rest + + +def test_upload_pending_checks_response_and_surfaces_error(): + body = _upload_pending_body() + # Must guard on the HTTP status before trusting the body... + assert re.search(r"if\s*\(\s*!res\.ok\s*\)", body), "uploadPending must check res.ok" + # ...and tell the user the upload failed (not swallow it). + assert "Upload failed" in body