From c5da385f7a45a9d9063d4c3432044e962a800cd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20S=C5=82uszniak?= Date: Wed, 23 Sep 2026 16:37:26 +0200 Subject: [PATCH] refactor(fetcher): drop the Android DownloadManager backend blob-util 0.24.11 fixed the in-process reader that stopped after 8 KB on Android (RonRadtke/react-native-blob-util#475), which was the only reason Android had a backend of its own. Both platforms now share the streaming fallback, so there is one mechanism (the optional background downloader) plus one shared fallback. - peer range raised to >=0.24.11; apps and the lib devDep move to 0.25.0 - delete downloadUrlViaAndroidDownloadManager and the IS_ANDROID branch - delete reassemble32BitCounter: the 32-bit progress overflow came from DownloadManager's cursor column being read with getInt, and the in-process path reports bytes as real numbers - keep the Android cache directory where it is; moving it would orphan every model already downloaded Closes #1401 --- apps/computer-vision/package.json | 2 +- apps/nlp/package.json | 2 +- apps/speech/package.json | 2 +- .../01-fundamentals/02-downloading-models.md | 10 +- .../__tests__/fetcher/androidBackend.test.ts | 58 ++++-- .../__tests__/fetcher/download.test.ts | 2 +- .../__tests__/support/blobUtilMock.ts | 7 +- packages/react-native-executorch/package.json | 4 +- .../src/fetcher/fetcher.ts | 169 ++++-------------- yarn.lock | 41 +++-- 10 files changed, 109 insertions(+), 188 deletions(-) diff --git a/apps/computer-vision/package.json b/apps/computer-vision/package.json index 9804b0fc47..732e124e31 100644 --- a/apps/computer-vision/package.json +++ b/apps/computer-vision/package.json @@ -38,7 +38,7 @@ "expo-router": "~56.2.9", "react": "19.2.3", "react-native": "0.85.3", - "react-native-blob-util": "^0.24.0", + "react-native-blob-util": "^0.25.0", "react-native-drawer-layout": "^4.2.2", "react-native-executorch": "workspace:*", "react-native-gesture-handler": "~2.31.1", diff --git a/apps/nlp/package.json b/apps/nlp/package.json index 37266103e0..783f65528f 100644 --- a/apps/nlp/package.json +++ b/apps/nlp/package.json @@ -37,7 +37,7 @@ "expo-router": "~56.2.9", "react": "19.2.3", "react-native": "0.85.3", - "react-native-blob-util": "^0.24.0", + "react-native-blob-util": "^0.25.0", "react-native-drawer-layout": "^4.2.2", "react-native-executorch": "workspace:*", "react-native-gesture-handler": "~2.31.1", diff --git a/apps/speech/package.json b/apps/speech/package.json index 7cfe26d848..3287d2852f 100644 --- a/apps/speech/package.json +++ b/apps/speech/package.json @@ -28,7 +28,7 @@ "react": "19.2.3", "react-native": "0.85.3", "react-native-audio-api": "0.13.1", - "react-native-blob-util": "^0.24.0", + "react-native-blob-util": "^0.25.0", "react-native-device-info": "^15.0.2", "react-native-drawer-layout": "^4.2.2", "react-native-executorch": "workspace:*", diff --git a/docs/docs/01-fundamentals/02-downloading-models.md b/docs/docs/01-fundamentals/02-downloading-models.md index 66b2ada3ca..87e4c8b51a 100644 --- a/docs/docs/01-fundamentals/02-downloading-models.md +++ b/docs/docs/01-fundamentals/02-downloading-models.md @@ -98,9 +98,7 @@ common needs: isn't reported the same as a small tokenizer. - [**`signal`**](../06-api-reference/interfaces/DownloadOptions.md#signal) — an `AbortSignal` to cancel. The bytes fetched so far are kept so a later download - of the same source resumes instead of restarting (except on Android without - the optional background downloader, where the system `DownloadManager` discards - a cancelled transfer). + of the same source resumes instead of restarting. - [**`forceDownload`**](../06-api-reference/interfaces/DownloadOptions.md#forcedownload) — re-download even when cached. @@ -111,10 +109,8 @@ launches fast: - A file that is already cached resolves immediately — no network round trip. - Concurrent downloads of the same URL are deduplicated into one transfer. -- Without extra dependencies, fetching falls back to what each platform supports - natively: the system `DownloadManager` on Android (which continues in the - background), and a streaming request on iOS (which pauses when the app is - suspended and resumes when reopened). +- Without extra dependencies, both platforms fall back to the same streaming + request, which pauses when the app is suspended and resumes when reopened. - To keep transfers running in the background across both iOS and Android and survive the app being killed, install the optional peer dependency [`@kesha-antonov/react-native-background-downloader`](https://github.com/kesha-antonov/react-native-background-downloader) diff --git a/packages/react-native-executorch/__tests__/fetcher/androidBackend.test.ts b/packages/react-native-executorch/__tests__/fetcher/androidBackend.test.ts index 2b8bdd95ee..e4cf3654ce 100644 --- a/packages/react-native-executorch/__tests__/fetcher/androidBackend.test.ts +++ b/packages/react-native-executorch/__tests__/fetcher/androidBackend.test.ts @@ -1,12 +1,18 @@ /** * The Android download path. * - * `src/fetcher/fetcher.ts` decides between the two backends once, at module - * scope (`const IS_ANDROID = Platform.OS === 'android'`), so exercising the - * Android branch means re-importing the module with a different `Platform`. - * That also re-instantiates the blob-util mock, so every handle used here has - * to come from the same fresh module registry — hence the `load()` helper - * rather than the file-level imports the other fetcher suites use. + * Android used to have a backend of its own — the system `DownloadManager` — + * because blob-util's in-process reader stopped after 8 KB + * (RonRadtke/react-native-blob-util#475). That fix shipped in 0.24.11, so both + * platforms now share the streaming fallback, and what is left that is specific + * to Android is the cache directory. + * + * `src/fetcher/fetcher.ts` derives that directory once, at module scope + * (`const IS_ANDROID = Platform.OS === 'android'`), so exercising the Android + * branch means re-importing the module with a different `Platform`. That also + * re-instantiates the blob-util mock, so every handle used here has to come from + * the same fresh module registry — hence the `load()` helper rather than the + * file-level imports the other fetcher suites use. */ import type { Route } from '../support/blobUtilMock'; @@ -16,8 +22,12 @@ const HF_COUNTER = 'https://huggingface.co/software-mansion/model/resolve/main/c type Android = { download: typeof import('../../src/fetcher/fetcher').download; serve: (url: string, route?: Route) => void; + requests: () => readonly { method: string; url: string; headers: Record }[]; paths: () => string[]; readText: (path: string) => string | undefined; + write: (path: string, contents: string) => void; + remove: (path: string) => void; + has: (path: string) => boolean; countRequests: (method: string, url: string) => number; }; @@ -49,8 +59,12 @@ const load = async (): Promise => { return { download, serve: blobUtil.fakeNet.serve, + requests: blobUtil.fakeNet.requests, paths: blobUtil.fakeFs.paths, readText: blobUtil.fakeFs.readText, + write: blobUtil.fakeFs.write, + remove: blobUtil.fakeFs.remove, + has: blobUtil.fakeFs.has, countRequests: blobUtil.fakeNet.countRequests, }; }; @@ -71,30 +85,36 @@ describe('download on Android', () => { expect(android.readText(path)).toBe('model-bytes'); }); - it('downloads through a temporary file and moves it into place', async () => { + it('resumes from a partial file with a Range request, like iOS', async () => { const android = await load(); - android.serve(URL_A); + android.serve(URL_A, { body: 'abcdefgh' }); - await android.download(URL_A); - - expect(android.paths().filter((p) => p.endsWith('.downloading'))).toEqual([]); - }); + // Stage the aftermath of an interrupted download: the cached file is gone + // and `partial` bytes sit next to it. DownloadManager never resumed through + // a Range request, so this is what proves the shared backend is in use. + const path = await android.download(URL_A); + android.remove(path); + android.write(`${path}.partial`, 'abc'); + const before = android.requests().length; - it('treats an empty response as a failure, since DownloadManager reports no status', async () => { - const android = await load(); - android.serve(URL_A, { body: '' }); + await android.download(URL_A); - await expect(android.download(URL_A)).rejects.toThrow(/empty response/); - expect(android.paths()).toEqual([]); + const ranged = android + .requests() + .slice(before) + .find((r) => r.headers.Range !== undefined); + expect(ranged?.headers.Range).toBe('bytes=3-'); + expect(android.readText(path)).toBe('abcdefgh'); + expect(android.has(`${path}.partial`)).toBe(false); }); - it('does not send a Range header — DownloadManager resumes on its own', async () => { + it('leaves no temporary files behind on success', async () => { const android = await load(); android.serve(URL_A); await android.download(URL_A); - expect(android.countRequests('GET', URL_A)).toBe(1); + expect(android.paths().filter((p) => /\.(partial|chunk|downloading)$/.test(p))).toEqual([]); }); it('serves a second call from the cache', async () => { diff --git a/packages/react-native-executorch/__tests__/fetcher/download.test.ts b/packages/react-native-executorch/__tests__/fetcher/download.test.ts index 53f480a8b8..003ac5180a 100644 --- a/packages/react-native-executorch/__tests__/fetcher/download.test.ts +++ b/packages/react-native-executorch/__tests__/fetcher/download.test.ts @@ -356,7 +356,7 @@ describe('download — concurrent callers', () => { }); }); -describe('download — iOS resume', () => { +describe('download — resume', () => { /** * Stages the aftermath of an interrupted download: the cached file is gone * and `partial` bytes are sitting next to it. The cache path is only known diff --git a/packages/react-native-executorch/__tests__/support/blobUtilMock.ts b/packages/react-native-executorch/__tests__/support/blobUtilMock.ts index e4f0efecd5..974bb99eae 100644 --- a/packages/react-native-executorch/__tests__/support/blobUtilMock.ts +++ b/packages/react-native-executorch/__tests__/support/blobUtilMock.ts @@ -210,7 +210,7 @@ type FetchTask = Promise<{ info: () => { status: number } }> & { progress: (config: { count?: number }, cb: ProgressCallback) => FetchTask; // Undocumented in blob-util's typings, but real: it reports the response // state, so `src/` learns the status as soon as the headers land rather than - // only once the body is complete. See `downloadUrlViaIosStream`. + // only once the body is complete. See `downloadUrlViaStream`. stateChange: (cb: StateChangeCallback) => FetchTask; cancel: () => void; }; @@ -223,14 +223,13 @@ class CancelledError extends Error { } type Config = { - /** Destination path for a streamed download (iOS-style). */ + /** Destination path for a streamed download. */ path?: string; fileCache?: boolean; - addAndroidDownloads?: { path?: string; useDownloadManager?: boolean; [key: string]: unknown }; }; function startFetch(config: Config, method: string, url: string, headers: Record) { - const dest = config.addAndroidDownloads?.path ?? config.path; + const dest = config.path; requests.push({ method, url, headers }); let onProgress: ProgressCallback | undefined; diff --git a/packages/react-native-executorch/package.json b/packages/react-native-executorch/package.json index 4f422ddc4a..87a0fe7cd6 100644 --- a/packages/react-native-executorch/package.json +++ b/packages/react-native-executorch/package.json @@ -141,7 +141,7 @@ "@kesha-antonov/react-native-background-downloader": ">=4.4.0", "react": "*", "react-native": "*", - "react-native-blob-util": "^0.24.0", + "react-native-blob-util": ">=0.24.11", "react-native-worklets": ">=0.10.0 <0.13.0" }, "peerDependenciesMeta": { @@ -160,7 +160,7 @@ "jest": "^29.7.0", "react": "19.2.0", "react-native": "0.83.6", - "react-native-blob-util": "^0.24.0", + "react-native-blob-util": "^0.25.0", "react-native-builder-bob": "^0.40.18", "react-native-worklets": "0.10.3", "test-renderer": "^1.2.0", diff --git a/packages/react-native-executorch/src/fetcher/fetcher.ts b/packages/react-native-executorch/src/fetcher/fetcher.ts index 1ae112716d..dc0985c0a6 100644 --- a/packages/react-native-executorch/src/fetcher/fetcher.ts +++ b/packages/react-native-executorch/src/fetcher/fetcher.ts @@ -14,9 +14,11 @@ const IS_ANDROID = Platform.OS === 'android'; // Persistent, per-app directory where downloaded model assets are cached. // iOS: internal DocumentDir (not CacheDir) so the OS won't evict large models // between runs and force a costly re-download. -// Android: the app-private EXTERNAL files dir (getExternalFilesDir), so the -// system DownloadManager can write there and same-volume moves stay cheap -// even for multi-GB files. Falls back to DocumentDir if unmounted. +// Android: the app-private EXTERNAL files dir (getExternalFilesDir). It was +// picked so the system DownloadManager could stage into it; that backend +// is gone, but every model already on disk lives here, so moving the +// cache now would silently orphan all of them and re-download multi-GB +// files. Falls back to DocumentDir if unmounted. const ANDROID_DIRECTORY = RNBlobUtil.fs.dirs.SDCardDir || RNBlobUtil.fs.dirs.DocumentDir; const RNE_DIRECTORY = IS_ANDROID ? `${ANDROID_DIRECTORY}/react-native-executorch` @@ -31,9 +33,7 @@ export interface DownloadOptions { onProgress?: (progress: number) => void; /** * Aborts the download. The bytes fetched so far are kept so a later - * {@link download} of the same source resumes instead of restarting, except on - * Android without the optional background downloader, where the system - * DownloadManager discards a cancelled transfer. + * {@link download} of the same source resumes instead of restarting. */ signal?: AbortSignal; /** @@ -176,24 +176,6 @@ async function foldResumedChunkIntoPartial( await RNBlobUtil.fs.unlink(chunkPath).catch(() => {}); } -// DownloadManager's byte counter is 64-bit, but blob-util reads it out of the -// cursor with `getInt`, so what reaches JS is the low 32 bits reinterpreted as -// a signed int: past 2 GB it arrives NEGATIVE and wraps every 4 GB after that. -// Multi-GB LLM models spend most of their download inside that range, which is -// what collapsed their progress bar. The counter only ever grows, so the -// discarded high bits can be rebuilt by counting how often the low ones wrap. -const UINT32 = 0x100000000; -function reassemble32BitCounter(): (raw: number) => number { - let wraps = 0; - let previous = 0; - return (raw) => { - const low = raw < 0 ? raw + UINT32 : raw; - if (low < previous) wraps += 1; - previous = low; - return low + wraps * UINT32; - }; -} - // Reports absolute bytes for one file. `total` is 0 when the transfer does not // know the length yet — the receiver keeps using whatever length it already // had rather than treating the file as complete. @@ -249,8 +231,7 @@ async function downloadUrl(url: string, cb: DownloadUrlCallbacks): Promise 0) cb.onBytes?.(finalSize, finalSize); @@ -347,72 +317,6 @@ function joinDownload(entry: InFlightDownload, cb: DownloadUrlCallbacks): Promis }); } -// Android fallback used when that optional dependency is absent: the system -// DownloadManager streams to app-private external storage. blob-util's -// in-process reader cannot stand in for it — upstream #475 makes that path stop -// after 8 KB — and DownloadManager also handles files larger than 2 GB, keeps -// downloading while the app is in the background or killed, and resumes across -// transient network drops on its own, so no manual Range logic. -async function downloadUrlViaAndroidDownloadManager( - url: string, - dest: string, - cb: DownloadUrlCallbacks -): Promise { - const tmp = `${dest}.downloading`; - await RNBlobUtil.fs.unlink(tmp).catch(() => {}); - - if (cb.signal?.aborted) throw abortError(); - - const task = RNBlobUtil.config({ - addAndroidDownloads: { - useDownloadManager: true, - path: tmp, - notification: false, - mediaScannable: false, - mime: 'application/octet-stream', - }, - }).fetch('GET', url); - - const onAbort = () => task.cancel(); - cb.signal?.addEventListener('abort', onAbort); - - // DownloadManager reports total as -1 until the size is known; pass that on - // as 0 rather than echoing the received count, which would otherwise look - // like a file that is complete at every sample. - const absoluteBytes = reassemble32BitCounter(); - task.progress({ count: 100 }, (received, total) => { - const tot = Number(total); - cb.onBytes?.(absoluteBytes(Number(received)), tot > 0 ? tot : 0); - }); - - try { - await task; - } catch (e) { - await RNBlobUtil.fs.unlink(tmp).catch(() => {}); - throw cb.signal?.aborted ? abortError() : e; - } finally { - cb.signal?.removeEventListener('abort', onAbort); - } - - // DownloadManager doesn't surface an HTTP status; an empty file means failure. - const size = await fileSize(tmp); - if (size <= 0) { - await RNBlobUtil.fs.unlink(tmp).catch(() => {}); - throw RnExecuTorchError('DOWNLOAD_FAILED', `Download of ${url} failed (empty response).`); - } - // A non-empty file is not necessarily a complete one, and DownloadManager - // gives us no status to check. Promoting a short file would cache it under - // its final name forever — the existence-only cache check can't tell the - // difference, and a truncated .pte only fails much later, at load. - const expected = await expectedBytesFor(url, cb.expectedBytes, cb.signal); - if (isTruncated(size, expected)) { - await RNBlobUtil.fs.unlink(tmp).catch(() => {}); - throw incompleteError(url, size, expected); - } - await RNBlobUtil.fs.mv(tmp, dest); - return dest; -} - // One background task per destination file, under an id that stays the same // across app launches: that is what lets a later call adopt a transfer this // process never started. @@ -579,14 +483,19 @@ async function downloadUrlViaBackgroundSession( return dest; } -// iOS fallback used when that optional dependency is absent: blob-util streams -// via the iOS URL session straight to disk. It does NOT survive the app being -// suspended — iOS tears the connection down about a second later — so an -// interrupted transfer is picked up by the next `download` call instead. -// Interrupted downloads resume from a `.partial` file via an HTTP Range request. -// `canResume` is set to `false` on an internal retry to avoid recursing forever -// if partial-file assembly ever fails. -async function downloadUrlViaIosStream( +// The fallback used on either platform when that optional dependency is absent: +// blob-util streams the response straight to disk, in process. It does NOT +// survive the app being suspended — iOS tears the connection down about a +// second later, and Android may kill the process outright — so an interrupted +// transfer is picked up by the next `download` call instead: it resumes from a +// `.partial` file via an HTTP Range request. `canResume` is set to `false` on an +// internal retry to avoid recursing forever if partial-file assembly ever fails. +// +// This path needs react-native-blob-util >=0.24.11 on Android. 0.24.10's reader +// stopped after exactly 8 KB with "Download interrupted" +// (RonRadtke/react-native-blob-util#475), which is why Android used the system +// DownloadManager until that fix shipped. +async function downloadUrlViaStream( url: string, dest: string, cb: DownloadUrlCallbacks, @@ -638,9 +547,9 @@ async function downloadUrlViaIosStream( }); } - // Same granularity as Android. blob-util still floors the rate at one event - // per 250 ms, so this only means a large file advances in ~1% steps instead - // of the 5% ones that made a multi-GB download look stalled between jumps. + // blob-util floors the rate at one event per 250 ms regardless, so this only + // means a large file advances in ~1% steps instead of the 5% ones that made a + // multi-GB download look stalled between jumps. task.progress({ count: 100 }, (received, total) => { const recv = Number(received); const tot = Number(total); @@ -733,14 +642,14 @@ async function downloadUrlViaIosStream( // once as a plain full download so correctness never depends on resume. await RNBlobUtil.fs.unlink(part).catch(() => {}); await RNBlobUtil.fs.unlink(target).catch(() => {}); - if (canResume) return downloadUrlViaIosStream(url, dest, cb, false); + if (canResume) return downloadUrlViaStream(url, dest, cb, false); throw assemblyErr; } if (restart) { await RNBlobUtil.fs.unlink(part).catch(() => {}); await RNBlobUtil.fs.unlink(target).catch(() => {}); - return downloadUrlViaIosStream(url, dest, cb, false); + return downloadUrlViaStream(url, dest, cb, false); } // The transfer can also report success while the body was cut short — a @@ -756,7 +665,7 @@ async function downloadUrlViaIosStream( // start over rather than fail. A fresh transfer that overshoots is a wrong // expectation instead, and falls through to be accepted. await RNBlobUtil.fs.unlink(part).catch(() => {}); - return downloadUrlViaIosStream(url, dest, cb, false); + return downloadUrlViaStream(url, dest, cb, false); } if (isTruncated(assembled, expected)) { // Short: keep the partial so the next call resumes and finishes it. @@ -859,10 +768,8 @@ function substituteRemoteSources(node: T, resolved: ReadonlyMap=4.4.0" react: "*" react-native: "*" - react-native-blob-util: ^0.24.0 + react-native-blob-util: ">=0.24.11" react-native-worklets: ">=0.10.0 <0.13.0" peerDependenciesMeta: "@kesha-antonov/react-native-background-downloader": @@ -17912,7 +17911,7 @@ __metadata: react: "npm:19.2.3" react-native: "npm:0.85.3" react-native-audio-api: "npm:0.13.1" - react-native-blob-util: "npm:^0.24.0" + react-native-blob-util: "npm:^0.25.0" react-native-device-info: "npm:^15.0.2" react-native-drawer-layout: "npm:^4.2.2" react-native-executorch: "workspace:*"