fix(storage): ensure Task listeners correctly propagate exceptions an…

…d close properly. (#12160)
firebase · Jan 17, 2024 · 759684b · 759684b
1 parent 4630f8e
commit 759684b
Show file tree

Hide file tree

Showing 7 changed files with 141 additions and 37 deletions.
diff --git a/...droid/src/main/java/io/flutter/plugins/firebase/storage/FlutterFirebaseStoragePlugin.java b/...droid/src/main/java/io/flutter/plugins/firebase/storage/FlutterFirebaseStoragePlugin.java
@@ -50,10 +50,8 @@ static Map<String, String> getExceptionDetails(Exception exception) {
     GeneratedAndroidFirebaseStorage.FlutterError storageException =
         FlutterFirebaseStorageException.parserExceptionToFlutter(exception);
 
-    if (storageException != null) {
-      details.put("code", storageException.code);
-      details.put("message", storageException.getMessage());
-    }
+    details.put("code", storageException.code);
+    details.put("message", storageException.getMessage());
 
     return details;
   }

diff --git a/...roid/src/main/java/io/flutter/plugins/firebase/storage/TaskStateChannelStreamHandler.java b/...roid/src/main/java/io/flutter/plugins/firebase/storage/TaskStateChannelStreamHandler.java
@@ -83,8 +83,7 @@ public void onListen(Object arguments, EventSink events) {
           Map<String, Object> event = getTaskEventMap(null, exception);
           event.put(
               TASK_STATE_NAME, GeneratedAndroidFirebaseStorage.PigeonStorageTaskState.ERROR.index);
-          events.error(
-              FlutterFirebaseStoragePlugin.DEFAULT_ERROR_CODE, exception.getMessage(), event);
+          events.success(event);
           flutterTask.destroy();
         });
   }

diff --git a/packages/firebase_storage/firebase_storage/ios/Classes/FLTFirebaseStoragePlugin.h b/packages/firebase_storage/firebase_storage/ios/Classes/FLTFirebaseStoragePlugin.h
@@ -18,4 +18,5 @@
     : FLTFirebasePlugin <FlutterPlugin, FLTFirebasePlugin, FirebaseStorageHostApi>
 
 + (NSDictionary *)parseTaskSnapshot:(FIRStorageTaskSnapshot *)snapshot;
++ (NSDictionary *)NSDictionaryFromNSError:(NSError *)error;
 @end
diff --git a/packages/firebase_storage/firebase_storage/ios/Classes/FLTFirebaseStoragePlugin.m b/packages/firebase_storage/firebase_storage/ios/Classes/FLTFirebaseStoragePlugin.m
@@ -712,7 +712,7 @@ - (void)setState:(FLTFirebaseStorageTaskState)state
   completion(NO, nil);
 }
 
-- (NSDictionary *)NSDictionaryFromNSError:(NSError *)error {
++ (NSDictionary *)NSDictionaryFromNSError:(NSError *)error {
   NSMutableDictionary *dictionary = [[NSMutableDictionary alloc] init];
   NSString *code = @"unknown";
   NSString *message = [error localizedDescription];
@@ -765,7 +765,7 @@ - (FlutterError *_Nullable)FlutterErrorFromNSError:(NSError *_Nullable)error {
   if (error == nil) {
     return nil;
   }
-  NSDictionary *dictionary = [self NSDictionaryFromNSError:error];
+  NSDictionary *dictionary = [FLTFirebaseStoragePlugin NSDictionaryFromNSError:error];
   return [FlutterError errorWithCode:dictionary[@"code"]
                              message:dictionary[@"message"]
                              details:@{}];
@@ -779,7 +779,7 @@ - (NSDictionary *)NSDictionaryFromHandle:(NSNumber *)handle
       [FLTFirebasePlugin firebaseAppNameFromIosName:snapshot.reference.storage.app.name];
   dictionary[kFLTFirebaseStorageKeyBucket] = snapshot.reference.bucket;
   if (snapshot.error != nil) {
-    dictionary[@"error"] = [self NSDictionaryFromNSError:snapshot.error];
+    dictionary[@"error"] = [FLTFirebaseStoragePlugin NSDictionaryFromNSError:snapshot.error];
   } else {
     dictionary[kFLTFirebaseStorageKeySnapshot] =
         [FLTFirebaseStoragePlugin parseTaskSnapshot:snapshot];

diff --git a/packages/firebase_storage/firebase_storage/ios/Classes/FLTTaskStateChannelStreamHandler.m b/packages/firebase_storage/firebase_storage/ios/Classes/FLTTaskStateChannelStreamHandler.m
@@ -41,7 +41,7 @@ - (FlutterError *)onListenWithArguments:(id)arguments eventSink:(FlutterEventSin
                      events(@{
                        @"taskState" : @(PigeonStorageTaskStateError),
                        @"appName" : snapshot.reference.storage.app.name,
-                       @"snapshot" : [FLTFirebaseStoragePlugin parseTaskSnapshot:snapshot],
+                       @"error" : [FLTFirebaseStoragePlugin NSDictionaryFromNSError:snapshot.error],
                      });
                    }];
   pausedHandle =

diff --git a/...orage/firebase_storage_platform_interface/lib/src/method_channel/method_channel_task.dart b/...orage/firebase_storage_platform_interface/lib/src/method_channel/method_channel_task.dart
@@ -6,6 +6,7 @@
 import 'dart:async';
 import 'dart:io';
 
+import 'package:firebase_core/firebase_core.dart';
 import 'package:flutter/services.dart';
 
 import '../../firebase_storage_platform_interface.dart';
@@ -35,21 +36,61 @@ abstract class MethodChannelTask extends TaskPlatform {
         await for (final events in nativePlatformStream) {
           final taskState = TaskState.values[events['taskState']];
 
-          if (_snapshot.state != TaskState.canceled) {
+          if (taskState == TaskState.error) {
+            _didComplete = true;
+            final errorMap = Map<String, dynamic>.from(events['error']);
+            final exception = FirebaseException(
+              plugin: 'firebase_storage',
+              code: errorMap['code'],
+              message: errorMap['message'],
+            );
+            _snapshot = MethodChannelTaskSnapshot(
+              storage,
+              taskState,
+              // We use previous snapshot data as errors from native do not provide snapshot data.
+              {
+                'path': path,
+                'bytesTransferred': _snapshot.bytesTransferred,
+                'totalBytes': _snapshot.totalBytes,
+                'metadata': _snapshot.metadata
+              },
+            );
+            _completer?.completeError(exception);
+            if (_userListening) {
+              // If the user is listening to the stream, yield the error. Otherwise, it results in an unhandled exception.
+              yield* Stream.error(exception);
+            }
+
+            break;
+          }
+          if (taskState == TaskState.canceled) {
+            _didComplete = true;
             MethodChannelTaskSnapshot snapshot = MethodChannelTaskSnapshot(
                 storage,
                 taskState,
                 Map<String, dynamic>.from(events['snapshot']));
             _snapshot = snapshot;
+            break;
           }
 
-          yield _snapshot;
+          if (taskState == TaskState.success ||
+              taskState == TaskState.running ||
+              taskState == TaskState.paused) {
+            MethodChannelTaskSnapshot snapshot = MethodChannelTaskSnapshot(
+                storage,
+                taskState,
+                Map<String, dynamic>.from(events['snapshot']));
+            _snapshot = snapshot;
+
+            yield snapshot;
+          }
 
           // If the stream event is complete, trigger the
           // completer to resolve with the snapshot.
-          if (snapshot.state == TaskState.success) {
+          if (taskState == TaskState.success) {
             _didComplete = true;
             _completer?.complete(snapshot);
+            break;
           }
         }
       } catch (exception, stack) {
@@ -58,7 +99,7 @@ abstract class MethodChannelTask extends TaskPlatform {
     }
 
     _stream = mapNativeStream().asBroadcastStream(
-        onListen: (sub) => sub.resume(), onCancel: (sub) => sub.pause());
+        onListen: (sub) => sub.resume(), onCancel: (sub) => sub.cancel());
 
     // Keep reference to whether the initial "start" task has completed.
     _snapshot = MethodChannelTaskSnapshot(storage, TaskState.running, {
@@ -68,6 +109,8 @@ abstract class MethodChannelTask extends TaskPlatform {
     });
   }
 
+  bool _userListening = false;
+
   ///  FirebaseApp pigeon instance
   static PigeonStorageFirebaseApp pigeonFirebaseApp(
       FirebaseStoragePlatform storage) {
@@ -114,6 +157,7 @@ abstract class MethodChannelTask extends TaskPlatform {
 
   @override
   Stream<TaskSnapshotPlatform> get snapshotEvents {
+    _userListening = true;
     return _stream;
   }
 

diff --git a/tests/integration_test/firebase_storage/task_e2e.dart b/tests/integration_test/firebase_storage/task_e2e.dart
@@ -2,6 +2,7 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
+import 'dart:convert';
 import 'dart:io';
 
 import 'package:firebase_storage/firebase_storage.dart';
@@ -116,35 +117,63 @@ void setupTaskTests() {
         skip: defaultTargetPlatform == TargetPlatform.macOS,
       );
 
-      //TODO(pr-mais): causes the emulator to crash
-      // test('handles errors, e.g. if permission denied', () async {
-      //   late FirebaseException streamError;
+      test('handles errors, e.g. if permission denied for `snapshotEvents`',
+          () async {
+        late FirebaseException streamError;
 
-      //   List<int> list = utf8.encode('hello world');
-      //   Uint8List data = Uint8List.fromList(list);
-      //   UploadTask task = storage.ref('/uploadNope.jpeg').putData(data);
+        List<int> list = utf8.encode('hello world');
+        Uint8List data = Uint8List.fromList(list);
+        UploadTask task = storage.ref('/uploadNope.jpeg').putData(data);
 
-      //   expect(task.snapshot.state, TaskState.running);
+        bool callsDoneWhenFinished = false;
+        task.snapshotEvents.listen(
+          (TaskSnapshot snapshot) {
+            // noop
+          },
+          onError: (error) {
+            streamError = error;
+          },
+          onDone: () {
+            callsDoneWhenFinished = true;
+          },
+        );
+        // Allow time for listener events to be called
+        await Future.delayed(
+          const Duration(seconds: 2),
+        );
+
+        expect(callsDoneWhenFinished, isTrue);
 
-      //   task.snapshotEvents.listen((TaskSnapshot snapshot) {
-      //     // noop
-      //   }, onError: (error) {
-      //     streamError = error;
-      //   }, cancelOnError: true);
+        expect(streamError.plugin, 'firebase_storage');
+        expect(streamError.code, 'unauthorized');
+        expect(
+          streamError.message,
+          'User is not authorized to perform the desired action.',
+        );
 
-      //   await expectLater(
-      //     task,
-      //     throwsA(isA<FirebaseException>()
-      //         .having((e) => e.code, 'code', 'unauthorized')),
-      //   );
+        expect(task.snapshot.state, TaskState.error);
+      });
 
-      //   expect(streamError.plugin, 'firebase_storage');
-      //   expect(streamError.code, 'unauthorized');
-      //   expect(streamError.message,
-      //       'User is not authorized to perform the desired action.');
+      test('handles errors, e.g. if permission denied for `await Task`',
+          () async {
+        List<int> list = utf8.encode('hello world');
+        Uint8List data = Uint8List.fromList(list);
+        UploadTask task = storage.ref('/uploadNope.jpeg').putData(data);
+        try {
+          await task;
+        } catch (e) {
+          expect(e, isA<FirebaseException>());
+          FirebaseException exception = e as FirebaseException;
+          expect(exception.plugin, 'firebase_storage');
+          expect(exception.code, 'unauthorized');
+          expect(
+            exception.message,
+            'User is not authorized to perform the desired action.',
+          );
+        }
 
-      //   expect(task.snapshot.state, TaskState.error);
-      // });
+        expect(task.snapshot.state, TaskState.error);
+      });
     });
 
     group('snapshot', () {
@@ -247,5 +276,38 @@ void setupTaskTests() {
       },
       skip: true, // Cancel still cannot get correct result in e2e test.
     );
+
+    group('snapshotEvents', () {
+      test('loop through successful `snapshotEvents`', () async {
+        final snapshots = <TaskSnapshot>[];
+        final task = uploadRef.putString('This is an upload task!');
+        // ignore: prefer_foreach
+        await for (final event in task.snapshotEvents) {
+          snapshots.add(event);
+        }
+        expect(snapshots.last.state, TaskState.success);
+      });
+
+      test('failed `snapshotEvents` loop', () async {
+        final snapshots = <TaskSnapshot>[];
+        UploadTask task =
+            storage.ref('/uploadNope.jpeg').putString('This will fail');
+        try {
+          // ignore: prefer_foreach
+          await for (final event in task.snapshotEvents) {
+            snapshots.add(event);
+          }
+        } catch (e) {
+          expect(e, isA<FirebaseException>());
+          FirebaseException exception = e as FirebaseException;
+          expect(exception.plugin, 'firebase_storage');
+          expect(exception.code, 'unauthorized');
+          expect(
+            exception.message,
+            'User is not authorized to perform the desired action.',
+          );
+        }
+      });
+    });
   });
 }