From be18c7e498697306974b6b973cfa38430db7fff7 Mon Sep 17 00:00:00 2001 From: Trynax Date: Thu, 27 Aug 2026 11:12:35 +0100 Subject: [PATCH] fix(sandbox): enforce hard wait deadlines during polling --- src/client.ts | 30 +++++++++++++++---- src/sandbox-client.ts | 51 ++++++++++++++++++++++---------- src/tests/client.test.ts | 19 ++++++++++++ src/tests/sandbox-client.test.ts | 39 ++++++++++++++++++++++++ 4 files changed, 118 insertions(+), 21 deletions(-) diff --git a/src/client.ts b/src/client.ts index d5b0cda..b45bde8 100644 --- a/src/client.ts +++ b/src/client.ts @@ -100,8 +100,28 @@ function parseRetryAfterMs(res: Response): number | undefined { return Number.isFinite(at) ? Math.max(0, at - Date.now()) : undefined; } -function sleep(ms: number): Promise { - return new Promise((resolve) => setTimeout(resolve, ms)); +function abortError(signal?: AbortSignal | null): Error { + const reason = signal?.reason; + return reason instanceof Error + ? reason + : new DOMException('The operation was aborted', 'AbortError'); +} + +function sleep(ms: number, signal?: AbortSignal | null): Promise { + if (signal?.aborted) return Promise.reject(abortError(signal)); + return new Promise((resolve, reject) => { + let timer: ReturnType | undefined; + const onAbort = () => { + if (timer !== undefined) clearTimeout(timer); + signal?.removeEventListener('abort', onAbort); + reject(abortError(signal)); + }; + timer = setTimeout(() => { + signal?.removeEventListener('abort', onAbort); + resolve(); + }, ms); + signal?.addEventListener('abort', onAbort, { once: true }); + }); } export async function request( @@ -138,7 +158,7 @@ export async function request( if (isRetryableStatus(res.status) && attempt < retries) { await res.text().catch(() => ''); // drain body so the socket can be reused clearTimeout(timer); - await sleep(backoffDelayMs(attempt, retryAfterMs)); + await sleep(backoffDelayMs(attempt, retryAfterMs), callerSignal); attempt++; continue; } @@ -174,7 +194,7 @@ export async function request( if (timedOut) { const timeoutError = new RequestTimeoutError(timeoutMs); if (attempt < retries) { - await sleep(backoffDelayMs(attempt)); + await sleep(backoffDelayMs(attempt), callerSignal); attempt++; continue; } @@ -182,7 +202,7 @@ export async function request( } if (isRetryableNetworkError(e) && attempt < retries) { clearTimeout(timer); - await sleep(backoffDelayMs(attempt)); + await sleep(backoffDelayMs(attempt), callerSignal); attempt++; continue; } diff --git a/src/sandbox-client.ts b/src/sandbox-client.ts index 6b9fc9b..0c2fd36 100644 --- a/src/sandbox-client.ts +++ b/src/sandbox-client.ts @@ -263,24 +263,43 @@ export async function sandboxWait( signal?: AbortSignal, ): Promise { const deadline = Date.now() + timeoutMs; + const deadlineController = new AbortController(); + const abortFromCaller = () => deadlineController.abort(); + const deadlineTimer = setTimeout(() => deadlineController.abort(), Math.max(0, timeoutMs)); + if (signal?.aborted) deadlineController.abort(); + else signal?.addEventListener('abort', abortFromCaller, { once: true }); let last: SandboxDetail | undefined; - while (Date.now() < deadline) { - if (signal?.aborted) throw new Error(`sandbox wait interrupted while waiting for ${wanted.join(' or ')}`); - last = await sandboxGet(opts, id, signal); - const state = String(last.observedState || ''); - if (wanted.includes(state)) return last; - if (['FAILED', 'TERMINATED'].includes(state) && !wanted.includes(state)) { - throw new Error(`sandbox ${id} entered ${state} while waiting for ${wanted.join(' or ')}`); + try { + while (Date.now() < deadline) { + if (signal?.aborted) throw new Error(`sandbox wait interrupted while waiting for ${wanted.join(' or ')}`); + try { + last = await sandboxGet(opts, id, deadlineController.signal); + } catch (error) { + if (signal?.aborted) { + throw new Error(`sandbox wait interrupted while waiting for ${wanted.join(' or ')}`); + } + if (Date.now() >= deadline) break; + throw error; + } + if (Date.now() >= deadline) break; + const state = String(last.observedState || ''); + if (wanted.includes(state)) return last; + if (['FAILED', 'TERMINATED'].includes(state) && !wanted.includes(state)) { + throw new Error(`sandbox ${id} entered ${state} while waiting for ${wanted.join(' or ')}`); + } + await new Promise((resolve) => { + const done = () => { + clearTimeout(timer); + signal?.removeEventListener('abort', done); + resolve(); + }; + const timer = setTimeout(done, Math.min(intervalMs, Math.max(0, deadline - Date.now()))); + signal?.addEventListener('abort', done, { once: true }); + }); } - await new Promise((resolve) => { - const done = () => { - clearTimeout(timer); - signal?.removeEventListener('abort', done); - resolve(); - }; - const timer = setTimeout(done, Math.min(intervalMs, Math.max(0, deadline - Date.now()))); - signal?.addEventListener('abort', done, { once: true }); - }); + } finally { + clearTimeout(deadlineTimer); + signal?.removeEventListener('abort', abortFromCaller); } throw new Error( `sandbox ${id} did not enter ${wanted.join(' or ')} within ${timeoutMs}ms` + diff --git a/src/tests/client.test.ts b/src/tests/client.test.ts index f6c133b..f5b6696 100644 --- a/src/tests/client.test.ts +++ b/src/tests/client.test.ts @@ -59,6 +59,25 @@ describe('client.request', () => { expect(calls).toBe(1); }); + it('interrupts retry backoff when the caller aborts', async () => { + process.env.XAPI_RETRY_BASE_MS = '100'; + let calls = 0; + fetchSpy = mockFetch(async () => { + calls++; + return new Response('busy', { status: 503 }); + }); + const controller = new AbortController(); + const pending = request( + 'https://action.xapi.to/x', + { method: 'GET', signal: controller.signal }, + 5_000, + 2, + ); + controller.abort(); + await expect(pending).rejects.toThrow(/aborted/i); + expect(calls).toBe(1); + }); + it('does NOT retry by default (fail-safe for non-idempotent writes)', async () => { let calls = 0; fetchSpy = mockFetch(async () => { diff --git a/src/tests/sandbox-client.test.ts b/src/tests/sandbox-client.test.ts index d3eb1e1..fdc0324 100644 --- a/src/tests/sandbox-client.test.ts +++ b/src/tests/sandbox-client.test.ts @@ -155,6 +155,45 @@ describe('sandbox client', () => { expect(calls).toBe(3); }); + it('aborts an in-flight state read when the wait deadline expires', async () => { + let calls = 0; + fetchSpy = spyOn(globalThis, 'fetch').mockImplementation(((_url: any, init: any) => { + calls++; + return new Promise((_resolve, reject) => { + init?.signal?.addEventListener( + 'abort', + () => reject(new DOMException('aborted', 'AbortError')), + { once: true }, + ); + }); + }) as any); + await expect(client.sandboxWait( + { sandboxHost: 'sandbox.test.xapi.to', apiKey: 'sk-test' }, + 'box-1', + ['RUNNING'], + 20, + 1, + )).rejects.toThrow(/within 20ms/); + expect(calls).toBe(1); + }); + + it('does not accept a desired state returned after the wait deadline', async () => { + fetchSpy = spyOn(globalThis, 'fetch').mockImplementation((async () => { + await new Promise((resolve) => setTimeout(resolve, 30)); + return new Response(JSON.stringify({ id: 'box-1', observedState: 'RUNNING' }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + }) as any); + await expect(client.sandboxWait( + { sandboxHost: 'sandbox.test.xapi.to', apiKey: 'sk-test' }, + 'box-1', + ['RUNNING'], + 5, + 1, + )).rejects.toThrow(/within 5ms/); + }); + it('interrupts a state wait promptly so callers can clean up', async () => { fetchSpy = spyOn(globalThis, 'fetch').mockResolvedValue(new Response(JSON.stringify({ id: 'box-1', observedState: 'PROVISIONING',