mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-26 10:26:26 +01:00
fix(adapter-utils): bound the HTTP/2 bridge response stream lifecycle
The response promise settled only on stream "end", "error", or "aborted". A host that opened a stream and then went silent left the promise pending forever, so the local gateway request hung. Add a finite timer, armed per stream and reusing the existing response timeout budget, that destroys the stream and rejects on expiry. Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
59b064bc3f
commit
72994853ff
2 files changed
+99
-4
No files matched your search
@@ -3346,6 +3346,30 @@ describe("sandbox callback bridge", () => {
|
||||
expect(forwardCalls).toBe(0);
|
||||
}, 15_000);
|
||||
|
||||
it("the generated HTTP/2 gateway fails a request at the response bound instead of hanging when the host never finishes the stream", async () => {
|
||||
// The host settings handshake already bounds the wait before the first
|
||||
// stream opens. This test proves the generated gateway also bounds the
|
||||
// rest of the response lifecycle: a host that opens a stream and then
|
||||
// never sends a body, an error, or an abort must still fail the local
|
||||
// request instead of leaving it pending forever.
|
||||
const bridgeToken = createSandboxCallbackBridgeToken();
|
||||
const gateway = await startHttp2GatewayForTest({
|
||||
bridgeToken,
|
||||
responseTimeoutMs: 100,
|
||||
forwardRequest: () => new Promise(() => {}),
|
||||
});
|
||||
|
||||
const start = Date.now();
|
||||
const response = await fetch(`${gateway.baseUrl}/api/agents/me`, {
|
||||
headers: { authorization: `Bearer ${bridgeToken}` },
|
||||
});
|
||||
expect(Date.now() - start).toBeLessThan(5_000);
|
||||
expect(response.status).toBe(502);
|
||||
await expect(response.json()).resolves.toMatchObject({
|
||||
error: "Timed out waiting for the HTTP/2 bridge response stream to finish.",
|
||||
});
|
||||
}, 15_000);
|
||||
|
||||
it("rejects a request body over maxBodyBytes on the queue path before it writes the queue file", async () => {
|
||||
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-bridge-queue-maxbody-"));
|
||||
cleanupDirs.push(rootDir);
|
||||
@@ -3698,6 +3722,44 @@ describe("sandbox callback bridge", () => {
|
||||
).rejects.toThrow("Bridge response body exceeded the configured size limit.");
|
||||
});
|
||||
|
||||
it("fails a request at the response bound when the host opens a stream but never finishes it", async () => {
|
||||
// The settings handshake already completed by the time this stream
|
||||
// opens, so only the per-stream response timer can end the wait. Before
|
||||
// this timer existed, a host that opened a stream and then went silent
|
||||
// left `forwardRequest` pending forever.
|
||||
const bridgeToken = createSandboxCallbackBridgeToken();
|
||||
const [serverSide, clientSide] = duplexPair();
|
||||
const handle = createHttp2BridgeServer({
|
||||
bridgeToken,
|
||||
forwardRequest: () => new Promise(() => {}),
|
||||
});
|
||||
handle.bindChannel(channelFromDuplex(serverSide));
|
||||
|
||||
const gateway = createSandboxHttp2BridgeGateway({
|
||||
bridgeToken,
|
||||
createConnection: () => clientSide,
|
||||
responseTimeoutMs: 100,
|
||||
});
|
||||
cleanupFns.push(async () => {
|
||||
await gateway.close();
|
||||
serverSide.destroy();
|
||||
clientSide.destroy();
|
||||
});
|
||||
|
||||
const start = Date.now();
|
||||
await expect(
|
||||
gateway.forwardRequest({
|
||||
method: "GET",
|
||||
path: "/api/agents/me",
|
||||
query: "",
|
||||
headers: {},
|
||||
body: Buffer.alloc(0),
|
||||
receivedToken: bridgeToken,
|
||||
}),
|
||||
).rejects.toThrow("Timed out waiting for the HTTP/2 bridge response stream to finish.");
|
||||
expect(Date.now() - start).toBeLessThan(5_000);
|
||||
});
|
||||
|
||||
it("the generated gateway asset opens its first stream only after the host settings frame arrives", async () => {
|
||||
const bridgeToken = createSandboxCallbackBridgeToken();
|
||||
let sawStream = false;
|
||||
|
||||
@@ -1891,10 +1891,13 @@ export interface CreateSandboxHttp2BridgeGatewayOptions {
|
||||
*/
|
||||
onGoaway?: (record: { lastStreamId: number; errorCode: number }) => void;
|
||||
/**
|
||||
* The time bound, in milliseconds, on the wait for the host's first
|
||||
* `remoteSettings` event. The gateway opens no stream before that event
|
||||
* arrives or before this bound passes, whichever comes first. The default
|
||||
* is {@link DEFAULT_BRIDGE_RESPONSE_TIMEOUT_MS}.
|
||||
* The time bound, in milliseconds, applied twice: once on the wait for the
|
||||
* host's first `remoteSettings` event (the gateway opens no stream before
|
||||
* that event arrives or before this bound passes, whichever comes first),
|
||||
* and again, independently, on each opened stream's response lifecycle (an
|
||||
* opened stream that never sends "end", "error", or "aborted" is destroyed
|
||||
* and the request fails at this same bound). The default is
|
||||
* {@link DEFAULT_BRIDGE_RESPONSE_TIMEOUT_MS}.
|
||||
*/
|
||||
responseTimeoutMs?: number;
|
||||
/**
|
||||
@@ -1916,6 +1919,7 @@ function forwardOneHttp2Request(
|
||||
body: Buffer;
|
||||
},
|
||||
maxBodyBytes: number,
|
||||
responseTimeoutMs: number,
|
||||
): Promise<SandboxHttp2BridgeGatewayResponse> {
|
||||
return new Promise((resolve, reject) => {
|
||||
const query = request.query.trim();
|
||||
@@ -1942,8 +1946,22 @@ function forwardOneHttp2Request(
|
||||
const settle = (run: () => void) => {
|
||||
if (settled) return;
|
||||
settled = true;
|
||||
clearTimeout(responseTimer);
|
||||
run();
|
||||
};
|
||||
// The handshake before this call already bounds the wait for the host's
|
||||
// remote settings. This timer bounds the rest of the response lifecycle:
|
||||
// an opened stream that never sends "end", "error", or "aborted" would
|
||||
// otherwise leave this promise pending forever, which hangs the caller.
|
||||
// It is armed for exactly this one stream and cleared the moment the
|
||||
// stream settles any other way.
|
||||
const responseTimer = setTimeout(() => {
|
||||
settle(() => {
|
||||
stream.destroy();
|
||||
reject(new Error("Timed out waiting for the HTTP/2 bridge response stream to finish."));
|
||||
});
|
||||
}, responseTimeoutMs);
|
||||
responseTimer.unref();
|
||||
stream.on("response", (headers) => {
|
||||
const rawStatus = headers[":status"];
|
||||
status = typeof rawStatus === "number" ? rawStatus : Number(rawStatus) || 502;
|
||||
@@ -2056,6 +2074,7 @@ export function createSandboxHttp2BridgeGateway(
|
||||
body: request.body,
|
||||
},
|
||||
maxBodyBytes,
|
||||
responseTimeoutMs,
|
||||
),
|
||||
);
|
||||
},
|
||||
@@ -2527,8 +2546,22 @@ function runHttp2Gateway() {
|
||||
const settle = (run) => {
|
||||
if (settled) return;
|
||||
settled = true;
|
||||
clearTimeout(responseTimer);
|
||||
run();
|
||||
};
|
||||
// The handshake above already bounds the wait for the host's remote
|
||||
// settings. This timer bounds the rest of the response lifecycle: an
|
||||
// opened stream that never sends "end", "error", or "aborted" would
|
||||
// otherwise leave this promise pending forever, which hangs the local
|
||||
// gateway request. It is armed for exactly this one stream and
|
||||
// cleared the moment the stream settles any other way.
|
||||
const responseTimer = setTimeout(function () {
|
||||
settle(() => {
|
||||
stream.destroy();
|
||||
reject(new Error("Timed out waiting for the HTTP/2 bridge response stream to finish."));
|
||||
});
|
||||
}, responseTimeoutMs);
|
||||
responseTimer.unref();
|
||||
stream.on("response", (headers) => {
|
||||
const raw = headers[":status"];
|
||||
status = typeof raw === "number" ? raw : Number(raw) || 502;
|
||||
|
||||
Reference in new issue
Block a user