fix(upload): retry on dropped connections — the SDK reports XHR network failures as AbortError, which we treated as a user cancel (#172)
CI / Build & Quality Checks (push) Successful in 1m54s
CI / Docker image build & smoke test (push) Skipped
CI / Secret scan (gitleaks) (push) Successful in 7s
CI / Trigger Desktop Build (push) Successful in 5s
CI / Playwright smoke (e2e) (push) Successful in 2m6s
CI / Build & Quality Checks (push) Successful in 1m54s
CI / Docker image build & smoke test (push) Skipped
CI / Secret scan (gitleaks) (push) Successful in 7s
CI / Trigger Desktop Build (push) Successful in 5s
CI / Playwright smoke (e2e) (push) Successful in 2m6s
matrix-js-sdk rejects an upload whose XHR ends with status 0 (offline,
connection reset, DNS) with DOMException('AbortError') to mimic fetch,
the same name mx.cancelUpload() produces. isRetryableUploadError bailed
on any AbortError, so the one failure class the retry loop was built
for was never retried. Decide by our own cancel AbortSignal instead.
Verified with Playwright routing the upload endpoint: 502 → network drop
→ ok now completes in 3 attempts (1 s, 2 s back-off) and the image sends;
413 still fails fast after 1 attempt; persistent 503 gives up after 4.
Unit tests in utils/uploadRetry.test.ts.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PPmy3tPq869XDW4njjVaKA
This commit is contained in:
@@ -171,12 +171,14 @@ const matrixErrorFromUnknown = (e: unknown): MatrixError => {
|
||||
|
||||
// HTTP statuses that should not be retried — client errors are deterministic
|
||||
// (e.g. 413 payload too large, 400 bad request, 401/403 auth) and won't succeed on retry.
|
||||
const isRetryableUploadError = (e: unknown): boolean => {
|
||||
// A user-cancelled / aborted upload must never be retried. matrix-js-sdk's
|
||||
// mx.cancelUpload() rejects the upload with a DOMException named "AbortError";
|
||||
// without this guard the retry loop would resurrect an upload the user just
|
||||
// cancelled.
|
||||
if ((e as { name?: unknown } | null | undefined)?.name === 'AbortError') return false;
|
||||
const isRetryableUploadError = (e: unknown, cancelled: boolean): boolean => {
|
||||
// A user-cancelled upload must never be retried. BUT matrix-js-sdk rejects
|
||||
// with a DOMException named "AbortError" for BOTH mx.cancelUpload() and any
|
||||
// XHR that ends with status 0 — a dropped connection, going offline, DNS —
|
||||
// ("mimic fetch API", http-api/index.ts). Those are exactly the transient
|
||||
// failures this retry loop exists for, so decide by OUR cancel signal, not
|
||||
// by the error's name (Gitea #172).
|
||||
if ((e as { name?: unknown } | null | undefined)?.name === 'AbortError') return !cancelled;
|
||||
if (e instanceof MatrixError) {
|
||||
const status = e.httpStatus;
|
||||
// No status => network/transport failure (transient): retry.
|
||||
@@ -249,7 +251,7 @@ export const uploadContent = async (
|
||||
} catch (e: unknown) {
|
||||
lastError = matrixErrorFromUnknown(e);
|
||||
|
||||
if (retryCount === UPLOAD_MAX_RETRY_COUNT || !isRetryableUploadError(e)) {
|
||||
if (retryCount === UPLOAD_MAX_RETRY_COUNT || !isRetryableUploadError(e, !!signal?.aborted)) {
|
||||
onError(lastError);
|
||||
return;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user