From 0148b8734f4f3d7c85193619a63cccb24fd19269 Mon Sep 17 00:00:00 2001 From: Joe Nash Date: Thu, 30 Jul 2026 12:16:14 -0400 Subject: [PATCH] fix(mobile): allow stalled uploads to be cancelled Signed-off-by: Joe Nash --- mobile/lib/features/channels/compose_bar.dart | 27 ++++- .../channels/compose_bar/attachments.dart | 47 ++++++-- .../features/channels/compose_bar/layout.dart | 29 +++-- .../channels/compose_bar/send_button.dart | 3 + mobile/lib/shared/relay/media_upload.dart | 112 +++++++++++++----- .../features/channels/compose_bar_test.dart | 71 +++++++++++ .../test/shared/relay/media_upload_test.dart | 47 ++++++++ 7 files changed, 286 insertions(+), 50 deletions(-) diff --git a/mobile/lib/features/channels/compose_bar.dart b/mobile/lib/features/channels/compose_bar.dart index 1a9a02e409..07d0cab891 100644 --- a/mobile/lib/features/channels/compose_bar.dart +++ b/mobile/lib/features/channels/compose_bar.dart @@ -533,6 +533,8 @@ class ComposeBar extends HookConsumerWidget { if (uploaded != null && context.mounted) { attachments.value = [...attachments.value, uploaded]; } + } on MediaUploadCancelledException { + // User-initiated cancellation is an expected recovery action. } catch (error) { if (context.mounted) { uploadError.value = _formatUploadError(error); @@ -573,6 +575,8 @@ class ComposeBar extends HookConsumerWidget { .read(mediaUploadServiceProvider) .uploadImage(image); return (uploaded: uploaded, error: null); + } on MediaUploadCancelledException catch (error) { + return (uploaded: null, error: error); } catch (error) { return (uploaded: null, error: error); } @@ -588,12 +592,15 @@ class ComposeBar extends HookConsumerWidget { start + _maxConcurrentImageUploads, images.length, ); - results.addAll( - await Future.wait([ - for (final image in images.sublist(start, end)) - uploadImage(image), - ]), - ); + final batchResults = await Future.wait([ + for (final image in images.sublist(start, end)) uploadImage(image), + ]); + results.addAll(batchResults); + if (batchResults.any( + (result) => result.error is MediaUploadCancelledException, + )) { + break; + } } if (!context.mounted) return; @@ -603,6 +610,7 @@ class ComposeBar extends HookConsumerWidget { } final firstError = results .map((result) => result.error) + .where((error) => error is! MediaUploadCancelledException) .whereType() .firstOrNull; if (firstError != null) { @@ -672,6 +680,11 @@ class ComposeBar extends HookConsumerWidget { ); } + void cancelPendingUploads() { + ref.read(mediaUploadServiceProvider).cancelPendingUploads(); + uploadError.value = null; + } + // Wrap (or insert) markdown formatting around the current selection. void applyFormat(String prefix, [String? suffix]) { suffix ??= prefix; @@ -938,7 +951,9 @@ class ComposeBar extends HookConsumerWidget { attachments: attachments.value, uploadingCount: uploadingCount.value, onRemoveAttachment: removeAttachment, + onCancelUploads: cancelPendingUploads, uploadError: uploadError.value, + onDismissUploadError: () => uploadError.value = null, isExpanded: isComposerExpanded.value, controller: controller, focusNode: focusNode, diff --git a/mobile/lib/features/channels/compose_bar/attachments.dart b/mobile/lib/features/channels/compose_bar/attachments.dart index 7c53ae1098..e9e132d800 100644 --- a/mobile/lib/features/channels/compose_bar/attachments.dart +++ b/mobile/lib/features/channels/compose_bar/attachments.dart @@ -471,11 +471,13 @@ class _AttachmentStrip extends StatelessWidget { final List attachments; final int uploadingCount; final void Function(String url) onRemove; + final VoidCallback onCancelUploads; const _AttachmentStrip({ required this.attachments, required this.uploadingCount, required this.onRemove, + required this.onCancelUploads, }); @override @@ -495,9 +497,8 @@ class _AttachmentStrip extends StatelessWidget { ? 'Uploading attachment…' : 'Uploading $uploadingCount attachments…'; return Semantics( - excludeSemantics: true, - liveRegion: true, - label: label, + container: true, + explicitChildNodes: true, child: Container( key: const ValueKey('compose-upload-progress'), width: thumbWidth, @@ -509,15 +510,20 @@ class _AttachmentStrip extends StatelessWidget { child: Stack( alignment: Alignment.center, children: [ - BuzzLoadingIndicator( - size: 34, - color: context.colors.primary, - semanticLabel: label, + Semantics( + excludeSemantics: true, + liveRegion: true, + label: label, + child: BuzzLoadingIndicator( + size: 34, + color: context.colors.primary, + semanticLabel: label, + ), ), if (uploadingCount > 1) PositionedDirectional( top: Grid.quarter, - end: Grid.quarter, + start: Grid.quarter, child: Container( key: const ValueKey('compose-upload-count'), constraints: const BoxConstraints( @@ -539,6 +545,31 @@ class _AttachmentStrip extends StatelessWidget { ), ), ), + PositionedDirectional( + top: Grid.quarter, + end: Grid.quarter, + child: Semantics( + button: true, + label: 'Cancel upload', + child: ExcludeSemantics( + child: SizedBox( + width: 24, + height: 24, + child: IconButton( + key: const ValueKey('compose-upload-cancel'), + tooltip: 'Cancel upload', + onPressed: onCancelUploads, + style: IconButton.styleFrom( + backgroundColor: context.colors.surface, + foregroundColor: context.colors.onSurface, + padding: EdgeInsets.zero, + ), + icon: const Icon(LucideIcons.x, size: 16), + ), + ), + ), + ), + ), ], ), ), diff --git a/mobile/lib/features/channels/compose_bar/layout.dart b/mobile/lib/features/channels/compose_bar/layout.dart index 31b930e36d..7f75f65145 100644 --- a/mobile/lib/features/channels/compose_bar/layout.dart +++ b/mobile/lib/features/channels/compose_bar/layout.dart @@ -4,7 +4,9 @@ class _ComposeBarLayout extends StatelessWidget { final List attachments; final int uploadingCount; final ValueChanged onRemoveAttachment; + final VoidCallback onCancelUploads; final String? uploadError; + final VoidCallback onDismissUploadError; final bool isExpanded; final TextEditingController controller; final FocusNode focusNode; @@ -32,7 +34,9 @@ class _ComposeBarLayout extends StatelessWidget { required this.attachments, required this.uploadingCount, required this.onRemoveAttachment, + required this.onCancelUploads, required this.uploadError, + required this.onDismissUploadError, required this.isExpanded, required this.controller, required this.focusNode, @@ -81,18 +85,29 @@ class _ComposeBarLayout extends StatelessWidget { attachments: attachments, uploadingCount: uploadingCount, onRemove: onRemoveAttachment, + onCancelUploads: onCancelUploads, ), const SizedBox(height: Grid.xxs), ], if (uploadError case final error?) ...[ - Align( - alignment: Alignment.centerLeft, - child: Text( - error, - style: context.textTheme.bodySmall?.copyWith( - color: context.colors.error, + Row( + children: [ + Expanded( + child: Text( + error, + style: context.textTheme.bodySmall?.copyWith( + color: context.colors.error, + ), + ), ), - ), + IconButton( + key: const ValueKey('compose-upload-error-dismiss'), + tooltip: 'Dismiss upload error', + visualDensity: VisualDensity.compact, + onPressed: onDismissUploadError, + icon: const Icon(LucideIcons.x, size: 18), + ), + ], ), const SizedBox(height: Grid.xxs), ], diff --git a/mobile/lib/features/channels/compose_bar/send_button.dart b/mobile/lib/features/channels/compose_bar/send_button.dart index 54060ae948..f8a05c5398 100644 --- a/mobile/lib/features/channels/compose_bar/send_button.dart +++ b/mobile/lib/features/channels/compose_bar/send_button.dart @@ -43,5 +43,8 @@ class _SendButton extends StatelessWidget { } String _formatUploadError(Object error) { + if (error is TimeoutException) { + return 'Upload timed out. Check your connection and try again.'; + } return error.toString().replaceFirst('Exception: ', ''); } diff --git a/mobile/lib/shared/relay/media_upload.dart b/mobile/lib/shared/relay/media_upload.dart index 58c93979d7..98ff00e482 100644 --- a/mobile/lib/shared/relay/media_upload.dart +++ b/mobile/lib/shared/relay/media_upload.dart @@ -1,3 +1,4 @@ +import 'dart:async'; import 'dart:convert'; import 'dart:io'; @@ -27,6 +28,8 @@ const _readClipboardImageMethod = 'readClipboardImage'; const _clipboardHasImageMethod = 'clipboardHasImage'; const _uploadAuthKind = 24242; const _uploadAuthLifetimeSeconds = 300; +const _defaultMediaUploadTimeout = Duration(minutes: 2); +const _defaultVideoUploadTimeout = Duration(minutes: 10); const _heicBrands = { 'heic', 'heix', @@ -82,6 +85,14 @@ class MediaPolicyUploadException implements Exception { String toString() => _mediaPolicyUploadMessage; } +/// Thrown when the user intentionally cancels an in-flight media upload. +class MediaUploadCancelledException implements Exception { + const MediaUploadCancelledException(); + + @override + String toString() => 'Upload cancelled'; +} + @immutable class _PreparedUploadImage { final Uint8List bytes; @@ -185,6 +196,9 @@ class MediaUploadService { final DateTime Function() _now; final http.Client _http; final bool _ownsHttpClient; + final Duration _mediaUploadTimeout; + final Duration _videoUploadTimeout; + final Set> _pendingUploadAborts = {}; MediaUploadService({ required String baseUrl, @@ -199,6 +213,8 @@ class MediaUploadService { ReadClipboardImage? readClipboardImage, DateTime Function()? now, http.Client? httpClient, + Duration mediaUploadTimeout = _defaultMediaUploadTimeout, + Duration videoUploadTimeout = _defaultVideoUploadTimeout, }) : _baseUrl = baseUrl, _nsec = nsec, _pickGalleryImage = pickGalleryImage, @@ -217,14 +233,24 @@ class MediaUploadService { _readClipboardImage = readClipboardImage ?? _readPlatformClipboardImage, _now = now ?? DateTime.now, _http = httpClient ?? http.Client(), - _ownsHttpClient = httpClient == null; + _ownsHttpClient = httpClient == null, + _mediaUploadTimeout = mediaUploadTimeout, + _videoUploadTimeout = videoUploadTimeout; void dispose() { + cancelPendingUploads(); if (_ownsHttpClient) { _http.close(); } } + /// Cancels every upload currently owned by this service. + void cancelPendingUploads() { + for (final abort in _pendingUploadAborts.toList(growable: false)) { + if (!abort.isCompleted) abort.complete(); + } + } + Future pickAndUploadImage() async { final pickedImage = await _pickGalleryImage(); if (pickedImage == null) return null; @@ -363,49 +389,77 @@ class MediaUploadService { } final sha256 = _sha256Hex(bytes); - var request = _buildUploadRequest( - bytes: bytes, - mimeType: mimeType, - sha256: sha256, - path: _mediaUploadPath, - ); + final abort = Completer(); + _pendingUploadAborts.add(abort); + var timedOut = false; + final timeout = mimeType.startsWith('video/') + ? _videoUploadTimeout + : _mediaUploadTimeout; + final timer = Timer(timeout, () { + timedOut = true; + if (!abort.isCompleted) abort.complete(); + }); - var streamed = await _http.send(request); - var response = await http.Response.fromStream(streamed); - if (response.statusCode == HttpStatus.notFound || - response.statusCode == HttpStatus.methodNotAllowed) { - request = _buildUploadRequest( + try { + var request = _buildUploadRequest( bytes: bytes, mimeType: mimeType, sha256: sha256, - path: _legacyMediaUploadPath, + path: _mediaUploadPath, + abortTrigger: abort.future, ); - streamed = await _http.send(request); - response = await http.Response.fromStream(streamed); - } - if (response.statusCode < 200 || response.statusCode >= 300) { - if (_allowedImageMimeTypes.contains(mimeType) && - (response.statusCode == HttpStatus.unsupportedMediaType || - response.statusCode == HttpStatus.unprocessableEntity)) { - throw const MediaPolicyUploadException(); + + var streamed = await _http.send(request); + var response = await http.Response.fromStream(streamed); + if (response.statusCode == HttpStatus.notFound || + response.statusCode == HttpStatus.methodNotAllowed) { + request = _buildUploadRequest( + bytes: bytes, + mimeType: mimeType, + sha256: sha256, + path: _legacyMediaUploadPath, + abortTrigger: abort.future, + ); + streamed = await _http.send(request); + response = await http.Response.fromStream(streamed); } - throw Exception( - 'upload failed (${response.statusCode}): ${response.body}', + if (response.statusCode < 200 || response.statusCode >= 300) { + if (_allowedImageMimeTypes.contains(mimeType) && + (response.statusCode == HttpStatus.unsupportedMediaType || + response.statusCode == HttpStatus.unprocessableEntity)) { + throw const MediaPolicyUploadException(); + } + throw Exception( + 'upload failed (${response.statusCode}): ${response.body}', + ); + } + + return BlobDescriptor.fromJson( + jsonDecode(response.body) as Map, ); + } on http.RequestAbortedException { + if (timedOut) { + throw TimeoutException('Upload timed out', timeout); + } + throw const MediaUploadCancelledException(); + } finally { + timer.cancel(); + _pendingUploadAborts.remove(abort); } - - return BlobDescriptor.fromJson( - jsonDecode(response.body) as Map, - ); } - http.Request _buildUploadRequest({ + http.AbortableRequest _buildUploadRequest({ required Uint8List bytes, required String mimeType, required String sha256, required String path, + required Future abortTrigger, }) { - final request = http.Request('PUT', Uri.parse(_baseUrl).resolve(path)); + final request = http.AbortableRequest( + 'PUT', + Uri.parse(_baseUrl).resolve(path), + abortTrigger: abortTrigger, + ); request.bodyBytes = bytes; request.headers.addAll( _buildUploadHeaders(mimeType: mimeType, sha256: sha256), diff --git a/mobile/test/features/channels/compose_bar_test.dart b/mobile/test/features/channels/compose_bar_test.dart index 7b747adfac..8667626d45 100644 --- a/mobile/test/features/channels/compose_bar_test.dart +++ b/mobile/test/features/channels/compose_bar_test.dart @@ -133,6 +133,18 @@ const _nativeAttachmentPopoverChannel = MethodChannel( 'buzz/native_attachment_popover', ); +class _AbortAwareClient extends http.BaseClient { + final started = Completer(); + + @override + Future send(http.BaseRequest request) async { + if (!started.isCompleted) started.complete(); + final abortTrigger = (request as http.AbortableRequest).abortTrigger; + await abortTrigger; + throw http.RequestAbortedException(request.url); + } +} + void _setMockMediaUploadPlatformHandler( Future Function(MethodCall call)? handler, ) { @@ -1502,6 +1514,55 @@ void main() { ); }); + testWidgets('cancels a pending upload without showing an error', ( + tester, + ) async { + final client = _AbortAwareClient(); + final uploadService = MediaUploadService( + baseUrl: 'https://relay.example', + nsec: nostr.Keys.generate().nsec, + httpClient: client, + pickGalleryVideo: () async => null, + pickGalleryImage: () async => null, + pickGalleryImages: () async => [ + XFile.fromData(_pngBytes, name: 'tiny.png'), + ], + ); + + await tester.pumpWidget( + _buildComposeBar( + uploadService: uploadService, + onSend: + ( + content, + mentionPubkeys, { + mediaTags = const >[], + }) async {}, + ), + ); + + await _openSystemPhotoPicker(tester); + await client.started.future; + await tester.pump(); + expect( + find.byKey(const ValueKey('compose-upload-progress')), + findsOneWidget, + ); + expect(find.bySemanticsLabel(RegExp(r'Cancel upload')), findsOneWidget); + + await tester.tap(find.byKey(const ValueKey('compose-upload-cancel'))); + await tester.pumpAndSettle(); + + expect( + find.byKey(const ValueKey('compose-upload-progress')), + findsNothing, + ); + expect( + find.byKey(const ValueKey('compose-upload-error-dismiss')), + findsNothing, + ); + }); + testWidgets('renders markdown formatting without visible delimiters', ( tester, ) async { @@ -2117,6 +2178,16 @@ void main() { await tester.pumpAndSettle(); expect(find.textContaining('upload failed'), findsOneWidget); + expect( + find.byKey(const ValueKey('compose-upload-error-dismiss')), + findsOneWidget, + ); + + await tester.tap( + find.byKey(const ValueKey('compose-upload-error-dismiss')), + ); + await tester.pumpAndSettle(); + expect(find.textContaining('upload failed'), findsNothing); }); for (final statusCode in [ diff --git a/mobile/test/shared/relay/media_upload_test.dart b/mobile/test/shared/relay/media_upload_test.dart index dcd04359b2..baed135b20 100644 --- a/mobile/test/shared/relay/media_upload_test.dart +++ b/mobile/test/shared/relay/media_upload_test.dart @@ -1,3 +1,4 @@ +import 'dart:async'; import 'dart:convert'; import 'dart:io'; @@ -238,6 +239,18 @@ final _animatedWebpBytes = Uint8List.fromList([ const _mediaUploadPlatformChannel = MethodChannel('buzz/media_upload'); +class _AbortAwareClient extends http.BaseClient { + final started = Completer(); + + @override + Future send(http.BaseRequest request) async { + if (!started.isCompleted) started.complete(); + final abortTrigger = (request as http.AbortableRequest).abortTrigger; + await abortTrigger; + throw http.RequestAbortedException(request.url); + } +} + void _setMockMediaUploadPlatformHandler( Future Function(MethodCall call)? handler, ) { @@ -353,6 +366,40 @@ void main() { }); group('MediaUploadService', () { + test('cancels an in-flight upload', () async { + final client = _AbortAwareClient(); + final service = MediaUploadService( + baseUrl: 'https://relay.example', + nsec: nostr.Keys.generate().nsec, + httpClient: client, + pickGalleryVideo: () async => null, + pickGalleryImage: () async => null, + ); + + final upload = service.uploadBytes(_pngBytes, mimeType: 'image/png'); + await client.started.future; + service.cancelPendingUploads(); + + await expectLater(upload, throwsA(isA())); + }); + + test('times out an upload that never receives a response', () async { + final client = _AbortAwareClient(); + final service = MediaUploadService( + baseUrl: 'https://relay.example', + nsec: nostr.Keys.generate().nsec, + httpClient: client, + mediaUploadTimeout: const Duration(milliseconds: 10), + pickGalleryVideo: () async => null, + pickGalleryImage: () async => null, + ); + + await expectLater( + service.uploadBytes(_pngBytes, mimeType: 'image/png'), + throwsA(isA()), + ); + }); + test('signs Blossom auth and uploads gallery image bytes', () async { final keychain = nostr.Keys.generate(); final nsec = keychain.nsec;