fix: add timeouts to build-time network requests#570
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a custom fetchWithTimeout helper to prevent hangs when fetching Hugging Face model dates and provider logos. However, the custom implementation clears the timeout as soon as the headers are received, which leaves the subsequent response body streaming unprotected. The reviewer recommends replacing this custom helper with the native AbortSignal.timeout(delay) API, which is cleaner, standard, and covers both the connection and body streaming phases.
| async function fetchWithTimeout(url, options = {}, timeout = 10000) { | ||
| const controller = new AbortController(); | ||
| const id = setTimeout(() => controller.abort(), timeout); | ||
| try { | ||
| const response = await fetch(url, { ...options, signal: controller.signal }); | ||
| clearTimeout(id); | ||
| return response; | ||
| } catch (error) { | ||
| clearTimeout(id); | ||
| throw error; | ||
| } | ||
| } | ||
|
|
||
| async function fetchCreatedAt(modelId) { | ||
| try { | ||
| const res = await fetch(`https://huggingface.co/api/models/${modelId}`, { | ||
| const res = await fetchWithTimeout(`https://huggingface.co/api/models/${modelId}`, { | ||
| headers: { "User-Agent": "vllm-recipes-build/1.0" }, | ||
| }); |
There was a problem hiding this comment.
The custom fetchWithTimeout helper clears the timeout immediately after the fetch promise resolves (when headers are received). However, the response body is read asynchronously afterwards (e.g., await res.json()). If the connection hangs or becomes extremely slow during the body streaming phase, the timeout will have already been cleared, and the script will still stall indefinitely.
Since this project runs in a modern Node.js environment (supporting global fetch), you can use the native AbortSignal.timeout(delay) API. This is much cleaner, standard, and automatically covers both the connection phase and the body streaming phase, completely eliminating the need for a custom fetchWithTimeout helper.
async function fetchCreatedAt(modelId) {
try {
const res = await fetch("https://huggingface.co/api/models/" + modelId, {
headers: { "User-Agent": "vllm-recipes-build/1.0" },
signal: AbortSignal.timeout(10000),
});There was a problem hiding this comment.
Done. Replaced custom fetchWithTimeout helper with native AbortSignal.timeout(10000).
| async function fetchWithTimeout(url, options = {}, timeout = 10000) { | ||
| const controller = new AbortController(); | ||
| const id = setTimeout(() => controller.abort(), timeout); | ||
| try { | ||
| const response = await fetch(url, { ...options, signal: controller.signal }); | ||
| clearTimeout(id); | ||
| return response; | ||
| } catch (error) { | ||
| clearTimeout(id); | ||
| throw error; | ||
| } | ||
| } | ||
|
|
||
| async function fetchOrgAvatarUrl(org) { | ||
| const res = await fetch(`https://huggingface.co/${org}`, { | ||
| const res = await fetchWithTimeout(`https://huggingface.co/${org}`, { | ||
| headers: { "User-Agent": "vllm-recipes-build/1.0" }, | ||
| }); |
There was a problem hiding this comment.
Similar to the other script, the custom fetchWithTimeout helper clears the timeout before the response body is read (e.g., await res.text()). If the connection stalls during the body streaming phase, the script will hang indefinitely.
Using the native AbortSignal.timeout(10000) API simplifies the code and ensures the timeout covers both the connection and the body streaming phases.
async function fetchOrgAvatarUrl(org) {
const res = await fetch("https://huggingface.co/" + org, {
headers: { "User-Agent": "vllm-recipes-build/1.0" },
signal: AbortSignal.timeout(10000),
});There was a problem hiding this comment.
Done. Updated fetch to use native AbortSignal.timeout(10000).
| continue; | ||
| } | ||
| const imgRes = await fetch(avatarUrl); | ||
| const imgRes = await fetchWithTimeout(avatarUrl); |
There was a problem hiding this comment.
Update this fetch call to use the native AbortSignal.timeout(10000) API to ensure the image download is fully protected by the timeout.
const imgRes = await fetch(avatarUrl, { signal: AbortSignal.timeout(10000) });There was a problem hiding this comment.
Done. Updated fetch to use native AbortSignal.timeout(10000).
1 participant
The build-time scripts
fetch-hf-dates.mjsandfetch-provider-logos.mjsperformed network requests to the HuggingFace API and external resources without any timeout configurations. In scenarios of network instability or API latency, these scripts could stall indefinitely, blocking the entirepnpm buildpipeline. I have implemented anAbortControllerwith a 10-second timeout for allfetchrequests within these scripts to ensure the build process remains robust and fails fast in the event of unreachable external services.