diff --git a/lib/services/translation_service.dart b/lib/services/translation_service.dart index 7b1d7f5..d0ebee7 100644 --- a/lib/services/translation_service.dart +++ b/lib/services/translation_service.dart @@ -35,6 +35,78 @@ class TranslationDownloadCancelled implements Exception { String toString() => 'Download canceled.'; } +/// Max attempts (initial + retries) for a transient model-download failure. #229 +const int kTranslationDownloadMaxAttempts = 5; + +/// HTTP statuses worth retrying on a model download: server overload / 5xx and +/// 429. Terminal 4xx (e.g. 404) are not retried. #229 +bool isRetryableDownloadStatus(int statusCode) => + statusCode == 429 || (statusCode >= 500 && statusCode <= 599); + +/// Exponential backoff for download [attempt] (1-based): 1, 2, 4, 8, 16 s capped +/// at 30 s; a larger server `Retry-After` (seconds) wins, also capped. No jitter +/// (single-client download — no thundering-herd concern). #229 +Duration translationDownloadBackoff(int attempt, {int? retryAfterSeconds}) { + final exp = (1 << (attempt - 1)).clamp(1, 30); + final seconds = (retryAfterSeconds != null && retryAfterSeconds > exp) + ? retryAfterSeconds.clamp(1, 30) + : exp; + return Duration(seconds: seconds); +} + +int? _retryAfterHeaderSeconds(Map headers) => + int.tryParse(headers['retry-after']?.trim() ?? ''); + +/// Sends the request built by [buildRequest] on [client], retrying transient +/// failures — 5xx / 429 responses and network exceptions — with bounded +/// exponential backoff. Terminal responses (2xx, 3xx, non-retryable 4xx) are +/// returned for the caller to validate; a sustained failure throws after +/// [maxAttempts]. Cancellable via [isCancelled]. Deps injected → unit-testable. +/// #229 +Future sendModelDownloadWithRetry( + http.Client client, + http.Request Function() buildRequest, { + required Future Function(Duration) sleep, + required bool Function() isCancelled, + int maxAttempts = kTranslationDownloadMaxAttempts, +}) async { + var attempt = 0; + while (true) { + attempt++; + if (isCancelled()) throw const TranslationDownloadCancelled(); + try { + final response = await client.send(buildRequest()); + if (isRetryableDownloadStatus(response.statusCode) && + attempt < maxAttempts) { + await response.stream.drain(); + appLogger.warn( + 'Model download HTTP ${response.statusCode}; retry $attempt/$maxAttempts', + ); + await sleep( + translationDownloadBackoff( + attempt, + retryAfterSeconds: _retryAfterHeaderSeconds(response.headers), + ), + ); + continue; + } + return response; + } on TranslationDownloadCancelled { + rethrow; + } on Exception catch (e) { + if ((e is http.ClientException || e is TimeoutException) && + attempt < maxAttempts) { + appLogger.warn( + 'Model download error ($e); retry $attempt/$maxAttempts', + ); + await sleep(translationDownloadBackoff(attempt)); + continue; + } + rethrow; + } + } +} + class TranslationService extends ChangeNotifier { final AppSettingsService _appSettingsService; final TranslationFileStore _fileStore; @@ -205,7 +277,10 @@ class TranslationService extends ChangeNotifier { int? totalSize; bool supportsRange = false; try { - final headResponse = await headClient.send(http.Request('HEAD', uri)); + final headResponse = await _sendWithRetry( + headClient, + () => http.Request('HEAD', uri), + ); totalSize = headResponse.contentLength; supportsRange = headResponse.headers['accept-ranges']?.contains('bytes') == true; @@ -257,13 +332,37 @@ class TranslationService extends ChangeNotifier { }); } + Future _sendWithRetry( + http.Client client, + http.Request Function() buildRequest, + ) => sendModelDownloadWithRetry( + client, + buildRequest, + sleep: _cancellableBackoff, + isCancelled: () => _cancelDownloadRequested, + ); + + /// Backoff wait that aborts promptly when the user cancels the download. + Future _cancellableBackoff(Duration total) async { + const step = Duration(milliseconds: 250); + var waited = Duration.zero; + while (waited < total && !_cancelDownloadRequested) { + final remaining = total - waited; + await Future.delayed(remaining < step ? remaining : step); + waited += step; + } + } + Future _downloadSingle({ required Uri uri, required String fileName, }) async { final client = http.Client(); try { - final response = await client.send(http.Request('GET', uri)); + final response = await _sendWithRetry( + client, + () => http.Request('GET', uri), + ); if (response.statusCode < 200 || response.statusCode >= 300) { throw StateError('Model download failed: HTTP ${response.statusCode}'); } @@ -338,9 +437,11 @@ class TranslationService extends ChangeNotifier { required int start, required int end, }) async { - final request = http.Request('GET', uri); - request.headers['Range'] = 'bytes=$start-$end'; - final response = await client.send(request); + final response = await _sendWithRetry(client, () { + final request = http.Request('GET', uri); + request.headers['Range'] = 'bytes=$start-$end'; + return request; + }); if (response.statusCode != 206) { await response.stream.drain(); throw StateError( diff --git a/test/services/translation_download_retry_test.dart b/test/services/translation_download_retry_test.dart new file mode 100644 index 0000000..3a3067a --- /dev/null +++ b/test/services/translation_download_retry_test.dart @@ -0,0 +1,134 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:http/http.dart' as http; +import 'package:http/testing.dart'; +import 'package:meshcore_open/services/translation_service.dart'; + +void main() { + final uri = Uri.parse('https://example.test/model.gguf'); + http.Request get() => http.Request('GET', uri); + Future noSleep(Duration _) async {} + bool never() => false; + + group('isRetryableDownloadStatus', () { + test('5xx and 429 are retryable', () { + for (final s in [500, 502, 503, 504, 429]) { + expect(isRetryableDownloadStatus(s), isTrue, reason: '$s'); + } + }); + test('2xx / 3xx / terminal 4xx are not retryable', () { + for (final s in [200, 206, 301, 400, 403, 404]) { + expect(isRetryableDownloadStatus(s), isFalse, reason: '$s'); + } + }); + }); + + group('translationDownloadBackoff', () { + test('exponential 1,2,4,8,16 then capped at 30', () { + expect(translationDownloadBackoff(1), const Duration(seconds: 1)); + expect(translationDownloadBackoff(2), const Duration(seconds: 2)); + expect(translationDownloadBackoff(3), const Duration(seconds: 4)); + expect(translationDownloadBackoff(4), const Duration(seconds: 8)); + expect(translationDownloadBackoff(5), const Duration(seconds: 16)); + expect(translationDownloadBackoff(6), const Duration(seconds: 30)); + }); + test('a larger Retry-After wins, also capped at 30', () { + expect( + translationDownloadBackoff(1, retryAfterSeconds: 20), + const Duration(seconds: 20), + ); + expect( + translationDownloadBackoff(1, retryAfterSeconds: 999), + const Duration(seconds: 30), + ); + // Smaller Retry-After does not shrink the exponential floor. + expect( + translationDownloadBackoff(4, retryAfterSeconds: 2), + const Duration(seconds: 8), + ); + }); + }); + + group('sendModelDownloadWithRetry', () { + test('retries a transient 503 then succeeds', () async { + var calls = 0; + final client = MockClient((req) async { + calls++; + return http.Response( + calls == 1 ? 'busy' : 'ok', + calls == 1 ? 503 : 200, + ); + }); + final res = await sendModelDownloadWithRetry( + client, + get, + sleep: noSleep, + isCancelled: never, + ); + expect(res.statusCode, 200); + expect(calls, 2); + }); + + test('gives up after maxAttempts on persistent 503', () async { + var calls = 0; + final client = MockClient((req) async { + calls++; + return http.Response('busy', 503); + }); + final res = await sendModelDownloadWithRetry( + client, + get, + sleep: noSleep, + isCancelled: never, + maxAttempts: 3, + ); + expect(res.statusCode, 503); + expect(calls, 3); + }); + + test('does not retry a terminal 404', () async { + var calls = 0; + final client = MockClient((req) async { + calls++; + return http.Response('nope', 404); + }); + final res = await sendModelDownloadWithRetry( + client, + get, + sleep: noSleep, + isCancelled: never, + ); + expect(res.statusCode, 404); + expect(calls, 1); + }); + + test('retries a network exception then succeeds', () async { + var calls = 0; + final client = MockClient((req) async { + calls++; + if (calls == 1) throw http.ClientException('connection reset'); + return http.Response('ok', 200); + }); + final res = await sendModelDownloadWithRetry( + client, + get, + sleep: noSleep, + isCancelled: never, + ); + expect(res.statusCode, 200); + expect(calls, 2); + }); + + test('aborts immediately when cancelled', () async { + final client = MockClient((req) async => http.Response('ok', 200)); + expect( + () => sendModelDownloadWithRetry( + client, + get, + sleep: noSleep, + isCancelled: () => true, + ), + throwsA(isA()), + ); + }); + }); +}