RGJ
Septembre 1, 2026, 3:36
1
L’un de nos clients a rencontré un problème reproductible lors de l’envoi d’une sauvegarde Discourse locale via l’interface d’administration.
L’outil d’envoi de sauvegardes utilise des blocs fixes de 5 Mo et, sur une connexion suffisamment rapide, peut générer suffisamment de requêtes pour dépasser la limite de taux de requêtes au niveau de l’application de Discourse elle-même. Dans ce cas, le serveur renvoie un HTTP 429, mais l’outil d’envoi par blocs ne réessaie pas les réponses 429 et ne respecte pas l’en-tête Retry-After. En conséquence, l’envoi complet de la sauvegarde échoue.
Les réponses non 2xx sont converties en erreur :
if (ev.target.status < 200 || ev.target.status >= 300) {
const error = new Error("Non 2xx");
error.source = ev.target;
reject(error);
return;
}
La logique de réessai est la suivante :
_shouldRetry(err) {
if (err.source && typeof err.source.status === "number") {
const { status } = err.source;
return (
status === 0 ||
status === 409 ||
status === 423 ||
(status >= 500 && status < 600)
);
}
return false;
}
Le HTTP 429 n’est donc explicitement pas réessayable.
Une seule réponse 429 suffit pour que la promesse d’envoi par blocs échoue, déclenchant éventuellement upload-error pour l’envoi complet de la sauvegarde.
Il n’y a également aucune gestion de l’en-tête de réponse Retry-After.
J’ai trouvé ce sujet de 2021 qui a déraillé assez rapidement, mais qui semble être exactement le même problème
zogstrip
Septembre 8, 2026, 8:03
3
merci pour le rapport @RGJ
feature/rate-limit-client-helpers ← fix/retry-rate-limited-backup-chunks
merged 10:52AM - 15 Sep 26 UTC
Stacked on #43372. Fixes the report in https://meta.discourse.org/t/chunked-back… up-uploader-exceeds-discourse-request-rate-limit/411386.
Uploading a local backup through the admin UI fails outright on a fast connection. The uploader sends one POST per chunk, so throughput alone decides the request rate, and a large backup over a quick link trips the per-user request limiter. The server answers 429 and the upload dies, discarding however many gigabytes had already been sent. There is no resume, so a retry starts from chunk 1.
`_shouldRetry` allowed status 0, 409, 423 and 5xx. That list came from uppy's S3 multipart plugin, where it is correct — S3 does not answer 429 with a `Retry-After`. Discourse does, so the one status worth retrying was the one treated as fatal, and none of the four configured retry delays was ever consumed.
### Why three changes and not one
Adding 429 to the allowlist alone makes it worse. The limiters run in aggressive mode, so a rejected request re-arms the window; retrying on the existing sub-second ladder turns a fast failure into a permanent block. Measured against a persistent 429 on a 6-chunk file:
| | POSTs of the failing chunk |
|---|---|
| allowlist only | 15 in 10s, still fails |
| \+ honour `Retry-After` | 6 |
| \+ keep the chunk reserved | 2 — correct |
The third is needed because clearing `busy` before rejecting let every sibling completion re-pick the same chunk and start a second retry chain that POSTed immediately, defeating the backoff.
### Chunk size stays at 5 MB
An earlier revision of this PR raised it with the ladder S3 multipart has used since #22061. @Ethsim12 caught that this regresses stock installs, and reproduced it: these chunks go through the site's own nginx, whose shipped `client_max_body_size` is `10m`, and the multipart envelope puts a 10 MB chunk 962 bytes over that. nginx answers 413 before Rails sees the request, and 413 is not retryable — so every backup of 100 MB or more would have died on chunk 1. S3 parts have no proxy in front of them, which is why the ladder is correct there and stays there.
### Latent defects fixed alongside
- Registering the abort listener called `xhr.abort()` immediately and registered its return value, so aborting was a no-op and in-flight chunks kept running after a failure was reported.
- `chunkSize` was only assigned when no `getChunkSize` was supplied, so any caller providing one produced NaN bounds, a single empty chunk, and a silently truncated backup.
- The pause plumbing called a method the class never defined; nothing could reach it, so it is gone.
### Not a regression from the Uppy migration
Worth recording, since the linked topic asks. The 2021 report predates Uppy becoming the default backup uploader by two days — that reporter was on Resumable.js, which classified 429 as retryable but retried immediately, up to 100 times, against a limiter that was already aggressive. 429 has never been survivable here. What the port changed was the failure mode, from slow-and-noisy to instant-and-silent. The capability actually lost was resume, which this PR does not restore.
3 « J'aime »
sera corrigé par