fix(upload): plain-language upload failure text instead of the raw MatrixError (#213)
The upload card printed the SDK's toString — 'MatrixError: [413] nope (http://<hs>/_matrix/media/v3/upload?filename=…)'. describeUploadError() maps the common cases to one sentence: 413/M_TOO_LARGE → 'This file is larger than the server allows (limit N)' using m.upload.size when known, 429 → 'Slow down — try again in a moment.', 401/403 → 'The server refused this upload: <server text>', 5xx/transport after the retry loop → 'Couldn't reach the server. Check your connection and retry.', other 4xx → the server's own sentence, URL stripped. Both card renderers use it; the raw error is still console.warn-ed by uploadContent for debugging. Unit-tested; verified headless with routed 413/403/503 responses. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PPmy3tPq869XDW4njjVaKA
This commit is contained in:
@@ -6,6 +6,7 @@ import { useMatrixClient } from '../../hooks/useMatrixClient';
|
|||||||
import { TUploadContent } from '../../utils/matrix';
|
import { TUploadContent } from '../../utils/matrix';
|
||||||
import { bytesToSize, getFileTypeIcon } from '../../utils/common';
|
import { bytesToSize, getFileTypeIcon } from '../../utils/common';
|
||||||
import { useMediaConfig } from '../../hooks/useMediaConfig';
|
import { useMediaConfig } from '../../hooks/useMediaConfig';
|
||||||
|
import { describeUploadError } from '../../utils/uploadError';
|
||||||
|
|
||||||
type CompactUploadCardRendererProps = {
|
type CompactUploadCardRendererProps = {
|
||||||
isEncrypted?: boolean;
|
isEncrypted?: boolean;
|
||||||
@@ -91,7 +92,7 @@ export function CompactUploadCardRenderer({
|
|||||||
)}
|
)}
|
||||||
{upload.status === UploadStatus.Error && (
|
{upload.status === UploadStatus.Error && (
|
||||||
<UploadCardError>
|
<UploadCardError>
|
||||||
<Text size="T200">{upload.error.message}</Text>
|
<Text size="T200">{describeUploadError(upload.error, allowSize)}</Text>
|
||||||
</UploadCardError>
|
</UploadCardError>
|
||||||
)}
|
)}
|
||||||
{upload.status === UploadStatus.Idle && fileSizeExceeded && (
|
{upload.status === UploadStatus.Idle && fileSizeExceeded && (
|
||||||
|
|||||||
@@ -24,6 +24,7 @@ import {
|
|||||||
} from '../../state/room/roomInputDrafts';
|
} from '../../state/room/roomInputDrafts';
|
||||||
import { useObjectURL } from '../../hooks/useObjectURL';
|
import { useObjectURL } from '../../hooks/useObjectURL';
|
||||||
import { useMediaConfig } from '../../hooks/useMediaConfig';
|
import { useMediaConfig } from '../../hooks/useMediaConfig';
|
||||||
|
import { describeUploadError } from '../../utils/uploadError';
|
||||||
import { compressImage, formatFileSize, isCompressible } from '../../utils/imageCompression';
|
import { compressImage, formatFileSize, isCompressible } from '../../utils/imageCompression';
|
||||||
|
|
||||||
type PreviewImageProps = {
|
type PreviewImageProps = {
|
||||||
@@ -389,7 +390,7 @@ export function UploadCardRenderer({
|
|||||||
)}
|
)}
|
||||||
{upload.status === UploadStatus.Error && (
|
{upload.status === UploadStatus.Error && (
|
||||||
<UploadCardError>
|
<UploadCardError>
|
||||||
<Text size="T200">{upload.error.message}</Text>
|
<Text size="T200">{describeUploadError(upload.error, allowSize)}</Text>
|
||||||
</UploadCardError>
|
</UploadCardError>
|
||||||
)}
|
)}
|
||||||
{upload.status === UploadStatus.Idle && fileSizeExceeded && (
|
{upload.status === UploadStatus.Idle && fileSizeExceeded && (
|
||||||
|
|||||||
+11
-5
@@ -221,12 +221,18 @@ export const uploadContent = async (
|
|||||||
|
|
||||||
const abortError = () =>
|
const abortError = () =>
|
||||||
matrixErrorFromUnknown(new DOMException('Upload cancelled', 'AbortError'));
|
matrixErrorFromUnknown(new DOMException('Upload cancelled', 'AbortError'));
|
||||||
|
// The card shows a plain sentence (describeUploadError); keep the raw error
|
||||||
|
// (status, errcode, URL) in the console for debugging.
|
||||||
|
const fail = (err: MatrixError) => {
|
||||||
|
if (err.data?.error !== 'Upload cancelled') console.warn('[upload] failed:', err);
|
||||||
|
onError(err);
|
||||||
|
};
|
||||||
|
|
||||||
let lastError: MatrixError | undefined;
|
let lastError: MatrixError | undefined;
|
||||||
|
|
||||||
for (let retryCount = 0; retryCount <= UPLOAD_MAX_RETRY_COUNT; retryCount += 1) {
|
for (let retryCount = 0; retryCount <= UPLOAD_MAX_RETRY_COUNT; retryCount += 1) {
|
||||||
if (signal?.aborted) {
|
if (signal?.aborted) {
|
||||||
onError(abortError());
|
fail(abortError());
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
const uploadPromise = mx.uploadContent(file, {
|
const uploadPromise = mx.uploadContent(file, {
|
||||||
@@ -246,13 +252,13 @@ export const uploadContent = async (
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
// Missing content_uri is not a transient failure — fail immediately.
|
// Missing content_uri is not a transient failure — fail immediately.
|
||||||
onError(matrixErrorFromUploadResponse(data));
|
fail(matrixErrorFromUploadResponse(data));
|
||||||
return;
|
return;
|
||||||
} catch (e: unknown) {
|
} catch (e: unknown) {
|
||||||
lastError = matrixErrorFromUnknown(e);
|
lastError = matrixErrorFromUnknown(e);
|
||||||
|
|
||||||
if (retryCount === UPLOAD_MAX_RETRY_COUNT || !isRetryableUploadError(e, !!signal?.aborted)) {
|
if (retryCount === UPLOAD_MAX_RETRY_COUNT || !isRetryableUploadError(e, !!signal?.aborted)) {
|
||||||
onError(lastError);
|
fail(lastError);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -265,14 +271,14 @@ export const uploadContent = async (
|
|||||||
await sleepForMs(waitMS);
|
await sleepForMs(waitMS);
|
||||||
// Cancelled during the back-off — stop instead of resurrecting the upload.
|
// Cancelled during the back-off — stop instead of resurrecting the upload.
|
||||||
if (signal?.aborted) {
|
if (signal?.aborted) {
|
||||||
onError(abortError());
|
fail(abortError());
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Unreachable in practice, but keeps onError guaranteed if the loop exits.
|
// Unreachable in practice, but keeps onError guaranteed if the loop exits.
|
||||||
if (lastError) onError(lastError);
|
if (lastError) fail(lastError);
|
||||||
};
|
};
|
||||||
|
|
||||||
export const matrixEventByRecency = (m1: MatrixEvent, m2: MatrixEvent) => m2.getTs() - m1.getTs();
|
export const matrixEventByRecency = (m1: MatrixEvent, m2: MatrixEvent) => m2.getTs() - m1.getTs();
|
||||||
|
|||||||
@@ -0,0 +1,48 @@
|
|||||||
|
import { test } from 'node:test';
|
||||||
|
import assert from 'node:assert/strict';
|
||||||
|
import { MatrixError } from 'matrix-js-sdk';
|
||||||
|
import { describeUploadError } from './uploadError';
|
||||||
|
|
||||||
|
const err = (status: number | undefined, body: { errcode?: string; error?: string }) =>
|
||||||
|
new MatrixError(body, status, 'http://hs/_matrix/media/v3/upload?filename=x.png');
|
||||||
|
|
||||||
|
test('413 names the limit when known and never leaks the URL', () => {
|
||||||
|
const e = err(413, { errcode: 'M_TOO_LARGE', error: 'nope' });
|
||||||
|
assert.equal(
|
||||||
|
describeUploadError(e, 50 * 1024 * 1024),
|
||||||
|
'This file is larger than the server allows (limit 52.4 MB).',
|
||||||
|
);
|
||||||
|
assert.equal(describeUploadError(e), 'This file is larger than the server allows.');
|
||||||
|
assert.ok(!describeUploadError(e).includes('http'));
|
||||||
|
});
|
||||||
|
|
||||||
|
test('429, 403, 5xx and transport failures each get their sentence', () => {
|
||||||
|
assert.equal(
|
||||||
|
describeUploadError(err(429, { errcode: 'M_LIMIT_EXCEEDED', error: 'Too Many Requests' })),
|
||||||
|
'Slow down — try again in a moment.',
|
||||||
|
);
|
||||||
|
assert.equal(
|
||||||
|
describeUploadError(err(403, { errcode: 'M_FORBIDDEN', error: 'Uploads disabled' })),
|
||||||
|
'The server refused this upload: Uploads disabled',
|
||||||
|
);
|
||||||
|
assert.equal(
|
||||||
|
describeUploadError(err(502, { errcode: 'M_UNKNOWN', error: 'Bad Gateway' })),
|
||||||
|
"Couldn't reach the server. Check your connection and retry.",
|
||||||
|
);
|
||||||
|
assert.equal(
|
||||||
|
describeUploadError(new MatrixError({ error: 'request failed' })),
|
||||||
|
"Couldn't reach the server. Check your connection and retry.",
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a cancel and an unknown 4xx fall through to the server text', () => {
|
||||||
|
assert.equal(
|
||||||
|
describeUploadError(new MatrixError({ error: 'Upload cancelled' })),
|
||||||
|
'Upload cancelled.',
|
||||||
|
);
|
||||||
|
assert.equal(
|
||||||
|
describeUploadError(err(400, { errcode: 'M_BAD_JSON', error: 'Unsupported file type' })),
|
||||||
|
'Unsupported file type',
|
||||||
|
);
|
||||||
|
assert.equal(describeUploadError(err(400, {})), 'Upload failed.');
|
||||||
|
});
|
||||||
@@ -0,0 +1,37 @@
|
|||||||
|
import { MatrixError } from 'matrix-js-sdk';
|
||||||
|
import { bytesToSize } from './common';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* [Gitea #213] One plain sentence for a failed upload. `MatrixError.message`
|
||||||
|
* is the SDK's toString — "MatrixError: [413] nope (http://hs/_matrix/media/…)"
|
||||||
|
* — which is what the card used to print. The raw error still goes to the
|
||||||
|
* console for debugging.
|
||||||
|
*/
|
||||||
|
export const describeUploadError = (error: MatrixError, limitBytes?: number): string => {
|
||||||
|
const status = error.httpStatus;
|
||||||
|
const errcode = error.errcode;
|
||||||
|
const serverText = (error.data as { error?: string } | undefined)?.error;
|
||||||
|
|
||||||
|
if (status === 413 || errcode === 'M_TOO_LARGE') {
|
||||||
|
const limit =
|
||||||
|
typeof limitBytes === 'number' && Number.isFinite(limitBytes)
|
||||||
|
? ` (limit ${bytesToSize(limitBytes)})`
|
||||||
|
: '';
|
||||||
|
return `This file is larger than the server allows${limit}.`;
|
||||||
|
}
|
||||||
|
if (status === 429 || errcode === 'M_LIMIT_EXCEEDED') {
|
||||||
|
return 'Slow down — try again in a moment.';
|
||||||
|
}
|
||||||
|
if (status === 401 || status === 403 || errcode === 'M_FORBIDDEN') {
|
||||||
|
return serverText
|
||||||
|
? `The server refused this upload: ${serverText}`
|
||||||
|
: 'The server refused this upload.';
|
||||||
|
}
|
||||||
|
if (typeof status !== 'number' || status === 408 || status >= 500) {
|
||||||
|
// Transport failure after the retry loop gave up, or a server-side error.
|
||||||
|
if (serverText === 'Upload cancelled') return 'Upload cancelled.';
|
||||||
|
return "Couldn't reach the server. Check your connection and retry.";
|
||||||
|
}
|
||||||
|
// Any other 4xx: the server's own sentence, never the URL-bearing toString.
|
||||||
|
return serverText ? serverText.replace(/\s*\(https?:\/\/[^)]*\)\s*$/, '') : 'Upload failed.';
|
||||||
|
};
|
||||||
Reference in New Issue
Block a user