From 70e52539ce0bb29fed859aa4303cc3357b4baf28 Mon Sep 17 00:00:00 2001 From: Michael Date: Wed, 23 Sep 2026 14:34:44 +0200 Subject: [PATCH 1/2] fix(ios): resume interrupted downloads instead of hanging On iOS, react-native-fs never settles downloadFile() when a transfer stops and iOS can produce resume data. It only calls the optional `resumable` callback, which we never passed. Azure Blob always supports resuming (ETag and byte ranges), so a dropped connection, a request timeout or a stopDownload left the Packs screen spinning forever. Android settles every transfer, which is why it never showed there. - Pass `resumable` and resume the transfer in place. After five interruptions the attempt fails, so the retry loop still takes over. - Wait for the foreground before starting a transfer or the pack lookup. Tokens are stored WHEN_UNLOCKED, and iOS treats transfers started in the background as discretionary. - Hand iOS its background-session completion handler back after each transfer. - Report 100% once the transfer completes, before the SHA-256 pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../unit/services/fileDownloadService.test.ts | 185 +++++++++++++- __tests__/unit/utils/appForeground.test.ts | 61 +++++ jest.setup.ts | 2 + src/services/fileDownloadService/index.ts | 175 ++++--------- .../fileDownloadService/nativeDownload.ts | 235 ++++++++++++++++++ src/services/packDownloadService/index.ts | 5 + src/utils/appForeground.ts | 27 ++ 7 files changed, 556 insertions(+), 134 deletions(-) create mode 100644 __tests__/unit/utils/appForeground.test.ts create mode 100644 src/services/fileDownloadService/nativeDownload.ts create mode 100644 src/utils/appForeground.ts diff --git a/__tests__/unit/services/fileDownloadService.test.ts b/__tests__/unit/services/fileDownloadService.test.ts index 5de5d6fcd..b0402d6b9 100644 --- a/__tests__/unit/services/fileDownloadService.test.ts +++ b/__tests__/unit/services/fileDownloadService.test.ts @@ -6,13 +6,49 @@ jest.mock('react-native-fs', () => ({ moveFile: jest.fn(() => Promise.resolve()), downloadFile: jest.fn(), stopDownload: jest.fn(), + resumeDownload: jest.fn(), + completeHandlerIOS: jest.fn(() => Promise.resolve()), })); +import { AppState, Platform } from 'react-native'; +import type { AppStateStatus } from 'react-native'; import RNFS from 'react-native-fs'; -import { downloadFileWithIntegrityCheck } from '../../../src/services/fileDownloadService'; +import { + MAX_IN_PLACE_RESUMES, + downloadFileWithIntegrityCheck, +} from '../../../src/services/fileDownloadService'; const mockDownloadFile = RNFS.downloadFile as jest.Mock; const mockStopDownload = RNFS.stopDownload as jest.Mock; +const mockResumeDownload = RNFS.resumeDownload as jest.Mock; +const mockCompleteHandlerIOS = RNFS.completeHandlerIOS as jest.Mock; + +// An earlier test restores React Native's AppState mock, which leaves it +// returning undefined under Jest 29; stub it so these tests do not depend on order. +const stubAppStateListeners = () => + jest.spyOn(AppState, 'addEventListener').mockImplementation(() => ({ remove: jest.fn() })); + +interface CapturedDownloadOptions { + resumable?: () => void; +} + +/** A native transfer that settles only when the test says so, exposing RNFS's callbacks. */ +function controllableDownload(jobId: number) { + const control: { + options: CapturedDownloadOptions; + finish: (result: { statusCode: number; bytesWritten: number }) => void; + } = { options: {}, finish: () => {} }; + mockDownloadFile.mockImplementation((options: CapturedDownloadOptions) => { + control.options = options; + return { + jobId, + promise: new Promise(resolve => { + control.finish = resolve; + }), + }; + }); + return control; +} const target = { source: { @@ -121,7 +157,6 @@ describe('background transfer', () => { }); it('re-arms the inactivity deadline when the app returns to the foreground', async () => { - const { AppState } = require('react-native'); const addEventListener = jest.spyOn(AppState, 'addEventListener'); mockDownloadFile.mockReturnValue({ jobId: 1, @@ -136,3 +171,149 @@ describe('background transfer', () => { addEventListener.mockRestore(); }); }); + +// RNFS on iOS neither resolves nor rejects a transfer that stopped with resume +// data -- it only calls `resumable`. Azure Blob always allows resuming, so any +// interruption used to leave the Packs screen spinning forever. +describe('interrupted iOS transfers', () => { + beforeEach(() => { + jest.clearAllMocks(); + jest.useFakeTimers(); + stubAppStateListeners(); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it('resumes an interrupted transfer in place and completes it', async () => { + const download = controllableDownload(7); + + const pending = downloadFileWithIntegrityCheck(target, { maxAttempts: 1 }); + await jest.advanceTimersByTimeAsync(0); + download.options.resumable?.(); + await jest.advanceTimersByTimeAsync(0); + + expect(mockResumeDownload).toHaveBeenCalledWith(7); + download.finish({ statusCode: 200, bytesWritten: 1000 }); + await expect(pending).resolves.toMatchObject({ ok: true }); + expect(mockDownloadFile).toHaveBeenCalledTimes(1); + }); + + it(`fails instead of hanging after ${MAX_IN_PLACE_RESUMES} resumes`, async () => { + const download = controllableDownload(7); + + const pending = downloadFileWithIntegrityCheck(target, { maxAttempts: 1 }); + await jest.advanceTimersByTimeAsync(0); + for (let interruption = 0; interruption <= MAX_IN_PLACE_RESUMES; interruption++) { + download.options.resumable?.(); + await jest.advanceTimersByTimeAsync(0); + } + + await expect(pending).resolves.toMatchObject({ + ok: false, + code: 'network-error', + message: `download interrupted ${MAX_IN_PLACE_RESUMES + 1} times`, + }); + expect(mockResumeDownload).toHaveBeenCalledTimes(MAX_IN_PLACE_RESUMES); + }); + + it('does not resume a transfer that was deliberately stopped', async () => { + const download = controllableDownload(7); + const controller = new AbortController(); + + const pending = downloadFileWithIntegrityCheck(target, { + maxAttempts: 1, + signal: controller.signal, + }); + await jest.advanceTimersByTimeAsync(0); + controller.abort(); + // iOS reports the stop itself as resumable. + download.options.resumable?.(); + + await expect(pending).resolves.toMatchObject({ ok: false, code: 'cancelled' }); + expect(mockStopDownload).toHaveBeenCalledWith(7); + expect(mockResumeDownload).not.toHaveBeenCalled(); + }); + + it('reports completion before hashing so the screen can show verification', async () => { + mockDownloadFile.mockReturnValue({ + jobId: 1, + promise: Promise.resolve({ statusCode: 200, bytesWritten: 1000 }), + }); + const onProgress = jest.fn(); + + await downloadFileWithIntegrityCheck(target, { onProgress }); + + expect(onProgress).toHaveBeenLastCalledWith(1000, 1000); + }); +}); + +describe('iOS app lifecycle', () => { + const appState = AppState as unknown as { currentState: unknown }; + const originalAppState = appState.currentState; + const originalPlatformOs = Object.getOwnPropertyDescriptor(Platform, 'OS'); + + beforeEach(() => { + jest.clearAllMocks(); + stubAppStateListeners(); + }); + + afterEach(() => { + appState.currentState = originalAppState; + if (originalPlatformOs) { + Object.defineProperty(Platform, 'OS', originalPlatformOs); + } + jest.restoreAllMocks(); + }); + + it('hands iOS its background-session completion handler back after a transfer', async () => { + mockDownloadFile.mockReturnValue({ + jobId: 9, + promise: Promise.resolve({ statusCode: 200, bytesWritten: 1000 }), + }); + + await downloadFileWithIntegrityCheck(target); + await Promise.resolve(); + + expect(mockCompleteHandlerIOS).toHaveBeenCalledWith(9); + }); + + it('does not call the iOS-only completion handler on Android', async () => { + Object.defineProperty(Platform, 'OS', { configurable: true, get: () => 'android' }); + mockDownloadFile.mockReturnValue({ + jobId: 9, + promise: Promise.resolve({ statusCode: 200, bytesWritten: 1000 }), + }); + + await downloadFileWithIntegrityCheck(target); + await Promise.resolve(); + + expect(mockCompleteHandlerIOS).not.toHaveBeenCalled(); + }); + + it('starts a transfer only once a backgrounded iOS app is back in the foreground', async () => { + const listeners: Array<(state: AppStateStatus) => void> = []; + jest + .spyOn(AppState, 'addEventListener') + .mockImplementation((_type, listener: (state: AppStateStatus) => void) => { + listeners.push(listener); + return { remove: jest.fn() }; + }); + appState.currentState = 'background'; + mockDownloadFile.mockReturnValue({ + jobId: 3, + promise: Promise.resolve({ statusCode: 200, bytesWritten: 1000 }), + }); + + const pending = downloadFileWithIntegrityCheck(target, { maxAttempts: 1 }); + await new Promise(resolve => setImmediate(resolve)); + expect(mockDownloadFile).not.toHaveBeenCalled(); + + appState.currentState = 'active'; + listeners.forEach(listener => listener('active')); + + await expect(pending).resolves.toMatchObject({ ok: true }); + expect(mockDownloadFile).toHaveBeenCalledTimes(1); + }); +}); diff --git a/__tests__/unit/utils/appForeground.test.ts b/__tests__/unit/utils/appForeground.test.ts new file mode 100644 index 000000000..f68cc2347 --- /dev/null +++ b/__tests__/unit/utils/appForeground.test.ts @@ -0,0 +1,61 @@ +import { AppState, Platform } from 'react-native'; +import type { AppStateStatus } from 'react-native'; +import { waitForForeground } from '../../../src/utils/appForeground'; + +const appState = AppState as unknown as { currentState: unknown }; +const originalAppState = appState.currentState; +const originalPlatformOs = Object.getOwnPropertyDescriptor(Platform, 'OS'); + +const settled = async (promise: Promise): Promise => { + let done = false; + promise.then(() => { + done = true; + }); + await new Promise(resolve => setImmediate(resolve)); + return done; +}; + +describe('waitForForeground', () => { + afterEach(() => { + appState.currentState = originalAppState; + if (originalPlatformOs) { + Object.defineProperty(Platform, 'OS', originalPlatformOs); + } + jest.restoreAllMocks(); + }); + + it('resolves immediately when the app is already active', async () => { + appState.currentState = 'active'; + + await expect(settled(waitForForeground())).resolves.toBe(true); + }); + + it('waits on iOS until a backgrounded app becomes active, then stops listening', async () => { + const remove = jest.fn(); + let listener: ((state: AppStateStatus) => void) | undefined; + jest + .spyOn(AppState, 'addEventListener') + .mockImplementation((_type, handler: (state: AppStateStatus) => void) => { + listener = handler; + return { remove }; + }); + appState.currentState = 'background'; + + const waiting = waitForForeground(); + await expect(settled(waiting)).resolves.toBe(false); + + listener?.('inactive'); + await expect(settled(waiting)).resolves.toBe(false); + + listener?.('active'); + await expect(settled(waiting)).resolves.toBe(true); + expect(remove).toHaveBeenCalledTimes(1); + }); + + it('never waits on Android, where downloads are not suspended with the app', async () => { + Object.defineProperty(Platform, 'OS', { configurable: true, get: () => 'android' }); + appState.currentState = 'background'; + + await expect(settled(waitForForeground())).resolves.toBe(true); + }); +}); diff --git a/jest.setup.ts b/jest.setup.ts index 5186e26e4..3932ac621 100644 --- a/jest.setup.ts +++ b/jest.setup.ts @@ -255,6 +255,8 @@ jest.mock('react-native-fs', () => ({ promise: Promise.resolve({ statusCode: 200, bytesWritten: 1000 }), })), stopDownload: jest.fn(), + resumeDownload: jest.fn(), + completeHandlerIOS: jest.fn(() => Promise.resolve()), exists: jest.fn(() => Promise.resolve(false)), mkdir: jest.fn(() => Promise.resolve()), unlink: jest.fn(() => Promise.resolve()), diff --git a/src/services/fileDownloadService/index.ts b/src/services/fileDownloadService/index.ts index 4836aec26..09c82d9af 100644 --- a/src/services/fileDownloadService/index.ts +++ b/src/services/fileDownloadService/index.ts @@ -1,4 +1,3 @@ -import { AppState } from 'react-native'; import RNFS from 'react-native-fs'; import type { DownloadErrorCode, @@ -6,6 +5,12 @@ import type { DownloadOutcome, DownloadSource, } from './types'; +import { + failure, + finishBackgroundEvents, + runNativeDownload, +} from './nativeDownload'; +import { waitForForeground } from '../../utils/appForeground'; import logger from '../../utils/logger'; /** @@ -22,7 +27,10 @@ import logger from '../../utils/logger'; const DEFAULT_MAX_ATTEMPTS = 3; const DEFAULT_BASE_BACKOFF_MS = 1000; -export const DEFAULT_DOWNLOAD_INACTIVITY_TIMEOUT_MS = 60_000; +export { + DEFAULT_DOWNLOAD_INACTIVITY_TIMEOUT_MS, + MAX_IN_PLACE_RESUMES, +} from './nativeDownload'; const RETRYABLE_CODES: ReadonlySet = new Set([ 'network-error', @@ -55,12 +63,6 @@ const delay = (ms: number, signal?: AbortSignal): Promise => ); }); -const failure = ( - code: DownloadErrorCode, - message: string, - httpStatus?: number, -): DownloadOutcome => ({ ok: false, code, message, httpStatus }); - const dirnameOf = (path: string): string => path.substring(0, path.lastIndexOf('/')); @@ -72,53 +74,6 @@ async function cleanupStaging(stagingPath: string): Promise { } } -interface DownloadInactivityWatchdog { - promise: Promise; - reset: () => void; - didTimeout: () => boolean; - clear: () => void; -} - -function createDownloadInactivityWatchdog( - timeoutMs: number, - onTimeout: () => void, -): DownloadInactivityWatchdog { - let timer: ReturnType | undefined; - let timedOut = false; - let rejectTimeout: (error: Error) => void = () => {}; - const promise = new Promise((_resolve, reject) => { - rejectTimeout = reject; - }); - const reset = () => { - clearTimeout(timer); - timer = setTimeout(() => { - timedOut = true; - onTimeout(); - rejectTimeout( - new Error(`download received no data for ${timeoutMs / 1000}s`), - ); - }, timeoutMs); - }; - return { - promise, - reset, - didTimeout: () => timedOut, - clear: () => clearTimeout(timer), - }; -} - -function downloadFailure( - error: unknown, - signal: AbortSignal | undefined, - timedOut: boolean, -): DownloadOutcome { - if (signal?.aborted) { - return failure('cancelled', 'download cancelled'); - } - const message = error instanceof Error ? error.message : String(error); - return failure(timedOut ? 'timeout' : 'network-error', message); -} - /** Where a download comes from and where it lands, staged and final. */ export interface DownloadTarget { source: DownloadSource; @@ -126,87 +81,18 @@ export interface DownloadTarget { finalPath: string; } -async function attemptDownload( +interface CompletedTransfer { + statusCode: number; + contentLengthFromServer: number; +} + +async function verifyAndInstall( target: DownloadTarget, + transfer: CompletedTransfer, opts: DownloadOptions, ): Promise { const { source, stagingPath, finalPath } = target; - await RNFS.mkdir(dirnameOf(stagingPath)); - await RNFS.mkdir(dirnameOf(finalPath)); - await cleanupStaging(stagingPath); - - let contentLengthFromServer = 0; - const inactivityTimeoutMs = - opts.inactivityTimeoutMs ?? DEFAULT_DOWNLOAD_INACTIVITY_TIMEOUT_MS; - let jobId = -1; - const stopDownload = () => { - if (jobId >= 0) { - RNFS.stopDownload(jobId); - } - }; - const watchdog = createDownloadInactivityWatchdog( - inactivityTimeoutMs, - stopDownload, - ); - const download = RNFS.downloadFile({ - fromUrl: source.url, - toFile: stagingPath, - headers: source.headers, - progressInterval: 1000, - // A foreground session stops the moment iOS suspends the app, so the screen - // locking part-way through an 80MB model was enough to kill the transfer -- - // no data would arrive, and the watchdog below would correctly but uselessly - // report a stall. Upstream removed the foreground path for this reason - // ("use background downloads exclusively"); the rewrite of this service lost - // the flag while AppDelegate kept handling - // handleEventsForBackgroundURLSession for a session nothing was asking for. - // Ignored on Android, which has its own long-running download path. - background: true, - begin: (res: { contentLength: number }) => { - contentLengthFromServer = res.contentLength; - watchdog.reset(); - }, - progress: (res: { bytesWritten: number; contentLength: number }) => { - watchdog.reset(); - opts.onProgress?.(res.bytesWritten, res.contentLength); - }, - readTimeout: inactivityTimeoutMs, - }); - jobId = download.jobId; - const { promise } = download; - watchdog.reset(); - - // JS timers do not run while iOS has the app suspended, so a watchdog armed - // before suspension fires the instant the app wakes -- against a background - // transfer that may have been progressing the whole time. Re-arm on wake and - // judge inactivity from then, not from whenever the app went away. - const appStateSubscription = AppState.addEventListener('change', nextState => { - if (nextState === 'active') { - watchdog.reset(); - } - }); - - const onAbort = stopDownload; - if (opts.signal?.aborted) { - // The signal aborted before we could listen — an aborted signal never - // fires 'abort' again, so stop the job directly. - onAbort(); - } else { - opts.signal?.addEventListener('abort', onAbort, { once: true }); - } - - let statusCode: number; - try { - const result = await Promise.race([promise, watchdog.promise]); - statusCode = result.statusCode; - } catch (error) { - return downloadFailure(error, opts.signal, watchdog.didTimeout()); - } finally { - watchdog.clear(); - appStateSubscription.remove(); - opts.signal?.removeEventListener('abort', onAbort); - } - + const { statusCode, contentLengthFromServer } = transfer; if (opts.signal?.aborted) { return failure('cancelled', 'download cancelled'); } @@ -225,6 +111,9 @@ async function attemptDownload( `downloaded ${actualSize} bytes, expected ${expectedSize}`, ); } + // Progress events are throttled to one a second and none follows the last + // chunk, so report completion before the SHA-256 pass, which takes seconds. + opts.onProgress?.(actualSize, actualSize); const actualHash = (await RNFS.hash(stagingPath, 'sha256')).toLowerCase(); if (actualHash !== source.expectedSha256.toLowerCase()) { @@ -251,6 +140,28 @@ async function attemptDownload( }; } +async function attemptDownload( + target: DownloadTarget, + opts: DownloadOptions, +): Promise { + const { source, stagingPath, finalPath } = target; + // A retry, or the next artifact after one finished while the phone was + // locked, would otherwise start from the background -- see waitForForeground. + await waitForForeground(); + await RNFS.mkdir(dirnameOf(stagingPath)); + await RNFS.mkdir(dirnameOf(finalPath)); + await cleanupStaging(stagingPath); + + const transfer = await runNativeDownload(source, stagingPath, opts); + try { + return transfer.ok + ? await verifyAndInstall(target, transfer, opts) + : transfer.failure; + } finally { + finishBackgroundEvents(transfer.jobId); + } +} + /** * Download `target.source.url` to `target.stagingPath`, verify its length * and SHA-256, then atomically move it to `target.finalPath`. Retries diff --git a/src/services/fileDownloadService/nativeDownload.ts b/src/services/fileDownloadService/nativeDownload.ts new file mode 100644 index 000000000..de67742b0 --- /dev/null +++ b/src/services/fileDownloadService/nativeDownload.ts @@ -0,0 +1,235 @@ +import { AppState, Platform } from 'react-native'; +import RNFS from 'react-native-fs'; +import { waitForForeground } from '../../utils/appForeground'; +import type { + DownloadErrorCode, + DownloadOptions, + DownloadOutcome, + DownloadSource, +} from './types'; + +export const DEFAULT_DOWNLOAD_INACTIVITY_TIMEOUT_MS = 60_000; + +/** + * In-place resumes one attempt may make before it fails and hands back to the retry loop, + * which starts the next attempt from byte zero. + */ +export const MAX_IN_PLACE_RESUMES = 5; + +export const failure = ( + code: DownloadErrorCode, + message: string, + httpStatus?: number, +): DownloadOutcome => ({ ok: false, code, message, httpStatus }); + +interface DownloadInactivityWatchdog { + promise: Promise; + reset: () => void; + didTimeout: () => boolean; + clear: () => void; +} + +function createDownloadInactivityWatchdog( + timeoutMs: number, + onTimeout: () => void, +): DownloadInactivityWatchdog { + let timer: ReturnType | undefined; + let timedOut = false; + let rejectTimeout: (error: Error) => void = () => {}; + const promise = new Promise((_resolve, reject) => { + rejectTimeout = reject; + }); + const reset = () => { + clearTimeout(timer); + timer = setTimeout(() => { + timedOut = true; + onTimeout(); + rejectTimeout( + new Error(`download received no data for ${timeoutMs / 1000}s`), + ); + }, timeoutMs); + }; + return { + promise, + reset, + didTimeout: () => timedOut, + clear: () => clearTimeout(timer), + }; +} + +function createInterruption(): { + promise: Promise; + fail: (error: Error) => void; +} { + let fail: (error: Error) => void = () => {}; + const promise = new Promise((_resolve, reject) => { + fail = reject; + }); + return { promise, fail }; +} + +function downloadFailure( + error: unknown, + signal: AbortSignal | undefined, + timedOut: boolean, +): DownloadOutcome { + if (signal?.aborted) { + return failure('cancelled', 'download cancelled'); + } + const message = error instanceof Error ? error.message : String(error); + return failure(timedOut ? 'timeout' : 'network-error', message); +} + +export type NativeTransfer = + | { + ok: true; + jobId: number; + statusCode: number; + contentLengthFromServer: number; + } + | { ok: false; jobId: number; failure: DownloadOutcome }; + +/** + * Runs one RNFS transfer to `toFile` and always settles, which RNFS itself does not. + * + * On iOS, when a transfer stops and the server supports resuming it, RNFS neither resolves nor + * rejects: it only calls the optional `resumable` callback. Azure Blob always supports resuming + * (ETag plus byte ranges), so without that callback any dropped connection, request timeout or + * stopDownload left the promise pending forever, and the person saw a spinner that never ended. + * The transfer is now resumed from where it stopped, and fails after MAX_IN_PLACE_RESUMES. + */ +export async function runNativeDownload( + source: DownloadSource, + toFile: string, + opts: DownloadOptions, +): Promise { + const inactivityTimeoutMs = + opts.inactivityTimeoutMs ?? DEFAULT_DOWNLOAD_INACTIVITY_TIMEOUT_MS; + let jobId = -1; + let contentLengthFromServer = 0; + let stopRequested = false; + let settled = false; + let resumes = 0; + const stopDownload = () => { + stopRequested = true; + if (jobId >= 0) { + RNFS.stopDownload(jobId); + } + }; + const watchdog = createDownloadInactivityWatchdog( + inactivityTimeoutMs, + stopDownload, + ); + const interruption = createInterruption(); + const resumeInPlace = () => { + if (settled) { + return; + } + if (stopRequested || resumes >= MAX_IN_PLACE_RESUMES) { + interruption.fail( + new Error( + stopRequested + ? 'download stopped' + : `download interrupted ${resumes + 1} times`, + ), + ); + return; + } + resumes += 1; + waitForForeground() + .then(() => { + if (!settled && !stopRequested) { + watchdog.reset(); + RNFS.resumeDownload(jobId); + } + }) + .catch((error: unknown) => + interruption.fail(error instanceof Error ? error : new Error(String(error))), + ); + }; + const download = RNFS.downloadFile({ + fromUrl: source.url, + toFile, + headers: source.headers, + progressInterval: 1000, + // A foreground session stops the moment iOS suspends the app, so the screen + // locking part-way through an 80MB model was enough to kill the transfer -- + // no data would arrive, and the watchdog below would correctly but uselessly + // report a stall. Upstream removed the foreground path for this reason + // ("use background downloads exclusively"); the rewrite of this service lost + // the flag while AppDelegate kept handling + // handleEventsForBackgroundURLSession for a session nothing was asking for. + // Ignored on Android, which has its own long-running download path. + background: true, + begin: (res: { contentLength: number }) => { + contentLengthFromServer = res.contentLength; + watchdog.reset(); + }, + progress: (res: { bytesWritten: number; contentLength: number }) => { + watchdog.reset(); + opts.onProgress?.(res.bytesWritten, res.contentLength); + }, + resumable: resumeInPlace, + readTimeout: inactivityTimeoutMs, + }); + jobId = download.jobId; + watchdog.reset(); + + // JS timers do not run while iOS has the app suspended, so a watchdog armed + // before suspension fires the instant the app wakes -- against a background + // transfer that may have been progressing the whole time. Re-arm on wake and + // judge inactivity from then, not from whenever the app went away. + const appStateSubscription = AppState.addEventListener('change', nextState => { + if (nextState === 'active') { + watchdog.reset(); + } + }); + + if (opts.signal?.aborted) { + // The signal aborted before we could listen — an aborted signal never + // fires 'abort' again, so stop the job directly. + stopDownload(); + } else { + opts.signal?.addEventListener('abort', stopDownload, { once: true }); + } + + try { + const result = await Promise.race([ + download.promise, + watchdog.promise, + interruption.promise, + ]); + return { + ok: true, + jobId, + statusCode: result.statusCode, + contentLengthFromServer, + }; + } catch (error) { + return { + ok: false, + jobId, + failure: downloadFailure(error, opts.signal, watchdog.didTimeout()), + }; + } finally { + settled = true; + watchdog.clear(); + appStateSubscription.remove(); + opts.signal?.removeEventListener('abort', stopDownload); + } +} + +/** + * When a background transfer finishes while the app is suspended, AppDelegate hands iOS's + * completion handler to RNFS. iOS expects it back once the app has processed the result, and + * deprioritises background wakes for apps that never return it. Does nothing when no handler + * is pending, which is the case for a transfer that finished in the foreground. + */ +export function finishBackgroundEvents(jobId: number): void { + if (Platform.OS !== 'ios' || jobId < 0) { + return; + } + Promise.resolve() + .then(() => RNFS.completeHandlerIOS(jobId)) + .catch(() => {}); +} diff --git a/src/services/packDownloadService/index.ts b/src/services/packDownloadService/index.ts index 36ebc934b..90d1bd314 100644 --- a/src/services/packDownloadService/index.ts +++ b/src/services/packDownloadService/index.ts @@ -19,6 +19,7 @@ import { removeIfExists, } from './candidate'; import type { PreparedPack } from './candidate'; +import { waitForForeground } from '../../utils/appForeground'; import logger from '../../utils/logger'; /** @@ -204,6 +205,10 @@ async function acquireLatestPackInternal( opts: DownloadOptions = {}, preparedModel?: MiewIDModelRecord, ): Promise { + // The model download that precedes this can finish while the phone is + // locked and wake the app in the background, where the token read behind + // getLatestPack fails as "Not signed in" -- see waitForForeground. + await waitForForeground(); const resolved = await ganeshaApiClient.getLatestPack(projectId); if (!resolved.ok) { return resolved; diff --git a/src/utils/appForeground.ts b/src/utils/appForeground.ts new file mode 100644 index 000000000..5afdf20eb --- /dev/null +++ b/src/utils/appForeground.ts @@ -0,0 +1,27 @@ +import { AppState, Platform } from 'react-native'; + +/** + * Resolves once an iOS app that is in the background returns to the foreground. Resolves + * immediately everywhere else, including on Android. + * + * A background URLSession transfer that finishes while the phone is locked wakes the app in + * the background, and whatever was awaiting that transfer carries on. Two things break if the + * next step runs then: + * - the Entra tokens are stored WHEN_UNLOCKED, so the Keychain read fails and the next API + * call reports "Not signed in"; + * - iOS treats a transfer started from the background as discretionary and may defer it + * indefinitely, for example until the phone is charging on Wi-Fi. + */ +export function waitForForeground(): Promise { + if (Platform.OS !== 'ios' || AppState.currentState !== 'background') { + return Promise.resolve(); + } + return new Promise(resolve => { + const subscription = AppState.addEventListener('change', nextState => { + if (nextState === 'active') { + subscription.remove(); + resolve(); + } + }); + }); +} From 233f7193fd540832afd8b511cc314287a97d5384 Mon Sep 17 00:00:00 2001 From: Michael Date: Wed, 23 Sep 2026 14:37:27 +0200 Subject: [PATCH 2/2] feat: show model and pack download progress on the Packs screen "Download Latest Pack" fetches a 194.6 MB model and then a 217.5 MB pack behind a single spinner, so a slow link and a real stall looked the same. Show the stage, the percentage and megabytes, and a hint to keep the app open. The stage line is the live region, so screen readers hear stage changes rather than a new percentage every second. The screen is shared, so Android shows the same text. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- __tests__/rntl/screens/PacksScreen.test.tsx | 62 +++++++++++++-- .../unit/screens/packDownloadProgress.test.ts | 50 ++++++++++++ src/screens/PackDownloadStatus.tsx | 37 +++++++++ src/screens/PacksScreen.styles.ts | 9 +++ src/screens/PacksScreen.tsx | 76 +++++++++++-------- src/screens/packDownloadProgress.ts | 62 +++++++++++++++ 6 files changed, 258 insertions(+), 38 deletions(-) create mode 100644 __tests__/unit/screens/packDownloadProgress.test.ts create mode 100644 src/screens/PackDownloadStatus.tsx create mode 100644 src/screens/packDownloadProgress.ts diff --git a/__tests__/rntl/screens/PacksScreen.test.tsx b/__tests__/rntl/screens/PacksScreen.test.tsx index 63b73ff29..f50e8238f 100644 --- a/__tests__/rntl/screens/PacksScreen.test.tsx +++ b/__tests__/rntl/screens/PacksScreen.test.tsx @@ -149,6 +149,10 @@ const latestModelSource = { format: 'onnx' as const, }; +const withProgress = expect.objectContaining({ onProgress: expect.any(Function) }); + +type ReportProgress = (bytesWritten: number, contentLength: number) => void; + describe('PacksScreen', () => { beforeEach(() => { jest.clearAllMocks(); @@ -364,8 +368,8 @@ describe('PacksScreen', () => { await waitFor(() => expect(getByText('Update available')).toBeTruthy()); fireEvent.press(getByTestId('update-pack-button')); - await waitFor(() => expect(mockAcquireLatestPack).toHaveBeenCalledWith('example-project', {}, readyModel)); - expect(mockPrepareMiewidModel).toHaveBeenCalledWith(latestModelSource); + await waitFor(() => expect(mockAcquireLatestPack).toHaveBeenCalledWith('example-project', withProgress, readyModel)); + expect(mockPrepareMiewidModel).toHaveBeenCalledWith(latestModelSource, withProgress); }, ); @@ -443,7 +447,7 @@ describe('PacksScreen', () => { await waitFor(() => expect(mockAcquireLatestPack).toHaveBeenCalledWith( 'example-project', - {}, + withProgress, readyModel, ), ); @@ -543,7 +547,7 @@ describe('PacksScreen', () => { await waitFor(() => expect(mockAcquireLatestPack).toHaveBeenCalledWith( 'example-project', - {}, + withProgress, readyModel, ), ); @@ -561,7 +565,7 @@ describe('PacksScreen', () => { await waitFor(() => expect(mockAcquireLatestPack).toHaveBeenCalledWith( 'example-project', - {}, + withProgress, readyModel, ), ); @@ -687,11 +691,11 @@ describe('PacksScreen', () => { await waitFor(() => expect(mockAcquireLatestPack).toHaveBeenCalledWith( 'example-project', - {}, + withProgress, readyModel, ), ); - expect(mockPrepareMiewidModel).toHaveBeenCalledWith(latestModelSource); + expect(mockPrepareMiewidModel).toHaveBeenCalledWith(latestModelSource, withProgress); }); it('alerts and stops when resolving the model source fails', async () => { @@ -738,5 +742,49 @@ describe('PacksScreen', () => { await waitFor(() => expect(alertSpy).toHaveBeenCalled()); }); + + it('shows model and then pack progress, with a keep-open hint, while downloading', async () => { + let reportModel: ReportProgress | undefined; + let finishModel: ((record: MiewIDModelRecord) => void) | undefined; + mockPrepareMiewidModel.mockImplementation( + (_source: unknown, opts?: { onProgress?: ReportProgress }) => { + reportModel = opts?.onProgress; + return new Promise(resolve => { + finishModel = resolve; + }); + }, + ); + let reportPack: ReportProgress | undefined; + let finishPack: ((result: { ok: true; pack: EmbeddingPack }) => void) | undefined; + mockAcquireLatestPack.mockImplementation( + (_projectId: string, opts?: { onProgress?: ReportProgress }) => { + reportPack = opts?.onProgress; + return new Promise(resolve => { + finishPack = resolve; + }); + }, + ); + + const { getByTestId, getByText, queryByTestId } = render(); + fireEvent.press(getByTestId('download-pack-button')); + await waitFor(() => expect(reportModel).toBeDefined()); + expect(getByText('Preparing download...')).toBeTruthy(); + + act(() => reportModel?.(85_684_745, 204_011_297)); + expect(getByText('Downloading identification model...')).toBeTruthy(); + expect(getByText('42% (81.7 of 194.6 MB)')).toBeTruthy(); + expect(getByText('Keep EleBook open until this finishes.')).toBeTruthy(); + + await act(async () => finishModel?.(readyModel)); + await waitFor(() => expect(reportPack).toBeDefined()); + expect(getByText('Downloading embedding pack...')).toBeTruthy(); + expect(queryByTestId('pack-download-amount')).toBeNull(); + + act(() => reportPack?.(228_081_202, 228_081_202)); + expect(getByText('Verifying and installing embedding pack...')).toBeTruthy(); + + await act(async () => finishPack?.({ ok: true, pack: createPack() })); + await waitFor(() => expect(queryByTestId('pack-download-status')).toBeNull()); + }); }); }); diff --git a/__tests__/unit/screens/packDownloadProgress.test.ts b/__tests__/unit/screens/packDownloadProgress.test.ts new file mode 100644 index 000000000..5d034ae76 --- /dev/null +++ b/__tests__/unit/screens/packDownloadProgress.test.ts @@ -0,0 +1,50 @@ +import { + describeDownloadAmount, + describeDownloadStage, + progressReporter, +} from '../../../src/screens/packDownloadProgress'; + +const MODEL_BYTES = 204_011_297; +const PACK_BYTES = 228_081_202; + +describe('pack download progress text', () => { + it('describes the preparation step before any transfer has started', () => { + expect(describeDownloadStage(null)).toBe('Preparing download...'); + expect(describeDownloadAmount(null)).toBeNull(); + }); + + it('shows the percentage and 1024-based megabytes, like the pack card', () => { + const progress = { stage: 'model' as const, bytesWritten: 85_684_745, contentLength: MODEL_BYTES }; + + expect(describeDownloadStage(progress)).toBe('Downloading identification model...'); + expect(describeDownloadAmount(progress)).toBe('42% (81.7 of 194.6 MB)'); + }); + + it('switches to verification once every byte has arrived', () => { + const progress = { stage: 'pack' as const, bytesWritten: PACK_BYTES, contentLength: PACK_BYTES }; + + expect(describeDownloadStage(progress)).toBe('Verifying and installing embedding pack...'); + expect(describeDownloadAmount(progress)).toBeNull(); + }); + + it('shows only the received amount when the server sent no length', () => { + const progress = { stage: 'pack' as const, bytesWritten: 12 * 1024 * 1024, contentLength: 0 }; + + expect(describeDownloadStage(progress)).toBe('Downloading embedding pack...'); + expect(describeDownloadAmount(progress)).toBe('12.0 MB'); + }); + + it('hides the amount until the first bytes arrive', () => { + expect( + describeDownloadAmount({ stage: 'pack', bytesWritten: 0, contentLength: PACK_BYTES }), + ).toBeNull(); + }); + + it('tags service progress with its stage', () => { + const onChange = jest.fn(); + + progressReporter('model', onChange)(10, 20); + + expect(onChange).toHaveBeenCalledWith({ stage: 'model', bytesWritten: 10, contentLength: 20 }); + }); +}); diff --git a/src/screens/PackDownloadStatus.tsx b/src/screens/PackDownloadStatus.tsx new file mode 100644 index 000000000..4dcac83e6 --- /dev/null +++ b/src/screens/PackDownloadStatus.tsx @@ -0,0 +1,37 @@ +import React from 'react'; +import { Text, View } from 'react-native'; +import type { ViewStyle } from 'react-native'; +import { useThemedStyles } from '../theme/useThemedStyles'; +import { createStyles } from './PacksScreen.styles'; +import { + KEEP_APP_OPEN_HINT, + describeDownloadAmount, + describeDownloadStage, +} from './packDownloadProgress'; +import type { DownloadProgress } from './packDownloadProgress'; + +interface PackDownloadStatusProps { + progress: DownloadProgress | null; + style?: ViewStyle; +} + +export const PackDownloadStatus: React.FC = ({ + progress, + style, +}) => { + const styles = useThemedStyles(createStyles); + const amount = describeDownloadAmount(progress); + return ( + + + {describeDownloadStage(progress)} + + {amount ? ( + + {amount} + + ) : null} + {KEEP_APP_OPEN_HINT} + + ); +}; diff --git a/src/screens/PacksScreen.styles.ts b/src/screens/PacksScreen.styles.ts index c53f057d4..3f40552d6 100644 --- a/src/screens/PacksScreen.styles.ts +++ b/src/screens/PacksScreen.styles.ts @@ -69,4 +69,13 @@ export const createStyles = (colors: ThemeColors, shadows: ThemeShadows) => ({ marginTop: SPACING.lg, minWidth: 220, }, + downloadStatus: { + marginTop: SPACING.md, + }, + downloadDetail: { + ...TYPOGRAPHY.meta, + color: colors.textMuted, + textAlign: 'center' as const, + marginTop: SPACING.xs, + }, }); diff --git a/src/screens/PacksScreen.tsx b/src/screens/PacksScreen.tsx index 264ab4af5..fe1c6ecc5 100644 --- a/src/screens/PacksScreen.tsx +++ b/src/screens/PacksScreen.tsx @@ -23,6 +23,9 @@ import { import { ensureSignedIn } from '../utils/authGate'; import logger from '../utils/logger'; import { createStyles } from './PacksScreen.styles'; +import { PackDownloadStatus } from './PackDownloadStatus'; +import { progressReporter } from './packDownloadProgress'; +import type { DownloadProgress } from './packDownloadProgress'; type NavigationProp = NativeStackNavigationProp; @@ -37,6 +40,22 @@ type PackUpdateState = | 'available' | 'unavailable'; +const UPDATE_STATUS_TEXT: Record = { + unchecked: 'Update status not checked', + checking: 'Checking for updates...', + current: 'Up to date', + available: 'Update available', + unavailable: 'Unable to check for updates', +}; + +const UPDATE_BUTTON_TITLE: Record = { + unchecked: 'Check for Updates', + checking: 'Checking for Updates', + current: 'Check Again', + available: 'Update to Latest Pack', + unavailable: 'Check for Updates', +}; + function formatBytes(bytes: number): string { if (bytes < MB) { return `${(bytes / KB).toFixed(1)} KB`; @@ -77,6 +96,8 @@ export const PacksScreen: React.FC = () => { const { packs, miewidModel } = useWildlifeStore(); const preferGpuModel = useAppStore((s) => s.preferGpuModel); const [isDownloading, setIsDownloading] = useState(false); + const [downloadProgress, setDownloadProgress] = + useState(null); const [packUpdateState, setPackUpdateState] = useState('unchecked'); const updateInFlight = useRef(false); @@ -154,7 +175,9 @@ export const PacksScreen: React.FC = () => { // replaced before installing a pack from a newer embedding space. let modelForPack = miewidModel; if (!installedModelIsCurrent) { - modelForPack = await prepareMiewidModel(resolvedSource.source); + modelForPack = await prepareMiewidModel(resolvedSource.source, { + onProgress: progressReporter('model', setDownloadProgress), + }); if (modelForPack.status !== 'ready') { Alert.alert( 'Download failed', @@ -166,9 +189,10 @@ export const PacksScreen: React.FC = () => { } } + setDownloadProgress({ stage: 'pack', bytesWritten: 0, contentLength: 0 }); const packResult = await acquireLatestPack( GANESHA_PROJECT_ID, - {}, + { onProgress: progressReporter('pack', setDownloadProgress) }, modelForPack ?? undefined, ); if (!packResult.ok) { @@ -189,6 +213,7 @@ export const PacksScreen: React.FC = () => { } finally { updateInFlight.current = false; setIsDownloading(false); + setDownloadProgress(null); } }, [miewidModel, navigation, preferGpuModel]); @@ -209,27 +234,6 @@ export const PacksScreen: React.FC = () => { } }, [navigation, refreshPackStatus]); - const updateStatusText = isDownloading - ? 'Downloading and validating update...' - : effectivePackUpdateState === 'checking' - ? 'Checking for updates...' - : effectivePackUpdateState === 'current' - ? 'Up to date' - : effectivePackUpdateState === 'available' - ? 'Update available' - : effectivePackUpdateState === 'unavailable' - ? 'Unable to check for updates' - : 'Update status not checked'; - - const updateButtonTitle = - effectivePackUpdateState === 'current' - ? 'Check Again' - : effectivePackUpdateState === 'available' - ? 'Update to Latest Pack' - : effectivePackUpdateState === 'checking' - ? 'Checking for Updates' - : 'Check for Updates'; - const renderPack = ({ item, index, @@ -277,6 +281,12 @@ export const PacksScreen: React.FC = () => { style={styles.downloadButton} testID="download-pack-button" /> + {isDownloading ? ( + + ) : null} ) : ( { showsVerticalScrollIndicator={false} ListFooterComponent={ - - {updateStatusText} - + {isDownloading ? ( + + ) : ( + + {UPDATE_STATUS_TEXT[effectivePackUpdateState]} + + )}