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)
This commit is contained in:
@@ -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<ShareIntentUploadStateNotifier, List<ShareIntentAttachment>>(
|
||||
@@ -21,7 +20,6 @@ class ShareIntentUploadStateNotifier extends StateNotifier<List<ShareIntentAttac
|
||||
final AppRouter router;
|
||||
final ForegroundUploadService _foregroundUploadService;
|
||||
final ShareIntentService _shareIntentService;
|
||||
final Logger _logger = Logger('ShareIntentUploadStateNotifier');
|
||||
|
||||
ShareIntentUploadStateNotifier(this.router, this._foregroundUploadService, this._shareIntentService) : super([]);
|
||||
|
||||
@@ -74,8 +72,7 @@ class ShareIntentUploadStateNotifier extends StateNotifier<List<ShareIntentAttac
|
||||
onSuccess: (fileId, _) {
|
||||
_updateStatus(fileId, UploadStatus.complete, progress: 1.0);
|
||||
},
|
||||
onError: (fileId, errorMessage) {
|
||||
_logger.warning("Upload failed for file: $fileId, error: $errorMessage");
|
||||
onError: (fileId, _) {
|
||||
_updateStatus(fileId, UploadStatus.failed);
|
||||
},
|
||||
);
|
||||
|
||||
@@ -221,8 +221,6 @@ class BackupNotifier extends StateNotifier<BackupState> {
|
||||
}
|
||||
|
||||
void _handleForegroundBackupError(String localAssetId, String errorMessage) {
|
||||
_logger.severe("Upload failed for $localAssetId: $errorMessage");
|
||||
|
||||
final currentItem = state.uploadItems[localAssetId];
|
||||
if (currentItem != null) {
|
||||
state = state.copyWith(
|
||||
|
||||
@@ -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}) {
|
||||
|
||||
@@ -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!);
|
||||
|
||||
@@ -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 = <LogRecord>[];
|
||||
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);
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user