Repository navigation
fix(figma-icons-fetcher): give HTTP an explicit timeout and one retry path - #11
Merged
Merged
Conversation
… path
`pull-icons` failed with `fetch failed / Cause: Headers Timeout Error`, which
reads like a dead network or a bad token and was neither. Measured against the
real file:
- `api.figma.com` unauthenticated: HTTP 403 in 0.3 s, so the network was fine.
- `/v1/me` with the configured token: HTTP 200 in 0.97 s, so the token was fine.
- `/v1/files/:key/nodes` cold: exceeded the default budget. A retry landed at
122 s for 7.24 MB of JSON.
- The same call warm: HTTP 200 in 0.6 s.
Figma buffers that whole node tree server-side before sending a single response
header, so the wait is for the first byte. Node's default `headersTimeout` gives
up first, and it cannot be raised through `globalThis.fetch` — hence `undici` as
a direct dependency (already in the lockfile transitively; this workspace is
private and never published) and an explicit `Agent`.
`src/http.ts` is now the only HTTP layer, used by both the REST client and the
image CDN:
- Timeout. `headersTimeout` and `bodyTimeout` both get a 10-minute budget,
overridable with `FIGMA_FETCHER_TIMEOUT_MS`. A non-numeric or non-positive
override falls back to the default rather than passing `0` through, which in
undici means *no* timeout — a silent hang is worse than a slow failure.
- Retry. Five attempts, 500 ms exponential backoff, transient only. This is
`download-image.ts`'s original policy, moved rather than reinvented, so there
is one implementation instead of two and the CDN path gains the timeout it
never had.
- Fail fast where retrying cannot help. A definitive 4xx (a bad token, a wrong
file key) throws on the first attempt instead of reporting the same thing five
attempts later.
The last point is there because the first version of this change made things
worse in one case: with no network at all, the retry loop spent 7.5 minutes
before reporting `ENOTFOUND`. `isRetryableError` walks the `cause` chain, since
undici reports `fetch failed` at the surface, and treats an unreachable host as
fatal. `EAI_AGAIN` is deliberately excluded from that set: it is the transient
sibling and does come good.
Also fixes a latent bug this exposed. `fetchWrapper` used to `json()` the body
whatever the status, so a 403 became `{ data: { err: … } }` that downstream code
read as a malformed *file* — reporting a missing node instead of a rejected
request. Non-OK statuses now reject.
Verified: 21 new specs (111 total in this workspace, from 90), typecheck and lint
clean, and `pull-icons` runs end to end through the new layer — 495 icons, 10
manifests, 11.6 s warm. The fail-fast branch is negative-controlled: widening
`isTransientStatus` to treat every 4xx as transient fails two specs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Type of change
Description
pull-iconsfailed with:which reads like a dead network or a bad token, and was neither. Measured against
the real file:
api.figma.com, unauthenticated/v1/mewith the configured token/v1/files/:key/nodes, coldFigma buffers that entire node tree server-side before sending a single response
header, so the wait is for the first byte — which is why the error names headers
rather than the body. Node's default
headersTimeoutgives up first.That default cannot be raised through
globalThis.fetch, so this addsundiciasa direct dependency (already present transitively; this workspace is private and
never published) and routes requests through an explicit
Agent.src/http.ts— one HTTP layer for both call sitesPreviously the REST client had no timeout and no retry, and the image CDN had
retry but no timeout. Now both go through one place:
headersTimeoutandbodyTimeouteach get a 10-minute budget,overridable via
FIGMA_FETCHER_TIMEOUT_MS. A non-numeric or non-positiveoverride falls back to the default rather than passing
0through: in undici0means no timeout, and a silent hang is worse than a slow failure.download-image.ts's existing policy moved rather than reinvented, so thereis one implementation instead of two, and the CDN path gains the timeout it
never had.
file key) throws on the first attempt rather than reporting the same thing five
attempts and ~7 s later.
The mistake the first version made
Worth calling out, because it made one case worse before it made it better: with
no network at all, the retry loop spent 7.5 minutes backing off before
reporting
ENOTFOUND.isRetryableErrorwalks thecausechain — undici reportsa generic
fetch failedat the surface — and treats an unreachable host, aninvalid URL and a bad certificate as fatal.
EAI_AGAINis deliberately not inthat set: it is the transient sibling of
ENOTFOUNDand does come good.A latent bug this exposed
fetchWrapperused tojson()the response body whatever the status. A 403 froma bad token therefore became
{ data: { err: … } }, which downstream code read asa malformed file — so a rejected request was reported as a missing node. Non-OK
statuses now reject.
Verification
fetchis mocked:the specs assert when a request is repeated and when it is abandoned, which is
decided before any socket opens. Observing a real headers timeout would mean
waiting minutes for the condition the change exists to survive, so the budget is
asserted through
resolveTimeoutMsinstead.isTransientStatusso every 4xx lookstransient fails two specs; restoring it returns 21/21.
pull-iconsruns end to end through the new layer: 495 icons, 10 manifests,11.6 s warm.
pnpm -r typecheck,pnpm -r lint,pnpm format:checkall clean.Not addressed here
Every
pull-iconsrun also dirtiespackages/icons-svg/src/figma/solid-multi.json— the generator always writes expanded JSON, but Prettier keeps that one-entry
array on a single line. The
lint-stagedpre-commit hook silently fixes it, whichis why it has gone unnoticed. Left alone as unrelated to this fix.
Checklist