From 8da83e1cc6c70dab48029f57db1558fdcd8e6b6d Mon Sep 17 00:00:00 2001 From: "Syring, Nikolas" Date: Wed, 13 May 2026 11:18:12 +0200 Subject: [PATCH] fix(router): configurable upstream-fetch timeout for /api/proxy via PROXY_REQUEST_TIMEOUT_SECONDS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `/api/proxy` handler in `server/router/proxy.js` forwards client requests to arbitrary upstream HTTP services using the runtime fetch implementation (undici on Node.js). Undici applies a default `headersTimeout` of 300 seconds when no explicit timeout is supplied, which is too tight for upstream services that stall before producing response headers — most importantly long prompt-prefill phases on local LLM servers (llama.cpp, qwen serve, vLLM) where the upstream stays silent for minutes during PP on long-context conversations. When the undici timeout fires the proxy emits a 502 with "Upstream fetch failed: ..." to the client, which the in-app fetch proxy then surfaces as a connection error mid-prompt. Operators have no way to relax this from configuration today because the proxy never attaches a `signal` to the upstream `fetch(...)` call. Make the proxy timeout explicit and operator-configurable: - new `PROXY_REQUEST_TIMEOUT_SECONDS` runtime param, defaulting to `900` (15 minutes); `0` disables the proxy-imposed timeout entirely and falls back to whatever lower layers enforce; positive values apply as a wall-clock abort - `proxyExternalRequest(...)` now accepts a fourth `options` argument carrying `runtimeParams`, resolves the configured timeout, and attaches an `AbortSignal.timeout(...)` to the upstream `fetch(...)` - timeout aborts are detected by `error.name === "TimeoutError"` (and the matching undici code) and translated into a `504 Gateway Timeout` response with a hint that operators can raise or disable `PROXY_REQUEST_TIMEOUT_SECONDS`; other upstream failures keep the existing `502` semantic so the failure shape stays distinguishable `router.js` already has `runtimeParams` in scope for the request context and now forwards it to `proxyExternalRequest(...)`. No other caller exists. Add `tests/proxy_request_timeout_test.mjs` covering the new helper: - default fallback when the runtime params interface is missing, empty, or returns an unparseable value - positive override values in seconds - the `0` disable signal mapping - fractional seconds truncating to whole milliseconds - a regression guard that the default stays comfortably above undici's built-in 300-second `headersTimeout` so the proxy override actually takes effect Documentation: - list `PROXY_REQUEST_TIMEOUT_SECONDS` in `docs/cli/commands-and-runtime-params.md` alongside the existing high-value param descriptions - add a Proxy contract block in `server/router/AGENTS.md` describing the timeout flow, the `504` aborted-call response, and the per-request resolution path through `runtimeParams` Co-Authored-By: Claude Opus 4.7 (1M context) --- .../docs/cli/commands-and-runtime-params.md | 2 + server/router/AGENTS.md | 6 ++ server/router/proxy.js | 69 ++++++++++++++- server/router/router.js | 2 +- tests/proxy_request_timeout_test.mjs | 83 +++++++++++++++++++ 5 files changed, 158 insertions(+), 4 deletions(-) create mode 100644 tests/proxy_request_timeout_test.mjs diff --git a/app/L0/_all/mod/_core/documentation/docs/cli/commands-and-runtime-params.md b/app/L0/_all/mod/_core/documentation/docs/cli/commands-and-runtime-params.md index eac0c9c8..b9a55c2f 100644 --- a/app/L0/_all/mod/_core/documentation/docs/cli/commands-and-runtime-params.md +++ b/app/L0/_all/mod/_core/documentation/docs/cli/commands-and-runtime-params.md @@ -187,6 +187,7 @@ Current params: - `GIT_BACKEND` - `GIT_URL` - `USER_FOLDER_SIZE_LIMIT_BYTES` +- `PROXY_REQUEST_TIMEOUT_SECONDS` Important fields per param: @@ -213,6 +214,7 @@ Only params with `frontend_exposed: true` are injected into page-shell meta tags - `GIT_BACKEND`: selects the backend used by server-owned Git flows such as local history and Git-backed module installs; defaults to `auto`, which keeps the normal `native -> isomorphic` fallback order, while concrete values force one backend for local testing or troubleshooting - `GIT_URL`: optional Git repository URL used by `node space update` and `node space supervise`; if unset they fall back to the local `origin` remote URL and only then to the canonical repo URL - `USER_FOLDER_SIZE_LIMIT_BYTES`: optional per-user `L2//` folder cap in bytes; `0` disables it, and positive values make app-file mutations reject projected growth over the cap while still allowing mutations that reduce an already-over-limit folder +- `PROXY_REQUEST_TIMEOUT_SECONDS`: upstream-fetch timeout used by the `/api/proxy` handler in `server/router/proxy.js`; defaults to `900` (15 minutes) so long upstream prompt-prefill phases on local LLM servers are not cut off by the runtime fetch implementation's tight `headersTimeout`; `0` disables the proxy-imposed timeout entirely and leaves the call to lower-layer wall-clock limits; positive values apply as a wall-clock abort from the moment the proxy starts the upstream request - `user` and `group` commands flush pending local-history commits before returning when `CUSTOMWARE_GIT_HISTORY` is enabled because those commands are short-lived processes - `node space set CUSTOMWARE_PATH=` should be run before creating users or groups when writable state should live outside the source checkout, because `user` and `group` commands resolve that stored parameter before deciding where `L1` and `L2` files belong - `node space supervise` requires `CUSTOMWARE_PATH` and uses it as the stable writable state boundary across source-release swaps diff --git a/server/router/AGENTS.md b/server/router/AGENTS.md index b12317c9..65301ded 100644 --- a/server/router/AGENTS.md +++ b/server/router/AGENTS.md @@ -101,6 +101,12 @@ Direct app-file fetches: - read permission checks are delegated to `createAppAccessController(...)` - `.git` metadata paths are blocked even when they live inside a readable writable-layer owner root +Proxy: + +- `proxy.js` owns the outbound `/api/proxy` fetch transport plus header allow-listing for the upstream and response sides +- the proxy applies a configurable wall-clock timeout to the upstream fetch via `PROXY_REQUEST_TIMEOUT_SECONDS` so long upstream prompt-prefill phases on local LLM servers are not cut off by the runtime fetch implementation's tight `headersTimeout` default; the param accepts `0` to disable the proxy-imposed timeout entirely, and timeout aborts surface to the client as `504 Gateway Timeout` with a hint to raise or disable the param +- the proxy passes `runtimeParams` from the request context into `proxyExternalRequest(...)` so the timeout is resolved per-request and operators can change it at runtime via the params interface without restarting + Responses: - `responses.js` owns JSON serialization, redirects, file responses, stream responses, and Web `Response` bridging diff --git a/server/router/proxy.js b/server/router/proxy.js index 45600e3a..315c90e7 100644 --- a/server/router/proxy.js +++ b/server/router/proxy.js @@ -42,6 +42,39 @@ const PROXY_RESPONSE_TARGET_HEADER = "x-space-proxy-target-url"; const PROXY_RESPONSE_FINAL_HEADER = "x-space-proxy-final-url"; const PROXY_RESPONSE_REDIRECTED_HEADER = "x-space-proxy-redirected"; +// Default upstream-fetch timeout for `/api/proxy` calls, in seconds. The +// runtime fetch implementation (undici) imposes its own `headersTimeout` of +// 300 seconds by default, which is too tight for upstream services that +// stall before producing response headers — most importantly long +// prompt-prefill phases on local LLM servers (llama.cpp, qwen serve, vLLM) +// where the upstream stays silent for minutes during PP on long-context +// conversations. Raise the default to 15 minutes so cold-cache replies are +// not cut off by the proxy itself; operators can shorten it with +// `PROXY_REQUEST_TIMEOUT_SECONDS=N` or disable the timeout entirely with +// `PROXY_REQUEST_TIMEOUT_SECONDS=0`. +const PROXY_REQUEST_TIMEOUT_PARAM_NAME = "PROXY_REQUEST_TIMEOUT_SECONDS"; +const PROXY_REQUEST_TIMEOUT_DEFAULT_SECONDS = 900; + +function resolveProxyRequestTimeoutMs(runtimeParams) { + if (!runtimeParams || typeof runtimeParams.get !== "function") { + return PROXY_REQUEST_TIMEOUT_DEFAULT_SECONDS * 1000; + } + + const rawValue = runtimeParams.get(PROXY_REQUEST_TIMEOUT_PARAM_NAME); + + if (rawValue === undefined || rawValue === null || rawValue === "") { + return PROXY_REQUEST_TIMEOUT_DEFAULT_SECONDS * 1000; + } + + const numericValue = Number(rawValue); + + if (!Number.isFinite(numericValue) || numericValue < 0) { + return PROXY_REQUEST_TIMEOUT_DEFAULT_SECONDS * 1000; + } + + return Math.floor(numericValue * 1000); +} + function getTargetUrl(requestUrl, headers) { return requestUrl.searchParams.get("url") || headers[PROXY_TARGET_HEADER]; } @@ -115,7 +148,7 @@ async function pipeUpstreamBodyToResponse(res, upstreamResponse) { }); } -async function proxyExternalRequest(req, res, requestUrl) { +async function proxyExternalRequest(req, res, requestUrl, options = {}) { const targetUrlValue = getTargetUrl(requestUrl, req.headers); if (!targetUrlValue) { @@ -145,6 +178,18 @@ async function proxyExternalRequest(req, res, requestUrl) { const method = String(req.method || "GET").toUpperCase(); const upstreamHeaders = createUpstreamHeaders(req.headers); const body = requestCanHaveBody(method) ? await readRequestBody(req) : undefined; + + // Resolve the request-scoped abort signal. The optional caller-supplied + // timeout overrides undici's tight `headersTimeout` default so upstream + // services that stall before producing response headers (typically LLM + // servers during long prompt-prefill on warm-cache-miss requests) do not + // get cut off mid-PP. A configured timeout of 0 disables the timeout + // entirely; any other positive value applies as a wall-clock abort. + const timeoutMs = resolveProxyRequestTimeoutMs(options.runtimeParams); + const fetchSignal = timeoutMs > 0 && typeof AbortSignal?.timeout === "function" + ? AbortSignal.timeout(timeoutMs) + : undefined; + let upstreamResponse; try { @@ -152,9 +197,22 @@ async function proxyExternalRequest(req, res, requestUrl) { method, headers: upstreamHeaders, body, - redirect: "follow" + redirect: "follow", + signal: fetchSignal }); } catch (error) { + const isTimeoutError = error?.name === "TimeoutError" || error?.code === "UND_ERR_HEADERS_TIMEOUT"; + + if (isTimeoutError) { + sendProxyError( + res, + 504, + `Upstream did not respond within the configured proxy timeout of ${Math.round(timeoutMs / 1000)}s. ` + + `Increase ${PROXY_REQUEST_TIMEOUT_PARAM_NAME} (or set it to 0 to disable) for slower upstreams.` + ); + return; + } + sendProxyError(res, 502, `Upstream fetch failed: ${error.message}`); return; } @@ -165,4 +223,9 @@ async function proxyExternalRequest(req, res, requestUrl) { await pipeUpstreamBodyToResponse(res, upstreamResponse); } -export { proxyExternalRequest }; +export { + PROXY_REQUEST_TIMEOUT_DEFAULT_SECONDS, + PROXY_REQUEST_TIMEOUT_PARAM_NAME, + proxyExternalRequest, + resolveProxyRequestTimeoutMs +}; diff --git a/server/router/router.js b/server/router/router.js index 7acef055..0e8d9e68 100644 --- a/server/router/router.js +++ b/server/router/router.js @@ -332,7 +332,7 @@ function createRequestHandler(options) { return; } - await proxyExternalRequest(req, res, requestUrl); + await proxyExternalRequest(req, res, requestUrl, { runtimeParams }); return; } diff --git a/tests/proxy_request_timeout_test.mjs b/tests/proxy_request_timeout_test.mjs new file mode 100644 index 00000000..027366ca --- /dev/null +++ b/tests/proxy_request_timeout_test.mjs @@ -0,0 +1,83 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + PROXY_REQUEST_TIMEOUT_DEFAULT_SECONDS, + PROXY_REQUEST_TIMEOUT_PARAM_NAME, + resolveProxyRequestTimeoutMs +} from "../server/router/proxy.js"; + +function createStaticRuntimeParams(values = {}) { + return { + get(name) { + return Object.prototype.hasOwnProperty.call(values, name) ? values[name] : undefined; + } + }; +} + +test("resolveProxyRequestTimeoutMs falls back to the default when no runtime params are provided", () => { + const expectedMs = PROXY_REQUEST_TIMEOUT_DEFAULT_SECONDS * 1000; + + assert.equal(resolveProxyRequestTimeoutMs(undefined), expectedMs); + assert.equal(resolveProxyRequestTimeoutMs(null), expectedMs); + assert.equal(resolveProxyRequestTimeoutMs({}), expectedMs); +}); + +test("resolveProxyRequestTimeoutMs falls back to the default when the param is unset, empty, or non-numeric", () => { + const expectedMs = PROXY_REQUEST_TIMEOUT_DEFAULT_SECONDS * 1000; + + assert.equal(resolveProxyRequestTimeoutMs(createStaticRuntimeParams()), expectedMs); + assert.equal( + resolveProxyRequestTimeoutMs(createStaticRuntimeParams({ [PROXY_REQUEST_TIMEOUT_PARAM_NAME]: "" })), + expectedMs + ); + assert.equal( + resolveProxyRequestTimeoutMs(createStaticRuntimeParams({ [PROXY_REQUEST_TIMEOUT_PARAM_NAME]: "abc" })), + expectedMs + ); + assert.equal( + resolveProxyRequestTimeoutMs(createStaticRuntimeParams({ [PROXY_REQUEST_TIMEOUT_PARAM_NAME]: -10 })), + expectedMs + ); +}); + +test("resolveProxyRequestTimeoutMs honors a positive override value in seconds", () => { + assert.equal( + resolveProxyRequestTimeoutMs(createStaticRuntimeParams({ [PROXY_REQUEST_TIMEOUT_PARAM_NAME]: 1200 })), + 1_200_000 + ); + assert.equal( + resolveProxyRequestTimeoutMs(createStaticRuntimeParams({ [PROXY_REQUEST_TIMEOUT_PARAM_NAME]: "60" })), + 60_000 + ); +}); + +test("resolveProxyRequestTimeoutMs maps 0 to a disabled timeout so operators can turn the guard off", () => { + // A configured `PROXY_REQUEST_TIMEOUT_SECONDS=0` means the proxy should + // make no timeout decision of its own and rely on lower layers (kernel, + // upstream server, manual cancel). The caller checks for `timeoutMs > 0` + // before constructing an `AbortSignal.timeout(...)`. + assert.equal( + resolveProxyRequestTimeoutMs(createStaticRuntimeParams({ [PROXY_REQUEST_TIMEOUT_PARAM_NAME]: 0 })), + 0 + ); + assert.equal( + resolveProxyRequestTimeoutMs(createStaticRuntimeParams({ [PROXY_REQUEST_TIMEOUT_PARAM_NAME]: "0" })), + 0 + ); +}); + +test("resolveProxyRequestTimeoutMs truncates fractional seconds down to whole milliseconds", () => { + assert.equal( + resolveProxyRequestTimeoutMs(createStaticRuntimeParams({ [PROXY_REQUEST_TIMEOUT_PARAM_NAME]: 1.5 })), + 1500 + ); +}); + +test("PROXY_REQUEST_TIMEOUT_DEFAULT_SECONDS is at least one minute past the undici headers-timeout default", () => { + // undici's built-in `headersTimeout` default is 300 seconds. The proxy + // override must stay comfortably above that, otherwise long upstream + // prompt-prefill phases still get cut by the inner client default before + // our explicit timeout takes effect. + assert.ok(PROXY_REQUEST_TIMEOUT_DEFAULT_SECONDS >= 360); +});