Repository navigation
fix: add AbortController timeout to fetchImageEndpoint - #3105
diegosouzapw merged 2 commits into
Conversation
fetchImageEndpoint uses raw fetch() without timeout control. Long-running image generation requests (~20-30s) are killed by Next.js default timeout, producing upstream_error responses. Replace fetch() with fetchWithTimeout() from shared utils, defaulting to FETCH_TIMEOUT_MS (120s via OMNIROUTE_DEFAULT_FETCH_TIMEOUT_MS).
There was a problem hiding this comment.
Code Review
This pull request integrates fetchWithTimeout and getConfiguredTimeout into the fetchImageEndpoint function within open-sse/handlers/imageGeneration.ts to enforce request timeouts. The review feedback recommends adding unit tests for this new behavior to comply with the repository style guide, and catching FetchTimeoutError or AbortError to return a 504 Gateway Timeout status code instead of a misleading 502 error.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| extractComfyOutputFiles, | ||
| } from "../utils/comfyuiClient.ts"; | ||
| import { fetchRemoteImage } from "@/shared/network/remoteImageFetch"; | ||
| import { fetchWithTimeout, getConfiguredTimeout } from "@/shared/utils/fetchTimeout"; |
There was a problem hiding this comment.
According to Rule 9 of the Repository Style Guide, we must always include tests when changing production code in open-sse/. Please add unit tests to verify the new timeout behavior of fetchImageEndpoint and ensure that fetchWithTimeout is correctly invoked with the configured timeout.
| import { fetchWithTimeout, getConfiguredTimeout } from "@/shared/utils/fetchTimeout"; | |
| import { fetchWithTimeout, getConfiguredTimeout, FetchTimeoutError } from "@/shared/utils/fetchTimeout"; |
References
- Always include tests when changing production code (src/, open-sse/, electron/, bin/). (link)
| const response = await fetchWithTimeout(url, { | ||
| method: "POST", | ||
| headers, | ||
| body, | ||
| timeoutMs: getConfiguredTimeout(), | ||
| }); |
There was a problem hiding this comment.
When an upstream image generation request times out, returning a 502 (Bad Gateway) status code is misleading. To align with other timeout handlers in this file (which return 504), we should catch FetchTimeoutError or AbortError and return a 504 (Gateway Timeout) status code.
let response;
try {
response = await fetchWithTimeout(url, {
method: "POST",
headers,
body,
timeoutMs: getConfiguredTimeout(),
});
} catch (err: any) {
if (err instanceof FetchTimeoutError || err?.name === "AbortError") {
if (log) {
log.error("IMAGE", `${provider} fetch error: ${err.message}`);
}
return {
success: false,
status: 504,
error: `Image provider error: ${sanitizeErrorMessage(err.message || err)}`,
};
}
throw err;
}
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (1 file)
Reviewed by nemotron-3-super-120b-a12b-20230311:free · 372,991 tokens |
Address review feedback from gemini-code-assist: - Import FetchTimeoutError and catch it in fetchImageEndpoint - Return 504 (Gateway Timeout) instead of 502 for timeout/AbortError - Add 3 focused unit tests for timeout, non-timeout, and success paths Clarifies timeout semantics: timeout is a gateway timeout (504), not a bad gateway (502). Non-timeout fetch errors remain 502.
|
Thanks, @mgarmash! 🙏 Wrapping |
0331e81
into
cheaper-inference:release/v3.8.9
…ence#3105) * fix: add AbortController timeout to fetchImageEndpoint fetchImageEndpoint uses raw fetch() without timeout control. Long-running image generation requests (~20-30s) are killed by Next.js default timeout, producing upstream_error responses. Replace fetch() with fetchWithTimeout() from shared utils, defaulting to FETCH_TIMEOUT_MS (120s via OMNIROUTE_DEFAULT_FETCH_TIMEOUT_MS). * fix: return 504 for image fetch timeout, add tests Address review feedback from gemini-code-assist: - Import FetchTimeoutError and catch it in fetchImageEndpoint - Return 504 (Gateway Timeout) instead of 502 for timeout/AbortError - Add 3 focused unit tests for timeout, non-timeout, and success paths Clarifies timeout semantics: timeout is a gateway timeout (504), not a bad gateway (502). Non-timeout fetch errors remain 502. --------- Co-authored-by: mgarmash <mgarmash@37bytes.com>
…ors hall Adds entries for cheaper-inference#3097, cheaper-inference#3101 (deepseek-web cheaper-inference#2942/cheaper-inference#2820), cheaper-inference#3104, cheaper-inference#3105, cheaper-inference#3107, cheaper-inference#3109, cheaper-inference#3111, cheaper-inference#3113, cheaper-inference#3115, cheaper-inference#3122, cheaper-inference#3125, cheaper-inference#3127, cheaper-inference#3129, plus a Contributors section crediting all v3.8.9 contributors. Stamps the 3.8.9 release date.
…ence#3105) * fix: add AbortController timeout to fetchImageEndpoint fetchImageEndpoint uses raw fetch() without timeout control. Long-running image generation requests (~20-30s) are killed by Next.js default timeout, producing upstream_error responses. Replace fetch() with fetchWithTimeout() from shared utils, defaulting to FETCH_TIMEOUT_MS (120s via OMNIROUTE_DEFAULT_FETCH_TIMEOUT_MS). * fix: return 504 for image fetch timeout, add tests Address review feedback from gemini-code-assist: - Import FetchTimeoutError and catch it in fetchImageEndpoint - Return 504 (Gateway Timeout) instead of 502 for timeout/AbortError - Add 3 focused unit tests for timeout, non-timeout, and success paths Clarifies timeout semantics: timeout is a gateway timeout (504), not a bad gateway (502). Non-timeout fetch errors remain 502. --------- Co-authored-by: mgarmash <mgarmash@37bytes.com>
…ors hall Adds entries for cheaper-inference#3097, cheaper-inference#3101 (deepseek-web cheaper-inference#2942/cheaper-inference#2820), cheaper-inference#3104, cheaper-inference#3105, cheaper-inference#3107, cheaper-inference#3109, cheaper-inference#3111, cheaper-inference#3113, cheaper-inference#3115, cheaper-inference#3122, cheaper-inference#3125, cheaper-inference#3127, cheaper-inference#3129, plus a Contributors section crediting all v3.8.9 contributors. Stamps the 3.8.9 release date.
…ence#3105) * fix: add AbortController timeout to fetchImageEndpoint fetchImageEndpoint uses raw fetch() without timeout control. Long-running image generation requests (~20-30s) are killed by Next.js default timeout, producing upstream_error responses. Replace fetch() with fetchWithTimeout() from shared utils, defaulting to FETCH_TIMEOUT_MS (120s via OMNIROUTE_DEFAULT_FETCH_TIMEOUT_MS). * fix: return 504 for image fetch timeout, add tests Address review feedback from gemini-code-assist: - Import FetchTimeoutError and catch it in fetchImageEndpoint - Return 504 (Gateway Timeout) instead of 502 for timeout/AbortError - Add 3 focused unit tests for timeout, non-timeout, and success paths Clarifies timeout semantics: timeout is a gateway timeout (504), not a bad gateway (502). Non-timeout fetch errors remain 502. --------- Co-authored-by: mgarmash <mgarmash@37bytes.com>
…ors hall Adds entries for cheaper-inference#3097, cheaper-inference#3101 (deepseek-web cheaper-inference#2942/cheaper-inference#2820), cheaper-inference#3104, cheaper-inference#3105, cheaper-inference#3107, cheaper-inference#3109, cheaper-inference#3111, cheaper-inference#3113, cheaper-inference#3115, cheaper-inference#3122, cheaper-inference#3125, cheaper-inference#3127, cheaper-inference#3129, plus a Contributors section crediting all v3.8.9 contributors. Stamps the 3.8.9 release date.
Problem
fetchImageEndpoint()inopen-sse/handlers/imageGeneration.tsuses rawfetch()without any timeout control. When calling upstream image generation APIs (OpenAI, Nebius, etc.) that take 20-30+ seconds, Next.js self-hosted default timeout kills the connection, producing:{"error":{"message":"terminated","type":"upstream_error","code":"upstream_error"}}The chat/SSE endpoint already uses
fetchWithTimeout()withFETCH_TIMEOUT_MS(default 600s) viaopen-sse/config/constants.ts, but image generation does not.Fix
fetchWithTimeoutandgetConfiguredTimeoutfrom@/shared/utils/fetchTimeout(the same utility already used in the SSE code path)fetch()withfetchWithTimeout()infetchImageEndpoint(), usingtimeoutMs: getConfiguredTimeout()Default timeout: 120s (
OMNIROUTE_DEFAULT_FETCH_TIMEOUT_MSenv var) orFETCH_TIMEOUT_MSenv var.Files changed
open-sse/handlers/imageGeneration.ts— add import, replace fetch with fetchWithTimeout in fetchImageEndpointTesting
Reproduced issue with
/v1/images/generationsrequests taking >30s. After fix, requests complete successfully within the configured timeout window.