Skip to content

Commit 97c04a4

Browse files
crypticsaiyanzeshi-du
crypticsaiyan
and
zeshi-du
authored
fix(bundle): add retry loop to streamUrlToFile for transport failures (#23)
Co-authored-by: zeshi-du <zeshi@testsprite.com>
1 parent 1f37de1 commit 97c04a4

2 files changed

Lines changed: 174 additions & 43 deletions

File tree

‎src/lib/bundle.test.ts‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@
88
* the full http+fetch path is wired against MSW).
99
*/
1010

11+
import { mkdtempSync } from 'node:fs';
12+
import { tmpdir } from 'node:os';
13+
import { join } from 'node:path';
1114
import { describe, expect, it } from 'vitest';
1215
import {
1316
applyFailedOnly,
@@ -16,6 +19,8 @@ import {
1619
buildMeta,
1720
pickCodeExtension,
1821
resolveBundleDir,
22+
STREAM_URL_MAX_RETRIES,
23+
streamUrlToFile,
1924
stepFilenamePrefix,
2025
type AssertContextIntegrityOptions,
2126
} from './bundle.js';
@@ -592,3 +597,95 @@ describe('resolveBundleDir', () => {
592597
expect(out).toBe('/tmp/x');
593598
});
594599
});
600+
601+
describe('streamUrlToFile retry', () => {
602+
const noSleep = () => Promise.resolve();
603+
604+
it('succeeds on the first attempt: file written, fetchImpl called once', async () => {
605+
const dir = mkdtempSync(join(tmpdir(), 'stream-test-'));
606+
const dest = join(dir, 'out.bin');
607+
let calls = 0;
608+
const fetchImpl = async () => {
609+
calls++;
610+
return new Response('hello', { status: 200 });
611+
};
612+
await streamUrlToFile('https://example.com/x', dest, fetchImpl as typeof globalThis.fetch, {
613+
sleep: noSleep,
614+
});
615+
expect(calls).toBe(1);
616+
});
617+
618+
it('retries on transport error and succeeds: fetchImpl called twice, file written', async () => {
619+
const dir = mkdtempSync(join(tmpdir(), 'stream-test-'));
620+
const dest = join(dir, 'out.bin');
621+
let calls = 0;
622+
const fetchImpl = async () => {
623+
calls++;
624+
if (calls === 1) throw new Error('ECONNRESET socket hang up');
625+
return new Response('retried-content', { status: 200 });
626+
};
627+
await streamUrlToFile('https://example.com/x', dest, fetchImpl as typeof globalThis.fetch, {
628+
sleep: noSleep,
629+
});
630+
expect(calls).toBe(2);
631+
});
632+
633+
it('throws TransportError after all retries exhausted', async () => {
634+
let calls = 0;
635+
const fetchImpl = async () => {
636+
calls++;
637+
throw new Error('ENETUNREACH dns lookup failed');
638+
};
639+
await expect(
640+
streamUrlToFile(
641+
'https://example.com/x',
642+
'/tmp/will-not-be-written',
643+
fetchImpl as typeof globalThis.fetch,
644+
{ sleep: noSleep },
645+
),
646+
).rejects.toMatchObject({
647+
name: 'TransportError',
648+
message: expect.stringContaining('ENETUNREACH'),
649+
});
650+
expect(calls).toBe(STREAM_URL_MAX_RETRIES);
651+
});
652+
653+
it('does NOT retry a non-2xx HTTP response (expired presigned URL)', async () => {
654+
let calls = 0;
655+
const fetchImpl = async () => {
656+
calls++;
657+
return new Response('Forbidden', { status: 403 });
658+
};
659+
await expect(
660+
streamUrlToFile(
661+
'https://example.com/x',
662+
'/tmp/will-not-be-written',
663+
fetchImpl as typeof globalThis.fetch,
664+
{ sleep: noSleep },
665+
),
666+
).rejects.toMatchObject({ code: 'UNAVAILABLE' });
667+
expect(calls).toBe(1);
668+
});
669+
670+
it('sleeps between retries', async () => {
671+
const sleepDelays: number[] = [];
672+
const fetchImpl = async () => {
673+
throw new Error('flaky');
674+
};
675+
await expect(
676+
streamUrlToFile(
677+
'https://example.com/x',
678+
'/tmp/will-not-be-written',
679+
fetchImpl as typeof globalThis.fetch,
680+
{
681+
sleep: ms => {
682+
sleepDelays.push(ms);
683+
return Promise.resolve();
684+
},
685+
},
686+
),
687+
).rejects.toThrow();
688+
expect(sleepDelays).toHaveLength(STREAM_URL_MAX_RETRIES - 1);
689+
expect(sleepDelays.every(d => d > 0)).toBe(true);
690+
});
691+
});

‎src/lib/bundle.ts‎

Lines changed: 77 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,11 @@ import type { FetchImpl } from './http.js';
4646
/** Schema version stamped into `meta.json`. Bumps with the contract. */
4747
export const BUNDLE_SCHEMA_VERSION = 'cli-v1' as const;
4848

49+
/** Max fetch attempts for each presigned URL (initial + retries). */
50+
export const STREAM_URL_MAX_RETRIES = 3;
51+
/** Delay between retries for transient transport failures (ms). */
52+
export const STREAM_URL_RETRY_DELAY_MS = 1000;
53+
4954
/** Default radius around the failed step when `--failed-only` is set. */
5055
export const FAILED_ONLY_RADIUS = 1;
5156

@@ -695,55 +700,84 @@ function sidecarExtension(kind: 'log' | 'network' | 'console' | 'screenshot' | '
695700
* the upstream reader rather than buffering chunks in V8 heap. This
696701
* matters for video files (multi-MB) and for very large HTML
697702
* snapshots.
703+
*
704+
* Transport failures (network reset, DNS blip, mid-stream EOF) are
705+
* retried up to STREAM_URL_MAX_RETRIES times with a fixed delay.
706+
* Presigned URLs are valid for 15 minutes, so retries are safe.
698707
*/
699708
export async function streamUrlToFile(
700709
url: string,
701710
filePath: string,
702711
fetchImpl: FetchImpl,
712+
deps?: { sleep?: (ms: number) => Promise<void> },
703713
): Promise<void> {
704-
let response: Response;
705-
try {
706-
response = await fetchImpl(url);
707-
} catch (err) {
708-
const message = err instanceof Error ? err.message : String(err);
709-
throw new TransportError(`Failed to download presigned URL ${url}: ${message}`);
710-
}
711-
if (!response.ok) {
712-
throw ApiError.fromEnvelope({
713-
error: {
714-
code: 'UNAVAILABLE',
715-
message: `Failed to download presigned URL (HTTP ${response.status}).`,
716-
nextAction:
717-
'Re-run `testsprite test failure get`. Presigned URLs in the bundle expire after 15 minutes.',
718-
requestId: 'local',
719-
details: { status: response.status, url },
720-
},
721-
});
722-
}
723-
if (!response.body) {
724-
// Some test runtimes / fetch polyfills don't expose `body` as a
725-
// ReadableStream. Fall back to a buffered write — same correctness,
726-
// just no streaming benefit. The bundle is bounded by the
727-
// backend's 15-min TTL, so even a multi-MB video buffers fully in
728-
// a tolerable amount of memory.
729-
const buffer = Buffer.from(await response.arrayBuffer());
730-
await writeFile(filePath, buffer);
731-
return;
732-
}
733-
await mkdir(dirname(filePath), { recursive: true });
734-
// `response.body` is a Web ReadableStream. Node's `pipeline` accepts
735-
// it via `Readable.fromWeb` (Node ≥ 18). Wrap in a try so any error
736-
// from the stream propagates as a TransportError, preserving the
737-
// exit-code contract.
738-
const fileSink = createWriteStream(filePath);
739-
try {
740-
const webBody = response.body as unknown as NodeReadableStream<Uint8Array>;
741-
const { Readable } = await import('node:stream');
742-
const nodeStream = Readable.fromWeb(webBody);
743-
await pipeline(nodeStream, fileSink as unknown as Writable);
744-
} catch (err) {
745-
const message = err instanceof Error ? err.message : String(err);
746-
throw new TransportError(`Failed mid-download of ${url}: ${message}`);
714+
const sleepFn = deps?.sleep ?? ((ms: number) => new Promise<void>(r => setTimeout(r, ms)));
715+
for (let attempt = 1; attempt <= STREAM_URL_MAX_RETRIES; attempt++) {
716+
let response: Response;
717+
try {
718+
response = await fetchImpl(url);
719+
} catch (err) {
720+
const message = err instanceof Error ? err.message : String(err);
721+
if (attempt < STREAM_URL_MAX_RETRIES) {
722+
await sleepFn(STREAM_URL_RETRY_DELAY_MS);
723+
continue;
724+
}
725+
throw new TransportError(`Failed to download presigned URL ${url}: ${message}`);
726+
}
727+
if (!response.ok) {
728+
// Non-2xx: the URL itself is bad (expired, unauthorized, not found).
729+
// Retrying the same URL won't help — surface immediately.
730+
throw ApiError.fromEnvelope({
731+
error: {
732+
code: 'UNAVAILABLE',
733+
message: `Failed to download presigned URL (HTTP ${response.status}).`,
734+
nextAction:
735+
'Re-run `testsprite test failure get`. Presigned URLs in the bundle expire after 15 minutes.',
736+
requestId: 'local',
737+
details: { status: response.status, url },
738+
},
739+
});
740+
}
741+
if (!response.body) {
742+
// Some test runtimes / fetch polyfills don't expose `body` as a
743+
// ReadableStream. Fall back to a buffered write — same correctness,
744+
// just no streaming benefit. The bundle is bounded by the
745+
// backend's 15-min TTL, so even a multi-MB video buffers fully in
746+
// a tolerable amount of memory.
747+
try {
748+
const buffer = Buffer.from(await response.arrayBuffer());
749+
await writeFile(filePath, buffer);
750+
return;
751+
} catch (err) {
752+
const message = err instanceof Error ? err.message : String(err);
753+
if (attempt < STREAM_URL_MAX_RETRIES) {
754+
await sleepFn(STREAM_URL_RETRY_DELAY_MS);
755+
continue;
756+
}
757+
throw new TransportError(`Failed to download presigned URL ${url}: ${message}`);
758+
}
759+
}
760+
await mkdir(dirname(filePath), { recursive: true });
761+
// `response.body` is a Web ReadableStream. Node's `pipeline` accepts
762+
// it via `Readable.fromWeb` (Node >= 18). Wrap in a try so any error
763+
// from the stream propagates as a TransportError, preserving the
764+
// exit-code contract. `pipeline` destroys both streams on error, so
765+
// a new WriteStream on retry starts clean.
766+
const fileSink = createWriteStream(filePath);
767+
try {
768+
const webBody = response.body as unknown as NodeReadableStream<Uint8Array>;
769+
const { Readable } = await import('node:stream');
770+
const nodeStream = Readable.fromWeb(webBody);
771+
await pipeline(nodeStream, fileSink as unknown as Writable);
772+
return;
773+
} catch (err) {
774+
const message = err instanceof Error ? err.message : String(err);
775+
if (attempt < STREAM_URL_MAX_RETRIES) {
776+
await sleepFn(STREAM_URL_RETRY_DELAY_MS);
777+
continue;
778+
}
779+
throw new TransportError(`Failed mid-download of ${url}: ${message}`);
780+
}
747781
}
748782
}
749783

0 commit comments

Comments
 (0)