Skip to content

Commit fcd141c

Browse files
cortinicometa-codesync[bot]
authored andcommitted
Retry stalled SwiftPM artifact downloads (#59214)
Summary: SwiftPM artifact setup currently fails an entire CI lane when a Maven response stalls mid-body. This change: - aborts a download after 60 seconds without body progress; - retries transient transport errors and HTTP 408/429/5xx responses up to three total attempts with short backoff; - keeps deterministic HTTP client errors fail-fast; and - removes partial `.download` files before retrying, so a retry cannot reuse truncated content. ## Changelog: [INTERNAL] [FIXED] - Retry stalled SwiftPM artifact downloads ## Diagnosis `main` commit `f1f516e8cb2f6c085bcc08838066b5b49c08da3b` failed `test_ios_spm_rntester / test (Debug)` in job https://github.com/react/react-native/actions/runs/39353507614/job/118412160607. The Hermes release artifact stopped at 8.0 MB of 24.2 MB and 0 KB/s for about ten minutes before Node fetch reported `terminated`. This is a transport flake rather than a deterministic source regression: - the Release sibling on the same SHA completed successfully; - both SwiftPM RNTester lanes passed on each of the 19 preceding `main` workflow runs (38/38 checks); - the tip commit only changes Fantom SelectableParagraph rendering; and - the exact failed artifact downloaded successfully on a subsequent probe (25,372,145 bytes in 0.29 seconds). Because the failure is nondeterministic and independent of the commit contents, a code bisect cannot identify a meaningful introducing commit. Searches of open PRs and prior CI-monitor runs found no existing fix for this stalled-download signature. PR #59068 mentions the downloader only in its diagnosis and changes different prebuild files. Pull Request resolved: #59214 Test Plan: - `node --check packages/react-native/scripts/spm/download-spm-artifacts.js` - `npx -y prettier@3.9.4 --write packages/react-native/scripts/spm/download-spm-artifacts.js packages/react-native/scripts/spm/__tests__/download-spm-artifacts-test.js` (no changes) - `jest --runInBand packages/react-native/scripts/spm/__tests__/download-spm-artifacts-test.js` — 76/76 tests passed, including transient stream retry, idle-body retry, partial-file cleanup, and fail-fast HTTP 404 coverage - Live curl probe of the exact Hermes artifact — HTTP 200, 25,372,145 bytes ## Risk and limitations A genuinely slow transfer that makes no progress for 60 seconds is restarted, which may add up to roughly two minutes plus backoff before the final failure. The retry is bounded and final errors are still surfaced; deterministic 4xx responses are not retried. Full RNTester/Xcode CI was not reproduced locally because the failure is in remote transport rather than compilation. Reviewed By: cipolleschi Differential Revision: D126318143 Pulled By: cortinico fbshipit-source-id: c413b2278c4ca735e69a1d275f33e74fcea679ca
1 parent 6200203 commit fcd141c

2 files changed

Lines changed: 185 additions & 8 deletions

File tree

‎packages/react-native/scripts/spm/__tests__/download-spm-artifacts-test.js‎

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
'use strict';
1212

1313
const {
14+
download,
1415
exists,
1516
extractXCFramework,
1617
findFirst,
@@ -545,6 +546,125 @@ describe('exists', () => {
545546
});
546547
});
547548

549+
// ---------------------------------------------------------------------------
550+
// download — retries transient transport failures without masking client errors.
551+
// ---------------------------------------------------------------------------
552+
553+
describe('download', () => {
554+
let originalFetch;
555+
let tempDir;
556+
let warnSpy;
557+
558+
beforeEach(() => {
559+
originalFetch = globalThis.fetch;
560+
tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'spm-download-'));
561+
warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
562+
});
563+
564+
afterEach(() => {
565+
globalThis.fetch = originalFetch;
566+
warnSpy.mockRestore();
567+
fs.rmSync(tempDir, {recursive: true, force: true});
568+
});
569+
570+
function downloadResponse(body, error, stall = false) {
571+
const bytes = Buffer.from(body);
572+
return {
573+
ok: true,
574+
status: 200,
575+
statusText: 'OK',
576+
headers: {
577+
get: name =>
578+
name.toLowerCase() === 'content-length' ? String(bytes.length) : null,
579+
},
580+
body: new ReadableStream({
581+
start(controller) {
582+
controller.enqueue(bytes);
583+
if (error != null) {
584+
controller.error(error);
585+
} else if (!stall) {
586+
controller.close();
587+
}
588+
},
589+
}),
590+
};
591+
}
592+
593+
it('retries a transient stream failure and replaces the partial download', async () => {
594+
globalThis.fetch = jest
595+
.fn()
596+
.mockResolvedValueOnce(
597+
downloadResponse('partial', new Error('terminated')),
598+
)
599+
.mockResolvedValueOnce(downloadResponse('complete'));
600+
const destination = path.join(tempDir, 'artifact.tar.gz');
601+
602+
const pending = download(
603+
'https://example/artifact.tar.gz',
604+
destination,
605+
() => {},
606+
);
607+
await jest.advanceTimersByTimeAsync(1_500);
608+
await pending;
609+
610+
expect(globalThis.fetch).toHaveBeenCalledTimes(2);
611+
expect(warnSpy).toHaveBeenCalledTimes(1);
612+
expect(warnSpy).toHaveBeenCalledWith(
613+
expect.stringContaining('retrying in 1000ms'),
614+
);
615+
expect(fs.readFileSync(destination, 'utf8')).toBe('complete');
616+
expect(fs.existsSync(`${destination}.download`)).toBe(false);
617+
});
618+
619+
it('retries when an otherwise healthy response stops making progress', async () => {
620+
jest.useFakeTimers();
621+
try {
622+
globalThis.fetch = jest
623+
.fn()
624+
.mockResolvedValueOnce(downloadResponse('partial', null, true))
625+
.mockResolvedValueOnce(downloadResponse('complete'));
626+
const destination = path.join(tempDir, 'stalled.tar.gz');
627+
628+
const pending = download(
629+
'https://example/stalled.tar.gz',
630+
destination,
631+
() => {},
632+
);
633+
await jest.advanceTimersByTimeAsync(61_500);
634+
await pending;
635+
636+
expect(globalThis.fetch).toHaveBeenCalledTimes(2);
637+
expect(warnSpy).toHaveBeenCalledTimes(1);
638+
expect(warnSpy).toHaveBeenCalledWith(
639+
expect.stringContaining('retrying in 1000ms'),
640+
);
641+
expect(fs.readFileSync(destination, 'utf8')).toBe('complete');
642+
expect(fs.existsSync(`${destination}.download`)).toBe(false);
643+
} finally {
644+
jest.useRealTimers();
645+
}
646+
});
647+
648+
it('does not retry a deterministic HTTP client error', async () => {
649+
globalThis.fetch = jest.fn(async () => ({
650+
ok: false,
651+
status: 404,
652+
statusText: 'Not Found',
653+
headers: {get: () => null},
654+
body: null,
655+
}));
656+
const destination = path.join(tempDir, 'missing.tar.gz');
657+
658+
await expect(
659+
download('https://example/missing.tar.gz', destination, () => {}),
660+
).rejects.toThrow(/HTTP 404/);
661+
662+
expect(globalThis.fetch).toHaveBeenCalledTimes(1);
663+
expect(warnSpy).not.toHaveBeenCalled();
664+
expect(fs.existsSync(destination)).toBe(false);
665+
});
666+
});
667+
548668
// ---------------------------------------------------------------------------
549669
// resolveSnapshotUrl — parses maven-metadata.xml into a timestamped URL.
550670
// ---------------------------------------------------------------------------

‎packages/react-native/scripts/spm/download-spm-artifacts.js‎

Lines changed: 65 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -640,16 +640,26 @@ function createProgressDisplay(
640640
type ProgressCallback = (label: string, downloaded: number, total: number, speed: number, done: boolean, elapsed: number) => void;
641641
*/
642642

643-
async function download(
643+
const DOWNLOAD_MAX_ATTEMPTS = 3;
644+
const DOWNLOAD_RETRY_BASE_DELAY_MS = 1000;
645+
const DOWNLOAD_IDLE_TIMEOUT_MS = 60_000;
646+
647+
function isRetryableDownloadError(error /*: Error */) /*: boolean */ {
648+
// Client errors are deterministic except for request timeout / rate limiting.
649+
// Network and stream errors do not carry an HTTP prefix and are safe to retry.
650+
const httpStatus = /^HTTP (\d{3})\b/.exec(error.message)?.[1];
651+
if (httpStatus == null) {
652+
return true;
653+
}
654+
const status = Number(httpStatus);
655+
return status === 408 || status === 429 || status >= 500;
656+
}
657+
658+
async function downloadOnce(
644659
url /*: string */,
645660
destPath /*: string */,
646661
onProgress /*:: ?: ProgressCallback */,
647662
) /*: Promise<void> */ {
648-
if (fs.existsSync(destPath)) {
649-
log(` Already cached: ${path.basename(destPath)}`);
650-
return;
651-
}
652-
653663
const tmpPath = destPath + '.download';
654664
try {
655665
// $FlowFixMe[incompatible-call] global fetch not in Flow stubs
@@ -662,6 +672,7 @@ async function download(
662672
let downloadedBytes = 0;
663673
let lastPrintTime = Date.now();
664674
let lastPrintBytes = 0;
675+
let lastDataTime = Date.now();
665676
const startTime = Date.now();
666677

667678
const fileStream = fs.createWriteStream(tmpPath);
@@ -719,14 +730,25 @@ async function download(
719730
await new Promise((resolve, reject) => {
720731
let progressInterval;
721732
try {
722-
progressInterval = setInterval(() => reportProgress(), 500);
733+
progressInterval = setInterval(() => {
734+
reportProgress();
735+
if (Date.now() - lastDataTime >= DOWNLOAD_IDLE_TIMEOUT_MS) {
736+
nodeReadable.destroy(
737+
new Error(
738+
`Download made no progress for ${DOWNLOAD_IDLE_TIMEOUT_MS / 1000}s: ${url}`,
739+
),
740+
);
741+
}
742+
}, 500);
723743

724744
nodeReadable
725745
.on('data', chunk => {
726746
downloadedBytes += chunk.length;
747+
lastDataTime = Date.now();
727748
})
728749
.on('error', err => {
729750
clearInterval(progressInterval);
751+
fileStream.destroy();
730752
reject(err);
731753
})
732754
.pipe(fileStream)
@@ -737,10 +759,13 @@ async function download(
737759
})
738760
.on('error', err => {
739761
clearInterval(progressInterval);
762+
nodeReadable.destroy();
740763
reject(err);
741764
});
742765
} catch (err) {
743766
if (progressInterval != null) clearInterval(progressInterval);
767+
nodeReadable.destroy();
768+
fileStream.destroy();
744769
reject(err);
745770
}
746771
});
@@ -753,7 +778,7 @@ async function download(
753778

754779
fs.renameSync(tmpPath, destPath);
755780
} catch (err) {
756-
// Clean up partial .download temp file on failure
781+
// Clean up partial .download temp file on failure.
757782
try {
758783
fs.unlinkSync(tmpPath);
759784
} catch {
@@ -763,6 +788,37 @@ async function download(
763788
}
764789
}
765790

791+
async function download(
792+
url /*: string */,
793+
destPath /*: string */,
794+
onProgress /*:: ?: ProgressCallback */,
795+
) /*: Promise<void> */ {
796+
if (fs.existsSync(destPath)) {
797+
log(` Already cached: ${path.basename(destPath)}`);
798+
return;
799+
}
800+
801+
for (let attempt = 1; attempt <= DOWNLOAD_MAX_ATTEMPTS; attempt++) {
802+
try {
803+
await downloadOnce(url, destPath, onProgress);
804+
return;
805+
} catch (error) {
806+
if (
807+
attempt === DOWNLOAD_MAX_ATTEMPTS ||
808+
!isRetryableDownloadError(error)
809+
) {
810+
throw error;
811+
}
812+
const delayMs = DOWNLOAD_RETRY_BASE_DELAY_MS * attempt;
813+
warn(
814+
` Download failed (${error.message}); retrying in ${delayMs}ms ` +
815+
`(attempt ${attempt + 1}/${DOWNLOAD_MAX_ATTEMPTS})`,
816+
);
817+
await new Promise(resolve => setTimeout(resolve, delayMs));
818+
}
819+
}
820+
}
821+
766822
/**
767823
* Extracts a .tar.gz and returns the path to the first .xcframework found.
768824
*/
@@ -1544,6 +1600,7 @@ module.exports = {
15441600
resolveLatestV1Version,
15451601
resolveRNCoreArtifact,
15461602
resolveRNDepsArtifact,
1603+
download,
15471604
exists,
15481605
formatBytes,
15491606
formatSpeed,

0 commit comments

Comments
 (0)