From 1539ae8b07dfdf093005a3e81ad518b91f00ea1a Mon Sep 17 00:00:00 2001 From: Santo Shakil Date: Tue, 18 Aug 2026 22:38:52 +0600 Subject: [PATCH] fix(mobile): resend an upload once when the connection dies before a response (#30843) --- .../lib/repositories/upload.repository.dart | 35 +++-- .../repositories/upload_repository_test.dart | 127 ++++++++++++++++++ 2 files changed, 150 insertions(+), 12 deletions(-) create mode 100644 mobile/test/repositories/upload_repository_test.dart diff --git a/mobile/lib/repositories/upload.repository.dart b/mobile/lib/repositories/upload.repository.dart index 1ebc9825f..136684093 100644 --- a/mobile/lib/repositories/upload.repository.dart +++ b/mobile/lib/repositories/upload.repository.dart @@ -95,23 +95,34 @@ class UploadRepository { required Completer? cancelToken, void Function(int bytes, int totalBytes)? onProgress, required String logContext, + Client? httpClient, }) async { final String savedEndpoint = Store.get(StoreKey.serverEndpoint); - final baseRequest = ProgressMultipartRequest( - 'POST', - Uri.parse('$savedEndpoint/assets'), - abortTrigger: cancelToken?.future, - onProgress: onProgress, - ); + + ProgressMultipartRequest buildRequest() { + final request = ProgressMultipartRequest( + 'POST', + Uri.parse('$savedEndpoint/assets'), + abortTrigger: cancelToken?.future, + onProgress: onProgress, + ); + request.fields.addAll(fields); + request.files.add(MultipartFile("assetData", file.openRead(), file.lengthSync(), filename: originalFileName)); + return request; + } try { - final fileStream = file.openRead(); - final assetRawUploadData = MultipartFile("assetData", fileStream, file.lengthSync(), filename: originalFileName); + final client = httpClient ?? NetworkRepository.client; + StreamedResponse response; + try { + response = await client.send(buildRequest()); + } on RequestAbortedException { + rethrow; + } on ClientException catch (error) { + logger.warning("Upload $logContext failed before a response, resending once: $error"); + response = await client.send(buildRequest()); + } - baseRequest.fields.addAll(fields); - baseRequest.files.add(assetRawUploadData); - - final response = await NetworkRepository.client.send(baseRequest); final responseBodyString = await response.stream.bytesToString(); if (![200, 201].contains(response.statusCode)) { diff --git a/mobile/test/repositories/upload_repository_test.dart b/mobile/test/repositories/upload_repository_test.dart new file mode 100644 index 000000000..516559de5 --- /dev/null +++ b/mobile/test/repositories/upload_repository_test.dart @@ -0,0 +1,127 @@ +import 'dart:async'; +import 'dart:convert'; +import 'dart:io'; + +import 'package:background_downloader/background_downloader.dart'; +import 'package:drift/drift.dart'; +import 'package:drift/native.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:http/http.dart' as http; +import 'package:immich_mobile/domain/models/store.model.dart'; +import 'package:immich_mobile/domain/services/store.service.dart'; +import 'package:immich_mobile/entities/store.entity.dart'; +import 'package:immich_mobile/infrastructure/repositories/db.repository.dart'; +import 'package:immich_mobile/infrastructure/repositories/store.repository.dart'; +import 'package:immich_mobile/repositories/upload.repository.dart'; +import 'package:mocktail/mocktail.dart'; + +class _MockHttpClient extends Mock implements http.Client {} + +class _FakeBaseRequest extends Fake implements http.BaseRequest {} + +// keeps the FileDownloader singleton off the disk and off the platform channels +class _NoStorage extends Fake implements PersistentStorage { + @override + Future initialize() async {} +} + +void main() { + late _MockHttpClient client; + late UploadRepository sut; + late File file; + + setUpAll(() async { + TestWidgetsFlutterBinding.ensureInitialized(); + FileDownloader(persistentStorage: _NoStorage()); + final db = Drift(DatabaseConnection(NativeDatabase.memory(), closeStreamsSynchronously: true)); + await StoreService.init(storeRepository: StoreRepository(db)); + await Store.put(StoreKey.serverEndpoint, 'http://demo.immich.app/api'); + registerFallbackValue(_FakeBaseRequest()); + file = File('${Directory.systemTemp.createTempSync().path}/photo.jpg')..writeAsStringSync('bytes'); + }); + + setUp(() { + client = _MockHttpClient(); + sut = UploadRepository(); + }); + + // consumes the body like a real client would, so a reused request would blow up on the second send + void stubSend(FutureOr Function(int attempt) answer) { + var attempt = 0; + when(() => client.send(any())).thenAnswer((invocation) async { + final request = invocation.positionalArguments.single as http.BaseRequest; + await request.finalize().drain(); + return answer(++attempt); + }); + } + + http.StreamedResponse response(int status, String body) => + http.StreamedResponse(Stream.value(utf8.encode(body)), status); + + Future upload() => sut.uploadFile( + file: file, + originalFileName: 'photo.jpg', + fields: const {'deviceAssetId': 'a1'}, + cancelToken: null, + logContext: 'a1', + httpClient: client, + ); + + test('resends once when the first send dies before a response', () async { + stubSend((attempt) { + if (attempt == 1) { + throw http.ClientException('Broken pipe'); + } + return response(201, '{"id":"remote-1"}'); + }); + + final result = await upload(); + + expect(result.isSuccess, isTrue); + expect(result.remoteAssetId, 'remote-1'); + verify(() => client.send(any())).called(2); + }); + + test('a second transport failure is an error, no third send', () async { + stubSend((_) => throw http.ClientException('Connection reset')); + + final result = await upload(); + + expect(result.isSuccess, isFalse); + expect(result.isCancelled, isFalse); + verify(() => client.send(any())).called(2); + }); + + test('a cancelled upload is not resent', () async { + stubSend((_) => throw http.RequestAbortedException()); + + final result = await upload(); + + expect(result.isCancelled, isTrue); + verify(() => client.send(any())).called(1); + }); + + test('a cancel during the resend still counts as cancelled', () async { + stubSend((attempt) { + if (attempt == 1) { + throw http.ClientException('Broken pipe'); + } + throw http.RequestAbortedException(); + }); + + final result = await upload(); + + expect(result.isCancelled, isTrue); + verify(() => client.send(any())).called(2); + }); + + test('a server error response is not resent', () async { + stubSend((_) => response(500, '{"message":"boom"}')); + + final result = await upload(); + + expect(result.statusCode, 500); + expect(result.errorMessage, 'boom'); + verify(() => client.send(any())).called(1); + }); +}