fix: retry HTTP requests on transient 5xx responses - #763
Conversation
A single transient server error (503, 502, 504) from the Eik server causes the CLI to fail immediately with no recovery. Adds a shared fetchWithRetry utility that retries up to three times with exponential backoff on 5xx responses and network-level errors. 4xx responses are not retried — they indicate client errors that will not resolve on their own. Wires the retry behaviour into: - utils/http/integrity.js — used by Version.run() to fetch the current integrity hash before comparing against the local build - utils/http/request.js — used by publish operations to upload assets to the Eik server
… the repo integrity.js retry tests now use a real Eik server (fastify + memory sink) with beforeEach/afterEach lifecycle matching other test files. The first fetch call returns a mocked 503, the retry goes to the real server and returns an actual integrity hash — verifying the retry mechanism against a realistic response. request.js retry tests remain mock-based: the upload format (tar archive, multipart) makes real-server testing complex without adding value for verifying the retry behaviour itself.
Adds retries and retryDelay options to all HTTP-related API entry points (cli.publish, cli.version, cli.integrity) and a --retries flag to the publish, version, and integrity commands. Defaults remain 3 retries with 500ms base delay. Set retries: 0 to disable retry entirely. Also wires fetchWithRetry into classes/integrity.js which previously made its own bare fetch call without any retry protection.
wkillerud
left a comment
wkillerud
left a comment
There was a problem hiding this comment.
Makes sense 👍 Would be nice to have the default retry value listed in the JS docs and --help output.
| }, | ||
| retries: { | ||
| describe: | ||
| "Number of times to retry a request on transient server errors (0 to disable)", |
There was a problem hiding this comment.
It would be nice to spell out the default value here, and maybe in some of the JSDoc as well. Had to dig quite a bit in the code to find the default now.
latest-version.js was the only HTTP utility in the fetch path that lacked retry on transient 5xx responses. Wires in fetchWithRetry alongside integrity.js and request.js. Also accepts retries and retryDelay options for consistency with the other utilities.
- CLI --retries flag now shows default: 2 and explains the backoff schedule - JSDoc @Property descriptions include the default value and what it means - fetchWithRetry JSDoc clarifies that maxRetries is total attempts, not retries - Fix retries→maxRetries mapping so user-facing retries=N means N additional retry attempts (retries=0 → 1 attempt; retries=2 → 3 total = default)
|
An issue we had earlier is that 404s were being reported I think because thats what the bucket was reporting but retries fixed it because it was a blip in the machinery not an actual 404. These retries wont help against that. But maybe you've addressed the 404 issue?? |
|
There is done multiple improvements in the server to try to catch errors trough the whole upload, extraction and write process and report accurate http status codes when error happen. Hopefully real errors happening will now surface and not be masked behind a 404. Then this retry should be able to retry on errors and not found. |
|
🎉 This PR is included in version 3.2.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
3 participants
Adds retry logic with exponential backoff to the HTTP utilities used for package integrity checks and asset uploads. A transient server error (503, 502, 504) or network failure currently causes the operation to fail immediately with no recovery.
What changes
New utility:
utils/http/retry.js—fetchWithRetry(fn, { maxRetries, baseDelayMs })retries on 5xx and network-level errors. 4xx responses return immediately without retry.Wired into:
utils/http/integrity.js— integrity hash lookups (used byVersion.run()and publish)utils/http/request.js— asset upload requestsFixed:
classes/integrity.jshad a barefetch()call that bypassed the retry logic — now usesfetchWithRetry.Configurable retry count and delay
Retry behaviour is configurable at every layer:
CLI —
--retriesflag onpublish,version, andintegritycommands:API —
retriesandretryDelayoptions on all relevant entry points:Defaults: 3 retries, 500ms base delay (doubles each attempt: 500ms → 1000ms → 1500ms).
Tests
test/retry.test.js— 7 unit tests forfetchWithRetryutilitytest/http-retry.test.js— 4 integration tests:integrity.js: real Eik server withbeforeEach/afterEach, mocked first call returns 503, retry reaches real serverrequest.js: mock-based (upload format complexity makes real-server testing add no value for this specific check)