-
Notifications
You must be signed in to change notification settings - Fork 3.1k
fix(inference): relay Expect continuation #10254
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
97c0162
f9847b8
7428684
9bbed32
2c8996f
8d47cd0
4a7e10f
e257e91
ce72726
ab1a33d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ import net from "node:net"; | |
| const SHA256 = /^[a-f0-9]{64}$/u; | ||
| const API_KEY_FILE_DESCRIPTOR = 3; | ||
| const AUTH_MODE = "api-key-fd3"; | ||
| const UPSTREAM_CONTINUE_TIMEOUT_MS = 30_000; | ||
| const UNAUTHORIZED_BODY = `${JSON.stringify({ | ||
| error: { | ||
| code: "unauthorized", | ||
|
|
@@ -153,7 +154,10 @@ function writeUnauthorized(response: http.ServerResponse): void { | |
| response.end(UNAUTHORIZED_BODY); | ||
| } | ||
|
|
||
| function writeUpstreamUnavailable(response: http.ServerResponse): void { | ||
| function writeUpstreamUnavailable( | ||
| response: http.ServerResponse, | ||
| options: { closeConnection?: boolean } = {}, | ||
| ): void { | ||
| if (response.destroyed || response.writableEnded) return; | ||
| if (response.headersSent) { | ||
| response.destroy(); | ||
|
|
@@ -168,14 +172,15 @@ function writeUpstreamUnavailable(response: http.ServerResponse): void { | |
| })}\n`; | ||
| response.writeHead(502, { | ||
| "Cache-Control": "no-store", | ||
| ...(options.closeConnection ? { Connection: "close" } : {}), | ||
| "Content-Length": Buffer.byteLength(body), | ||
| "Content-Type": "application/json", | ||
| "X-Content-Type-Options": "nosniff", | ||
| }); | ||
| response.end(body); | ||
| } | ||
|
|
||
| export function createLlamaCppPrivateBridgeRequestHandler( | ||
| function createLlamaCppPrivateBridgeRequestHandler( | ||
| authority: Pick<LlamaCppPrivateBridgeArguments, "targetHost" | "targetPort">, | ||
| apiKey: string, | ||
| ): http.RequestListener { | ||
|
|
@@ -204,6 +209,15 @@ export function createLlamaCppPrivateBridgeRequestHandler( | |
| delete headers["x-forwarded-host"]; | ||
| delete headers["x-forwarded-proto"]; | ||
|
|
||
| const expectsContinue = request.headers.expect?.toLowerCase() === "100-continue"; | ||
| let upstreamResponded = false; | ||
| let forwardingRequestBody = false; | ||
| let continueTimer: ReturnType<typeof setTimeout> | undefined; | ||
| const clearContinueTimer = () => { | ||
| if (continueTimer === undefined) return; | ||
| clearTimeout(continueTimer); | ||
| continueTimer = undefined; | ||
| }; | ||
| const upstream = http.request( | ||
| { | ||
| headers, | ||
|
|
@@ -213,29 +227,85 @@ export function createLlamaCppPrivateBridgeRequestHandler( | |
| port: targetPort, | ||
| }, | ||
| (upstreamResponse) => { | ||
| clearContinueTimer(); | ||
| upstreamResponded = true; | ||
| request.unpipe(upstream); | ||
| request.resume(); | ||
| response.writeHead(upstreamResponse.statusCode ?? 502, upstreamResponse.headers); | ||
| upstreamResponse.once("error", () => response.destroy()); | ||
| upstreamResponse.pipe(response); | ||
| upstreamResponse.once("end", () => { | ||
| if (!upstream.writableEnded) upstream.destroy(); | ||
| }); | ||
| }, | ||
| ); | ||
| upstream.once("error", () => writeUpstreamUnavailable(response)); | ||
| const forwardRequestBody = () => { | ||
| if (forwardingRequestBody || upstreamResponded) return; | ||
| forwardingRequestBody = true; | ||
| request.pipe(upstream); | ||
| }; | ||
| upstream.once("continue", () => { | ||
| clearContinueTimer(); | ||
| if (!response.destroyed && !response.writableEnded) response.writeContinue(); | ||
| forwardRequestBody(); | ||
| }); | ||
| upstream.once("error", () => { | ||
| clearContinueTimer(); | ||
| if (!upstreamResponded) writeUpstreamUnavailable(response); | ||
| }); | ||
| request.once("close", () => { | ||
| if (!request.complete) upstream.destroy(); | ||
| if (!request.complete && !upstreamResponded) { | ||
| clearContinueTimer(); | ||
| upstream.destroy(); | ||
| } | ||
| }); | ||
| request.once("error", () => { | ||
| if (!upstreamResponded) { | ||
| clearContinueTimer(); | ||
| upstream.destroy(); | ||
| } | ||
| }); | ||
| request.once("error", () => upstream.destroy()); | ||
| response.once("close", () => { | ||
| if (!response.writableEnded) upstream.destroy(); | ||
| if (!response.writableEnded) { | ||
| clearContinueTimer(); | ||
| upstream.destroy(); | ||
| } | ||
| }); | ||
| request.pipe(upstream); | ||
| if (expectsContinue) { | ||
| continueTimer = setTimeout(() => { | ||
| continueTimer = undefined; | ||
| request.unpipe(upstream); | ||
| request.pause(); | ||
| upstream.destroy(); | ||
| response.once("finish", () => request.destroy()); | ||
| writeUpstreamUnavailable(response, { closeConnection: true }); | ||
| }, UPSTREAM_CONTINUE_TIMEOUT_MS); | ||
|
Comment on lines
+275
to
+282
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: #!/bin/bash
set -eu
file='src/lib/onboard/runtime-provider/docker-llama-cpp-private-bridge-process.ts'
printf '%s\n' '--- diff ---'
git diff -- "$file"
printf '%s\n' '--- file outline ---'
ast-grep outline "$file"
printf '%s\n' '--- target context ---'
sed -n '1,80p;190,315p' "$file"
printf '%s\n' '--- direct references and tests ---'
rg -n --glob '!node_modules' 'UPSTREAM_CONTINUE_TIMEOUT_MS|writeUpstreamUnavailable|continueTimer|docker-llama-cpp-private-bridge-process|llama-cpp-private' .Repository: NVIDIA/NemoClaw Length of output: 11014 🌐 Web query:
💡 Result: In Node.js, managing unread request bodies is critical for connection reuse, particularly when using keep-alive connections [1]. When an HTTP/1.1 connection is kept alive, the server must account for the entire request body before it can safely reuse the socket for subsequent requests [1]. If your server receives a request but chooses not to read its body—for example, if you reject a request with an error response like 401 Unauthorized before consuming the data—the unread bytes remain in the incoming stream [1]. Leaving these bytes unread can cause subsequent requests on the same connection to fail or behave unpredictably because the HTTP parser remains out of sync with the actual stream state [1]. To maintain a healthy connection, your application must explicitly discard or consume the unread body (e.g., by calling request.resume or draining the stream) or explicitly close the connection [1]. Regarding the 'checkContinue' event: This event is emitted when the server receives a request with the 'Expect: 100-continue' header [2][3][4]. 1. If you do not listen for 'checkContinue', Node.js automatically sends a '100 Continue' response, signaling the client to proceed with sending the request body [2][3][4]. 2. If you do handle this event, you are responsible for deciding whether to continue [2][3][4]. - Call response.writeContinue if the client should proceed [2][3][4]. - Alternatively, send a different response (such as 400 Bad Request or 401 Unauthorized) if the client should not proceed [2][3][4]. 3. When 'checkContinue' is handled, the standard 'request' event is not emitted [2][5][3][4]. Note that request.resume is often used to switch a stream into flowing mode, which effectively drains and discards any remaining data in the buffer [3][6][4]. Failing to consume the data or explicitly close the connection when a body is present will likely prevent proper keep-alive connection reuse and may lead to memory accumulation [1][3][4]. Citations:
🏁 Script executed: #!/bin/bash
set -eu
file='src/lib/onboard/runtime-provider/docker-llama-cpp-private-bridge.test.ts'
printf '%s\n' '--- response helper and handler entry ---'
sed -n '140,225p' src/lib/onboard/runtime-provider/docker-llama-cpp-private-bridge-process.ts
printf '%s\n' '--- timeout-related tests ---'
rg -n -C 12 'continue|timeout|Expect|keep.alive|keepAlive|same socket|502|unavailable' "$file"Repository: NVIDIA/NemoClaw Length of output: 14251 Drain If an eager client sends a body before upstream confirmation, resume 🤖 Prompt for AI Agents |
||
| continueTimer.unref(); | ||
| upstream.flushHeaders(); | ||
| } else { | ||
| forwardRequestBody(); | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| export function createLlamaCppPrivateBridgeServer( | ||
| authority: Pick<LlamaCppPrivateBridgeArguments, "targetHost" | "targetPort">, | ||
| apiKey: string, | ||
| ): http.Server { | ||
| const handler = createLlamaCppPrivateBridgeRequestHandler(authority, apiKey); | ||
| const server = http.createServer(); | ||
| server.on("checkContinue", handler); | ||
| server.on("request", handler); | ||
| return server; | ||
| } | ||
|
|
||
| export async function runLlamaCppPrivateBridge( | ||
| authority: LlamaCppPrivateBridgeArguments, | ||
| apiKey: string, | ||
| ): Promise<void> { | ||
| const handler = createLlamaCppPrivateBridgeRequestHandler(authority, apiKey); | ||
| const servers = authority.bindAddresses.map(() => http.createServer(handler)); | ||
| const servers = authority.bindAddresses.map(() => | ||
| createLlamaCppPrivateBridgeServer(authority, apiKey), | ||
| ); | ||
|
|
||
| const close = () => { | ||
| for (const server of servers) { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.