Surface upload failures instead of silently dropping the files (#1425)
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) <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
|
||||
31
tests/test_upload_error_surfaced.py
Normal file
31
tests/test_upload_error_surfaced.py
Normal file
@@ -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
|
||||
Reference in New Issue
Block a user