diff --git a/server/src/__tests__/workspace-runtime-start-terminality.test.ts b/server/src/__tests__/workspace-runtime-start-terminality.test.ts index 35e96a6db85..e1a0b7865d1 100644 --- a/server/src/__tests__/workspace-runtime-start-terminality.test.ts +++ b/server/src/__tests__/workspace-runtime-start-terminality.test.ts @@ -120,6 +120,44 @@ describe("managed runtime start terminality", () => { expect(probes).toBeLessThan(12); }); + it("preserves the last HTTP failure when the deadline-clamped probe aborts", async () => { + // The readiness budget (1s) is smaller than the per-probe timeout, so every probe is bounded + // by the deadline. The first probe observes the real failure (503); the second is admitted + // with 1ms left and aborts immediately. That abort says the budget ran out, not that the + // service misbehaved, so it must not overwrite the 503 in the thrown message. + let nowMs = 0; + let probes = 0; + const fetchImpl = (async (_url: string | URL | Request, init?: RequestInit) => { + probes += 1; + if (probes === 1) { + nowMs = 999; + return new Response(null, { status: 503 }); + } + return await new Promise((_resolve, reject) => { + const rejectOnAbort = () => { + nowMs = 1_000; + reject(new Error("The operation was aborted")); + }; + if (init?.signal?.aborted) { + rejectOnAbort(); + return; + } + init?.signal?.addEventListener("abort", rejectOnAbort, { once: true }); + }); + }) as unknown as typeof fetch; + + await expect( + waitForRuntimeServiceReadiness({ + service: { readiness: { type: "http", timeoutSec: 1, intervalMs: 100 } }, + url: "http://127.0.0.1:45439/", + readinessUrl: null, + fetchImpl, + now: () => nowMs, + }), + ).rejects.toThrow("Readiness check failed for http://127.0.0.1:45439/: received HTTP 503"); + expect(probes).toBe(2); + }); + it("hands two concurrent starts distinct ports even when the kernel repeats a candidate", async () => { // The kernel really does hand the same ephemeral port to two `listen(0)` probes once the // first probe socket has closed, which is exactly the reallocation collision in the report. diff --git a/server/src/services/workspace-runtime.ts b/server/src/services/workspace-runtime.ts index 2a553671640..d497fd1feeb 100644 --- a/server/src/services/workspace-runtime.ts +++ b/server/src/services/workspace-runtime.ts @@ -5276,21 +5276,33 @@ export async function waitForRuntimeServiceReadiness(input: { let lastError = "service did not become ready"; let lastCause: unknown; let probes = 0; + let sawServiceFailure = false; while (now() < deadline) { - const probeBudgetMs = Math.max(1, Math.min(RUNTIME_SERVICE_READINESS_PROBE_TIMEOUT_MS, deadline - now())); + const remainingMs = deadline - now(); + // When the readiness budget is what bounds this probe, aborting it only means we ran out of + // time — it is not an observation about the service, so it must not replace one we already have. + const boundedByDeadline = remainingMs <= RUNTIME_SERVICE_READINESS_PROBE_TIMEOUT_MS; + const probeBudgetMs = Math.max(1, Math.min(RUNTIME_SERVICE_READINESS_PROBE_TIMEOUT_MS, remainingMs)); + let probeSignal: AbortSignal | undefined; probes += 1; try { - const response = await fetchImpl(readinessUrl, { signal: AbortSignal.timeout(probeBudgetMs) }); + probeSignal = AbortSignal.timeout(probeBudgetMs); + const response = await fetchImpl(readinessUrl, { signal: probeSignal }); if (response.ok) return; lastError = `received HTTP ${response.status}`; lastCause = undefined; + sawServiceFailure = true; } catch (err) { - lastCause = err; - lastError = err instanceof Error ? err.message : String(err); - // Node fetch hides connection errors behind "fetch failed". Retain the - // transport cause so a refused port is distinguishable from a timeout. - if (err instanceof Error && err.cause instanceof Error) { - lastError += `: ${err.cause.message}`; + const abortedByDeadline = Boolean(probeSignal?.aborted) && boundedByDeadline; + if (!abortedByDeadline || !sawServiceFailure) { + lastCause = err; + lastError = err instanceof Error ? err.message : String(err); + // Node fetch hides connection errors behind "fetch failed". Retain the + // transport cause so a refused port is distinguishable from a timeout. + if (err instanceof Error && err.cause instanceof Error) { + lastError += `: ${err.cause.message}`; + } + sawServiceFailure = true; } } if (now() >= deadline) break;