From e66d5a70e7d12be1098b9b0bcd403da694fb3bea Mon Sep 17 00:00:00 2001 From: Santo Shakil Date: Tue, 20 Oct 2026 20:35:10 +0600 Subject: [PATCH] fix(mobile): hide server addresses in upload error logs (#32550) * fix(mobile): hide server addresses in upload error logs * fix(mobile): remove duplicate upload failure logs (cherry picked from commit 211773f778ca24ac33e64d46e50b3ab46702c851) --- .../share_intent_upload.provider.dart | 5 +---- .../lib/providers/backup/backup.provider.dart | 2 -- mobile/lib/repositories/upload.repository.dart | 8 +++++--- .../services/foreground_upload.service.dart | 4 +++- .../repositories/upload_repository_test.dart | 18 ++++++++++++++++++ 5 files changed, 27 insertions(+), 10 deletions(-) diff --git a/mobile/lib/providers/asset_viewer/share_intent_upload.provider.dart b/mobile/lib/providers/asset_viewer/share_intent_upload.provider.dart index 47d67c967..48039f501 100644 --- a/mobile/lib/providers/asset_viewer/share_intent_upload.provider.dart +++ b/mobile/lib/providers/asset_viewer/share_intent_upload.provider.dart @@ -6,7 +6,6 @@ import 'package:immich_mobile/models/upload/share_intent_attachment.model.dart'; import 'package:immich_mobile/routing/router.dart'; import 'package:immich_mobile/services/foreground_upload.service.dart'; import 'package:immich_mobile/services/share_intent_service.dart'; -import 'package:logging/logging.dart'; import 'package:path/path.dart' as p; final shareIntentUploadProvider = StateNotifierProvider>( @@ -21,7 +20,6 @@ class ShareIntentUploadStateNotifier extends StateNotifier { } void _handleForegroundBackupError(String localAssetId, String errorMessage) { - _logger.severe("Upload failed for $localAssetId: $errorMessage"); - final currentItem = state.uploadItems[localAssetId]; if (currentItem != null) { state = state.copyWith( diff --git a/mobile/lib/repositories/upload.repository.dart b/mobile/lib/repositories/upload.repository.dart index 25960c6a5..22709a00e 100644 --- a/mobile/lib/repositories/upload.repository.dart +++ b/mobile/lib/repositories/upload.repository.dart @@ -88,7 +88,7 @@ class UploadRepository { } on RequestAbortedException { rethrow; } on ClientException catch (error) { - logger.warning("Upload $logContext failed before a response, resending once: $error"); + logger.warning("Upload $logContext failed before a response, resending once", error); response = await client.send(buildRequest()); } @@ -124,8 +124,8 @@ class UploadRepository { logger.warning("Upload $logContext was cancelled"); return UploadResult.cancelled(); } catch (error, stackTrace) { - logger.warning("Error uploading $logContext: $error: $stackTrace"); - return UploadResult.error(errorMessage: error.toString()); + logger.warning("Error uploading $logContext", error, stackTrace); + return UploadResult(isSuccess: false, isCancelled: false, errorMessage: error.toString(), error: error); } } } @@ -166,6 +166,7 @@ class UploadResult { final String? remoteAssetId; final String? errorMessage; final int? statusCode; + final Object? error; const UploadResult({ required this.isSuccess, @@ -173,6 +174,7 @@ class UploadResult { this.remoteAssetId, this.errorMessage, this.statusCode, + this.error, }); factory UploadResult.success({required String remoteAssetId}) { diff --git a/mobile/lib/services/foreground_upload.service.dart b/mobile/lib/services/foreground_upload.service.dart index 77042512a..023503d2f 100644 --- a/mobile/lib/services/foreground_upload.service.dart +++ b/mobile/lib/services/foreground_upload.service.dart @@ -182,6 +182,7 @@ class ForegroundUploadService { if (result.isSuccess) { onSuccess?.call(fileId, result.remoteAssetId!); } else if (!result.isCancelled && result.errorMessage != null) { + _logger.warning("Error(${result.statusCode}) uploading $fileId", result.error ?? result.errorMessage); onError?.call(fileId, result.errorMessage!); } }, @@ -395,7 +396,8 @@ class ForegroundUploadService { } else if (result.errorMessage != null) { _logger.severe( () => - "Error(${result.statusCode}) uploading ${asset.localId} | $originalFileName | Created on ${asset.createdAt} | ${result.errorMessage}", + "Error(${result.statusCode}) uploading ${asset.localId} | $originalFileName | Created on ${asset.createdAt}", + result.error ?? result.errorMessage, ); callbacks.onError?.call(asset.localId!, result.errorMessage!); diff --git a/mobile/test/repositories/upload_repository_test.dart b/mobile/test/repositories/upload_repository_test.dart index d2e324eb8..f01824929 100644 --- a/mobile/test/repositories/upload_repository_test.dart +++ b/mobile/test/repositories/upload_repository_test.dart @@ -13,6 +13,7 @@ import 'package:immich_mobile/domain/services/store.service.dart'; import 'package:immich_mobile/entities/store.entity.dart'; import 'package:immich_mobile/infrastructure/repositories/store.repository.dart'; import 'package:immich_mobile/repositories/upload.repository.dart'; +import 'package:logging/logging.dart'; import 'package:mocktail/mocktail.dart'; class _MockHttpClient extends Mock implements http.Client {} @@ -124,4 +125,21 @@ void main() { expect(result.errorMessage, 'boom'); verify(() => client.send(any())).called(1); }); + + test('a transport failure is logged as the error, not in the message', () async { + final error = http.ClientException('Failed to connect to /203.0.113.9:2283'); + stubSend((_) => throw error); + final level = Logger.root.level; + addTearDown(() => Logger.root.level = level); + Logger.root.level = Level.ALL; + final records = []; + final subscription = sut.logger.onRecord.listen(records.add); + + final result = await upload(); + await subscription.cancel(); + + expect(records.map((r) => r.error), [error, error]); + expect(records.map((r) => r.message), everyElement(isNot(contains('203.0.113.9')))); + expect(result.error, error); + }); }