mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-03 22:19:42 +02:00
Reported by @DodgyBadger. #237: the proxy wrapped each upstream fetch in a 30s `AbortSignal.timeout`, which bounded the ENTIRE exchange rather than the wait for response headers. A dashboard endpoint doing model inference, and any actively streaming response, both died at 30s as a generic 502 that Codeman never logged, so it read as an intermittent network error. The timeout now bounds time-to-headers only and is cleared the moment headers arrive, so a slow endpoint and a long stream both survive. The default moves to 300s because "the app is thinking" is normal for the dashboards people proxy; abandoned upstreams are reclaimed by the client-hangup abort rather than by this value. A browser that navigates away mid-request now aborts the upstream fetch, guarded by `writableFinished` for the same reason as `abortOnClientHangUp` in session-routes: `close` also fires after a completed response and must not abort anything. Header timeouts are logged as a warning with a sanitized identity (method plus origin plus path, never the query string, which can carry the dashboard's tokens), and a client hangup is deliberately not warned since nobody is listening and it would read as the dashboard being broken. The WebSocket handshake keeps its own 30s budget (`CODEMAN_WEBVIEW_WS_HANDSHAKE_TIMEOUT_MS`), decoupled from the request timeout: a handshake is connection establishment, and waiting minutes on one only delays the browser's reconnect logic. #238: the web-tab guide covered sandboxed dashboards having no cookies, but not cookie authentication in front of Codeman itself (Cloudflare Access and similar), where a sandboxed frame's asset and API requests carry no auth cookie, bounce to the login provider, and leave the embedded app looking unstyled or broken while trusted mode works. Documented, and the Test button's result now says it probes server-to-upstream reachability only, not how the page behaves in a sandboxed frame. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -708,7 +708,10 @@
|
||||
<span class="form-hint">
|
||||
Recommended. A proxied dashboard is served from Codeman's own address, so unchecking
|
||||
this lets its JavaScript read this page and call the API that starts agents. Uncheck
|
||||
only for a dashboard you fully trust, or one whose own login needs cookies.
|
||||
only for a dashboard you fully trust, or one whose own login needs cookies. Also
|
||||
uncheck it if Codeman itself sits behind a cookie-authenticated reverse proxy
|
||||
(e.g. Cloudflare Access): a sandboxed frame carries no auth cookie, so its asset
|
||||
and API requests bounce to the login provider and the page loads broken.
|
||||
</span>
|
||||
</div>
|
||||
<div class="form-row">
|
||||
|
||||
@@ -396,9 +396,13 @@ Object.assign(CodemanApp.prototype, {
|
||||
out.textContent = 'Test failed (invalid URL?).';
|
||||
return;
|
||||
}
|
||||
// #238: the probe runs server-to-upstream; say so, or a passing Test reads as
|
||||
// "the embedded page will work" when the browser sandbox / a cookie-auth
|
||||
// reverse proxy in front of Codeman can still break it.
|
||||
out.textContent = probe.reachable
|
||||
? `Reachable (HTTP ${probe.status}). ${probe.reason}`
|
||||
: `Not reachable. ${probe.reason}`;
|
||||
? `Reachable (HTTP ${probe.status}) from the Codeman server. ${probe.reason} ` +
|
||||
`(Tests server-to-upstream reachability only, not how the page behaves in a sandboxed frame.)`
|
||||
: `Not reachable from the Codeman server. ${probe.reason}`;
|
||||
out.className = 'form-hint webview-probe-result ' + (probe.reachable ? 'ok' : 'bad');
|
||||
},
|
||||
|
||||
|
||||
@@ -43,6 +43,7 @@ import {
|
||||
WEBVIEW_PROBE_TIMEOUT_MS,
|
||||
WEBVIEW_PROXY_PREFIX,
|
||||
WEBVIEW_UPSTREAM_TIMEOUT_MS,
|
||||
WEBVIEW_WS_HANDSHAKE_TIMEOUT_MS,
|
||||
} from '../../config/webview-limits.js';
|
||||
import { readWebviews, writeWebviews } from '../../webview-store.js';
|
||||
import { webviewCapabilities } from '../../webview-capabilities.js';
|
||||
@@ -420,6 +421,34 @@ async function proxyRequest(
|
||||
refererPath: typeof req.headers.referer === 'string' ? stripProxyPrefix(req.headers.referer, cap) : undefined,
|
||||
});
|
||||
|
||||
// #237: the timeout bounds TIME-TO-HEADERS only. A plain AbortSignal.timeout on
|
||||
// the fetch bounded the entire exchange, so a legitimately slow endpoint (AI
|
||||
// inference behind the dashboard) and an actively streaming response both died at
|
||||
// 30s as an unlogged generic 502. The timer is cleared the moment headers arrive;
|
||||
// what reclaims an abandoned upstream afterwards is the client hangup below.
|
||||
const startedAt = Date.now();
|
||||
const abort = new AbortController();
|
||||
let headerTimedOut = false;
|
||||
let clientGone = false;
|
||||
const headerTimer = setTimeout(() => {
|
||||
headerTimedOut = true;
|
||||
abort.abort();
|
||||
}, WEBVIEW_UPSTREAM_TIMEOUT_MS);
|
||||
// A browser that navigates away mid-request (or mid-stream) must abort the
|
||||
// upstream fetch, or slow endpoints accumulate as orphaned upstream sockets.
|
||||
// Guarded by writableFinished, same as abortOnClientHangUp in session-routes:
|
||||
// `close` also fires after a completed response, which must not abort anything.
|
||||
reply.raw.on('close', () => {
|
||||
if (!reply.raw.writableFinished) {
|
||||
clientGone = true;
|
||||
abort.abort();
|
||||
}
|
||||
});
|
||||
|
||||
// Sanitized request identity for logs: method + origin + path, never the query
|
||||
// string (it can carry the dashboard's tokens).
|
||||
const logTarget = `${req.method} ${upstream.origin}${upstream.pathname}`;
|
||||
|
||||
let response: Response;
|
||||
try {
|
||||
response = await fetch(upstream.href, {
|
||||
@@ -431,11 +460,37 @@ async function proxyRequest(
|
||||
// Redirects are rewritten into the proxy prefix instead of followed, so the
|
||||
// browser's URL stays inside the frame and relative assets keep resolving.
|
||||
redirect: 'manual',
|
||||
signal: AbortSignal.timeout(WEBVIEW_UPSTREAM_TIMEOUT_MS),
|
||||
signal: abort.signal,
|
||||
} as RequestInit);
|
||||
} catch (err) {
|
||||
const elapsed = Date.now() - startedAt;
|
||||
if (clientGone) {
|
||||
// Nobody is listening; the abort was ours and intentional. Not an upstream
|
||||
// failure, so no warn (it would read as the dashboard being broken).
|
||||
return reply;
|
||||
}
|
||||
if (headerTimedOut) {
|
||||
console.warn(
|
||||
`[Webview] upstream sent no response headers within ${WEBVIEW_UPSTREAM_TIMEOUT_MS}ms: ` +
|
||||
`${logTarget} (webview "${webview.name}")`
|
||||
);
|
||||
return reply
|
||||
.code(502)
|
||||
.type('text/plain')
|
||||
.send(
|
||||
`Dashboard unreachable: upstream sent no response headers within ${WEBVIEW_UPSTREAM_TIMEOUT_MS}ms ` +
|
||||
`(CODEMAN_WEBVIEW_TIMEOUT_MS raises this limit)`
|
||||
);
|
||||
}
|
||||
const message = err instanceof Error ? err.message : String(err);
|
||||
console.warn(
|
||||
`[Webview] upstream fetch failed after ${elapsed}ms: ${logTarget} (webview "${webview.name}"): ${message}`
|
||||
);
|
||||
return reply.code(502).type('text/plain').send(`Dashboard unreachable: ${message}`);
|
||||
} finally {
|
||||
// Headers arrived (or the fetch failed): from here on the timeout must never
|
||||
// fire, a streaming body is allowed to take as long as it takes.
|
||||
clearTimeout(headerTimer);
|
||||
}
|
||||
|
||||
const secureContext = req.protocol === 'https';
|
||||
@@ -581,7 +636,7 @@ function proxyWebSocket(socket: WebSocket, req: FastifyRequest<{ Params: ProxyPa
|
||||
origin: upstream.origin,
|
||||
...(webview.trusted && req.headers.cookie ? { cookie: String(req.headers.cookie) } : {}),
|
||||
},
|
||||
handshakeTimeout: WEBVIEW_UPSTREAM_TIMEOUT_MS,
|
||||
handshakeTimeout: WEBVIEW_WS_HANDSHAKE_TIMEOUT_MS,
|
||||
}
|
||||
);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user