feature/rate-limit-client-helpers ← fix/retry-rate-limited-backup-chunks
opened 07:57AM - 08 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.
### 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.
The chunk-size ladder S3 multipart has used since #22061 now also applies here, moving the trip point from roughly 140 to 560 Mbit/s so most sites stop reaching the limiter at all.
### 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.
将由以下 PR 修复：