From 7f4763cf0d2cda7b5147bdcb859b1195c793ddde Mon Sep 17 00:00:00 2001 From: Ali Mohammad Date: Sat, 3 Oct 2026 00:18:01 +0300 Subject: [PATCH] =?UTF-8?q?=EF=BB=BFMake=20the=20http=20timeout=20test=20d?= =?UTF-8?q?eterministic=20with=20a=20fake=20clock=20(#324)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test compared real elapsed time to a real 500 ms timeout (>= 450, < 2000), which failed when load delayed the event loop. It now fakes setTimeout and clearTimeout only, checks the request is pending at 499 ms, rejects with 'timed out after 500ms' at 500 ms, and checks the connection was closed. Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 1 + src/tools/built-in/http.test.ts | 52 +++++++++++++++++++++++++++------ 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2311ce40..37e32e4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,6 +59,7 @@ This section lists what is on `main` and not yet on npm. - Documentation corrections: install commands, CLI command lists, optional peers, durable execution and compaction descriptions, the Status section, reasoning and file-part notes in the providers guide, and the `KVStore` note in the deployment guide now match the code. Ticket ids are gone from user-facing prose. ### Tests +- `http.test.ts` "rejects near the configured timeout" no longer compares real elapsed time to a real timeout (flaky under load, #324): it drives the request timer with a fake clock and checks the request is still pending at 499 ms, rejects with "timed out after 500ms" at 500 ms, and that the connection is closed. - `pack-smoke` allows a 16 MiB unpacked tarball (was 14 MiB): `main` was 45 KB under the old cap and permission modes (N4) went over it; see #316 for shrinking the package instead. - Sandbox egress and the credential broker are tested against a real Docker Engine (M6): a new CI workflow, "Docker Engine" (`.github/workflows/docker.yml`, on pull requests touching the sandbox, broker or docker deploy adapter, on demand and weekly), runs `npm run test:docker` on GitHub's `ubuntu-latest` runner (rootful Engine 28.0.4). `src/security/sandboxEgress.docker.test.ts` checks nine cases of `SubprocessSandbox({ network: { allow }, broker })` with `curlimages/curl`: an allowed HTTPS host answers through the broker, another host gets a 403, bypassing the proxy and raw IPs have no route, outside names do not resolve, the broker injects a credential the container never sees, a container outside the internal network cannot use the gateway listener, an aborted run and `close()` leave no container or network, and a reused non-internal network is refused. The docker deploy image test (`/health` and `/chat`) moved to `src/deploy/adapters/docker.docker.test.ts` and runs in the same job; it was skipped in CI before and passes there. No product defect was found. `*.docker.test.ts` files are excluded from `npm test` and `npm run test:coverage`; they skip without a daemon unless `LOUSHO_DOCKER_TESTS=1` is set (the workflow sets it), which makes a missing daemon fail. docs/workspace-tools.md says what the job checks; CONTRIBUTING.md mentions `npm run test:docker`. diff --git a/src/tools/built-in/http.test.ts b/src/tools/built-in/http.test.ts index dc8c7585..22fedda8 100644 --- a/src/tools/built-in/http.test.ts +++ b/src/tools/built-in/http.test.ts @@ -64,23 +64,57 @@ describe('makeHttpRequest', () => { }); it('rejects near the configured timeout when the server never responds', async () => { - server = http.createServer(() => { + // #324: this used to compare a real elapsed time against the real 500 ms + // timeout (>= 450 and < 2000), which failed whenever a loaded machine + // delayed the event loop. The request timer is now driven by a fake clock + // (only setTimeout/clearTimeout are faked; the sockets stay real), so the + // test pins the exact boundary: still pending 1 ms before the timeout, + // aborted at the timeout, with the documented error. + let serverSawRequest!: () => void; + const requestArrived = new Promise((resolve) => (serverSawRequest = resolve)); + let clientHungUp!: () => void; + const connectionClosed = new Promise((resolve) => (clientHungUp = resolve)); + server = http.createServer((req) => { // Never respond + req.socket.once('close', clientHungUp); + serverSawRequest(); }); const baseUrl = await listen(server); - const start = Date.now(); - await expect( - makeHttpRequest({ + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }); + try { + let settled = false; + const request = makeHttpRequest({ url: baseUrl, method: 'GET', options: { timeout: 500, ...LOCAL }, - }) - ).rejects.toThrow(/timed out/i); - const elapsed = Date.now() - start; + }); + const outcome = request.then( + () => { + settled = true; + }, + (error: unknown) => { + settled = true; + return error; + } + ); + + // The request is in flight (the server has it) and the 500 ms timer is armed. + await requestArrived; - expect(elapsed).toBeGreaterThanOrEqual(450); - expect(elapsed).toBeLessThan(2000); + await vi.advanceTimersByTimeAsync(499); + expect(settled).toBe(false); + + await vi.advanceTimersByTimeAsync(1); + const error = await outcome; + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toMatch(/timed out after 500ms/i); + + // The abort tore the connection down rather than leaving it dangling. + await connectionClosed; + } finally { + vi.useRealTimers(); + } }); it('LOU-V1: the tool passes its abortSignal to fetch and rejects with an AbortError', async () => {