mirror of
https://github.com/tiennm99/DocsGPT.git
synced 2026-10-12 22:14:05 +00:00
Fail stalled attachment uploads and say why an upload failed
Uploads had no timeout, so a stalled XHR could leave a chip on "uploading" forever, and a queued send waited with it. Network failures also set no errorMessage, so the failed chip showed no reason. guardUploadStall aborts an upload once it goes 120 s without progress. It restarts on every upload progress event and again when the body finishes sending. A large file on a slow link keeps going; only a silent stall is cut off. That is why it is not xhr.timeout, which caps the whole request against a 100 MB file limit. onerror, onabort and ontimeout now mark the chip failed with "Upload failed. The file couldn't be read or the connection dropped." (single and multi-file paths). A failed chip is dropped at send, so a queued question always goes out in the end. The 5-minute processing cap is unchanged.
This commit is contained in:
1 parent
be6fe8d0c2
commit
2acfa34bc5
11 files changed
+232
-19
No files matched your search
@@ -18,6 +18,7 @@ import uploadReducer, {
|
||||
type Attachment,
|
||||
} from '../upload/uploadSlice';
|
||||
import MessageInput from './MessageInput';
|
||||
import { UPLOAD_STALL_TIMEOUT_MS } from './message-input/uploadStallGuard';
|
||||
|
||||
Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true });
|
||||
|
||||
@@ -41,14 +42,53 @@ const att = (over: Partial<Attachment> = {}): Attachment => ({
|
||||
...over,
|
||||
});
|
||||
|
||||
/** Just enough of XMLHttpRequest for the composer's upload paths. */
|
||||
class FakeXHR extends EventTarget {
|
||||
static instances: FakeXHR[] = [];
|
||||
|
||||
upload = new EventTarget();
|
||||
status = 0;
|
||||
responseText = '';
|
||||
timeout = 0;
|
||||
onload: (() => void) | null = null;
|
||||
onerror: (() => void) | null = null;
|
||||
onabort: (() => void) | null = null;
|
||||
ontimeout: (() => void) | null = null;
|
||||
|
||||
open() {}
|
||||
setRequestHeader() {}
|
||||
send() {
|
||||
FakeXHR.instances.push(this);
|
||||
}
|
||||
|
||||
private finish(handler: (() => void) | null, type: string) {
|
||||
handler?.();
|
||||
this.dispatchEvent(new Event(type));
|
||||
this.dispatchEvent(new Event('loadend'));
|
||||
}
|
||||
|
||||
/** What Android Chrome reported: no response, status 0. */
|
||||
failNetwork() {
|
||||
this.status = 0;
|
||||
this.finish(this.onerror, 'error');
|
||||
}
|
||||
|
||||
abort() {
|
||||
this.finish(this.onabort, 'abort');
|
||||
}
|
||||
}
|
||||
|
||||
describe('MessageInput send with a failed attachment', () => {
|
||||
let container: HTMLDivElement;
|
||||
let root: Root;
|
||||
let store: TestStore;
|
||||
let onSubmit: ReturnType<typeof vi.fn<(text: string) => void>>;
|
||||
const realXHR = globalThis.XMLHttpRequest;
|
||||
|
||||
beforeEach(() => {
|
||||
localStorage.clear();
|
||||
FakeXHR.instances = [];
|
||||
globalThis.XMLHttpRequest = FakeXHR as unknown as typeof XMLHttpRequest;
|
||||
container = document.createElement('div');
|
||||
document.body.appendChild(container);
|
||||
root = createRoot(container);
|
||||
@@ -59,6 +99,8 @@ describe('MessageInput send with a failed attachment', () => {
|
||||
afterEach(async () => {
|
||||
await act(async () => root.unmount());
|
||||
container.remove();
|
||||
globalThis.XMLHttpRequest = realXHR;
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
const render = async (loading = false) => {
|
||||
@@ -100,6 +142,27 @@ describe('MessageInput send with a failed attachment', () => {
|
||||
});
|
||||
};
|
||||
|
||||
const attachFile = async (name: string) => {
|
||||
const input = container.querySelector<HTMLInputElement>(
|
||||
'label input[type="file"]',
|
||||
)!;
|
||||
const file = new File(['%PDF-1.4'], name, { type: 'application/pdf' });
|
||||
Object.defineProperty(input, 'files', {
|
||||
value: [file],
|
||||
configurable: true,
|
||||
});
|
||||
const sent = FakeXHR.instances.length;
|
||||
await act(async () => {
|
||||
input.dispatchEvent(new Event('change', { bubbles: true }));
|
||||
});
|
||||
// partitionAttachmentFiles sniffs the file asynchronously.
|
||||
await act(async () => {
|
||||
await vi.waitFor(() =>
|
||||
expect(FakeXHR.instances.length).toBeGreaterThan(sent),
|
||||
);
|
||||
});
|
||||
};
|
||||
|
||||
it('sends the question and drops the failed file', async () => {
|
||||
store.dispatch(addAttachment(att()));
|
||||
store.dispatch(
|
||||
@@ -127,6 +190,24 @@ describe('MessageInput send with a failed attachment', () => {
|
||||
expect(textarea().value).toBe('');
|
||||
});
|
||||
|
||||
it('sends when the only attachment failed on the network', async () => {
|
||||
await render();
|
||||
await attachFile('scan.pdf');
|
||||
expect(FakeXHR.instances).toHaveLength(1);
|
||||
|
||||
await act(async () => FakeXHR.instances[0].failNetwork());
|
||||
const [chip] = store.getState().upload.attachments;
|
||||
expect(chip.status).toBe('failed');
|
||||
expect(chip.errorMessage).toBe('conversation.attachments.uploadFailed');
|
||||
|
||||
await type('Summarise this');
|
||||
await pressEnter();
|
||||
|
||||
expect(onSubmit).toHaveBeenCalledTimes(1);
|
||||
expect(onSubmit).toHaveBeenCalledWith('Summarise this');
|
||||
expect(store.getState().upload.attachments).toEqual([]);
|
||||
});
|
||||
|
||||
it('shows no send-blocked alert after a failed send attempt', async () => {
|
||||
store.dispatch(addAttachment(att({ id: 'bad-1', status: 'failed' })));
|
||||
await render();
|
||||
@@ -137,6 +218,26 @@ describe('MessageInput send with a failed attachment', () => {
|
||||
expect(container.querySelector('[role="alert"]')).toBeNull();
|
||||
});
|
||||
|
||||
it('fails a stalled upload and then flushes the queued send', async () => {
|
||||
vi.useFakeTimers();
|
||||
await render();
|
||||
await attachFile('huge.pdf');
|
||||
expect(store.getState().upload.attachments[0].status).toBe('uploading');
|
||||
|
||||
await type('Is it done?');
|
||||
await pressEnter();
|
||||
// Pending: the send is queued, not dropped.
|
||||
expect(onSubmit).not.toHaveBeenCalled();
|
||||
|
||||
await act(async () => {
|
||||
vi.advanceTimersByTime(UPLOAD_STALL_TIMEOUT_MS);
|
||||
});
|
||||
|
||||
expect(onSubmit).toHaveBeenCalledTimes(1);
|
||||
expect(onSubmit).toHaveBeenCalledWith('Is it done?');
|
||||
expect(store.getState().upload.attachments).toEqual([]);
|
||||
});
|
||||
|
||||
it('keeps a queued question while another answer is streaming', async () => {
|
||||
store.dispatch(addAttachment(att({ status: 'processing', progress: 30 })));
|
||||
await render();
|
||||
|
||||
@@ -51,6 +51,7 @@ import {
|
||||
ToolsTrigger,
|
||||
} from './message-input';
|
||||
import { useArmedSend } from './message-input/armedSend';
|
||||
import { guardUploadStall } from './message-input/uploadStallGuard';
|
||||
import { cannotReadAttachment } from './message-input/attachmentReadability';
|
||||
import { handleAbort } from '../conversation/conversationSlice';
|
||||
import {
|
||||
@@ -555,6 +556,7 @@ export default function MessageInput({
|
||||
const files = supported;
|
||||
|
||||
const apiHost = envVar('VITE_API_HOST');
|
||||
const uploadFailedMessage = t('conversation.attachments.uploadFailed');
|
||||
|
||||
if (files.length > 1) {
|
||||
const formData = new FormData();
|
||||
@@ -830,20 +832,29 @@ export default function MessageInput({
|
||||
}
|
||||
};
|
||||
|
||||
xhr.onerror = () => {
|
||||
console.error('Upload network error');
|
||||
Object.values(indexToUiId).forEach((id) =>
|
||||
dispatch(
|
||||
updateAttachment({
|
||||
id,
|
||||
updates: { status: 'failed' },
|
||||
}),
|
||||
),
|
||||
);
|
||||
};
|
||||
// No response at all (status 0): a file the browser couldn't read,
|
||||
// a dropped connection, or a stall the guard aborted.
|
||||
xhr.onerror =
|
||||
xhr.onabort =
|
||||
xhr.ontimeout =
|
||||
() => {
|
||||
console.error('Upload network error');
|
||||
Object.values(indexToUiId).forEach((id) =>
|
||||
dispatch(
|
||||
updateAttachment({
|
||||
id,
|
||||
updates: {
|
||||
status: 'failed',
|
||||
errorMessage: uploadFailedMessage,
|
||||
},
|
||||
}),
|
||||
),
|
||||
);
|
||||
};
|
||||
|
||||
xhr.open('POST', `${apiHost}${endpoints.USER.STORE_ATTACHMENT}`);
|
||||
if (token) xhr.setRequestHeader('Authorization', `Bearer ${token}`);
|
||||
guardUploadStall(xhr);
|
||||
xhr.send(formData);
|
||||
return;
|
||||
}
|
||||
@@ -951,17 +962,24 @@ export default function MessageInput({
|
||||
}
|
||||
};
|
||||
|
||||
xhr.onerror = () => {
|
||||
dispatch(
|
||||
updateAttachment({
|
||||
id: uniqueId,
|
||||
updates: { status: 'failed' },
|
||||
}),
|
||||
);
|
||||
};
|
||||
xhr.onerror =
|
||||
xhr.onabort =
|
||||
xhr.ontimeout =
|
||||
() => {
|
||||
dispatch(
|
||||
updateAttachment({
|
||||
id: uniqueId,
|
||||
updates: {
|
||||
status: 'failed',
|
||||
errorMessage: uploadFailedMessage,
|
||||
},
|
||||
}),
|
||||
);
|
||||
};
|
||||
|
||||
xhr.open('POST', `${apiHost}${endpoints.USER.STORE_ATTACHMENT}`);
|
||||
if (token) xhr.setRequestHeader('Authorization', `Bearer ${token}`);
|
||||
guardUploadStall(xhr);
|
||||
xhr.send(formData);
|
||||
});
|
||||
},
|
||||
|
||||
@@ -0,0 +1,54 @@
|
||||
import { guardUploadStall, UPLOAD_STALL_TIMEOUT_MS } from './uploadStallGuard';
|
||||
|
||||
class FakeXHR extends EventTarget {
|
||||
upload = new EventTarget();
|
||||
abort = vi.fn(() => {
|
||||
this.dispatchEvent(new Event('abort'));
|
||||
this.dispatchEvent(new Event('loadend'));
|
||||
});
|
||||
}
|
||||
|
||||
const make = () => {
|
||||
const xhr = new FakeXHR();
|
||||
guardUploadStall(xhr as unknown as XMLHttpRequest);
|
||||
return xhr;
|
||||
};
|
||||
|
||||
describe('guardUploadStall', () => {
|
||||
beforeEach(() => vi.useFakeTimers());
|
||||
afterEach(() => vi.useRealTimers());
|
||||
|
||||
it('aborts an upload that never reports progress', () => {
|
||||
const xhr = make();
|
||||
vi.advanceTimersByTime(UPLOAD_STALL_TIMEOUT_MS - 1);
|
||||
expect(xhr.abort).not.toHaveBeenCalled();
|
||||
vi.advanceTimersByTime(1);
|
||||
expect(xhr.abort).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('lets a slow upload run while it keeps making progress', () => {
|
||||
const xhr = make();
|
||||
for (let i = 0; i < 5; i += 1) {
|
||||
vi.advanceTimersByTime(UPLOAD_STALL_TIMEOUT_MS - 1);
|
||||
xhr.upload.dispatchEvent(new Event('progress'));
|
||||
}
|
||||
expect(xhr.abort).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('gives the server a fresh window once the body is sent', () => {
|
||||
const xhr = make();
|
||||
vi.advanceTimersByTime(UPLOAD_STALL_TIMEOUT_MS - 1);
|
||||
xhr.upload.dispatchEvent(new Event('load'));
|
||||
vi.advanceTimersByTime(UPLOAD_STALL_TIMEOUT_MS - 1);
|
||||
expect(xhr.abort).not.toHaveBeenCalled();
|
||||
vi.advanceTimersByTime(1);
|
||||
expect(xhr.abort).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('stops watching once the request settles', () => {
|
||||
const xhr = make();
|
||||
xhr.dispatchEvent(new Event('loadend'));
|
||||
vi.advanceTimersByTime(UPLOAD_STALL_TIMEOUT_MS * 2);
|
||||
expect(xhr.abort).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,33 @@
|
||||
// How long an attachment upload may go without any sign of life. It is an
|
||||
// idle window, not a total cap: a large file on a slow link keeps resetting
|
||||
// it with progress events, so only a silent stall is cut off.
|
||||
export const UPLOAD_STALL_TIMEOUT_MS = 120_000;
|
||||
|
||||
/**
|
||||
* Abort ``xhr`` once it makes no progress for ``idleMs``.
|
||||
*
|
||||
* ``xhr.timeout`` would cap the whole request, which a large file on a slow
|
||||
* connection can legitimately exceed. This resets on every upload progress
|
||||
* event, and again when the body finishes sending so the server gets its own
|
||||
* window to answer. The abort fires the request's ``onabort`` handler, which
|
||||
* is where the caller marks the attachment failed.
|
||||
*
|
||||
* Args:
|
||||
* xhr: The request to watch. Call before ``send()``.
|
||||
* idleMs: The longest allowed gap between signs of progress.
|
||||
*/
|
||||
export function guardUploadStall(
|
||||
xhr: XMLHttpRequest,
|
||||
idleMs: number = UPLOAD_STALL_TIMEOUT_MS,
|
||||
): void {
|
||||
let timer: ReturnType<typeof setTimeout> | undefined;
|
||||
const restart = () => {
|
||||
clearTimeout(timer);
|
||||
timer = setTimeout(() => xhr.abort(), idleMs);
|
||||
};
|
||||
xhr.upload.addEventListener('progress', restart);
|
||||
xhr.upload.addEventListener('load', restart);
|
||||
xhr.addEventListener('progress', restart);
|
||||
xhr.addEventListener('loadend', () => clearTimeout(timer));
|
||||
restart();
|
||||
}
|
||||
@@ -1373,6 +1373,7 @@
|
||||
"attachments": {
|
||||
"attach": "Anhängen",
|
||||
"remove": "Anhang entfernen",
|
||||
"uploadFailed": "Upload fehlgeschlagen. Die Datei konnte nicht gelesen werden oder die Verbindung wurde unterbrochen.",
|
||||
"attached": "Anhang",
|
||||
"failed": "Fehlgeschlagen",
|
||||
"attachment": "Anhang"
|
||||
|
||||
@@ -1379,6 +1379,7 @@
|
||||
"attachments": {
|
||||
"attach": "Attach",
|
||||
"remove": "Remove attachment",
|
||||
"uploadFailed": "Upload failed. The file couldn't be read or the connection dropped.",
|
||||
"waitingToSend_one": "Will send when {{count}} file finishes processing…",
|
||||
"waitingToSend_other": "Will send when {{count}} files finish processing…",
|
||||
"cancelQueuedSend": "Cancel",
|
||||
|
||||
@@ -1373,6 +1373,7 @@
|
||||
"attachments": {
|
||||
"attach": "Adjuntar",
|
||||
"remove": "Eliminar adjunto",
|
||||
"uploadFailed": "Error al subir. No se pudo leer el archivo o se perdió la conexión.",
|
||||
"attached": "Adjunto",
|
||||
"failed": "Error",
|
||||
"attachment": "Adjunto"
|
||||
|
||||
@@ -1370,6 +1370,7 @@
|
||||
"attachments": {
|
||||
"attach": "添付",
|
||||
"remove": "添付ファイルを削除",
|
||||
"uploadFailed": "アップロードに失敗しました。ファイルを読み取れなかったか、接続が切断されました。",
|
||||
"attached": "添付ファイル",
|
||||
"failed": "失敗",
|
||||
"attachment": "添付ファイル"
|
||||
|
||||
@@ -1417,6 +1417,7 @@
|
||||
"attachments": {
|
||||
"attach": "Прикрепить",
|
||||
"remove": "Удалить вложение",
|
||||
"uploadFailed": "Не удалось загрузить. Файл не удалось прочитать или соединение прервалось.",
|
||||
"attached": "Вложение",
|
||||
"failed": "Ошибка",
|
||||
"attachment": "Вложение"
|
||||
|
||||
@@ -1370,6 +1370,7 @@
|
||||
"attachments": {
|
||||
"attach": "附件",
|
||||
"remove": "刪除附件",
|
||||
"uploadFailed": "上傳失敗。無法讀取檔案或連線已中斷。",
|
||||
"attached": "附件",
|
||||
"failed": "失敗",
|
||||
"attachment": "附件"
|
||||
|
||||
@@ -1370,6 +1370,7 @@
|
||||
"attachments": {
|
||||
"attach": "附件",
|
||||
"remove": "删除附件",
|
||||
"uploadFailed": "上传失败。无法读取文件或连接已断开。",
|
||||
"attached": "附件",
|
||||
"failed": "失败",
|
||||
"attachment": "附件"
|
||||
|
||||
Reference in new issue
Block a user