fix: Exceptions inside Query.snapshots() and more now have a stack tr…

…ace that correctly points to the invocation of the throwing method (#9639)
firebase · Oct 6, 2022 · 2f7adcb · 2f7adcb
1 parent 3a3e521
commit 2f7adcb
Show file tree

Hide file tree

Showing 10 changed files with 109 additions and 82 deletions.
diff --git a/packages/_flutterfire_internals/lib/src/exception.dart b/packages/_flutterfire_internals/lib/src/exception.dart
@@ -11,10 +11,15 @@ import 'package:flutter/services.dart';
 /// If the [Exception] is a [PlatformException], a [FirebaseException] is returned.
 Never convertPlatformExceptionToFirebaseException(
   Object exception,
-  StackTrace stackTrace, {
+  StackTrace rawStackTrace, {
   required String plugin,
 }) {
-  if (exception is! Exception || exception is! PlatformException) {
+  var stackTrace = rawStackTrace;
+  if (stackTrace == StackTrace.empty) {
+    stackTrace = StackTrace.current;
+  }
+
+  if (exception is! PlatformException) {
     Error.throwWithStackTrace(exception, stackTrace);
   }
 
@@ -51,3 +56,21 @@ FirebaseException platformExceptionToFirebaseException(
     message: message,
   );
 }
+
+/// A custom [EventChannel] with default error handling logic.
+extension EventChannelExtension on EventChannel {
+  /// Similar to [receiveBroadcastStream], but with enforced error handling.
+  Stream<dynamic> receiveGuardedBroadcastStream({
+    dynamic arguments,
+    required dynamic Function(Object error, StackTrace stackTrace) onError,
+  }) {
+    final incomingStackTrace = StackTrace.current;
+
+    return receiveBroadcastStream(arguments).handleError((Object error) {
+      // TODO(rrousselGit): use package:stack_trace to merge the error's StackTrace with "incomingStackTrace"
+      // This TODO assumes that EventChannel is updated to actually pass a StackTrace
+      // (as it currently only sends StackTrace.empty)
+      return onError(error, incomingStackTrace);
+    });
+  }
+}
diff --git a/...irestore_platform_interface/lib/src/method_channel/method_channel_document_reference.dart b/...irestore_platform_interface/lib/src/method_channel/method_channel_document_reference.dart
@@ -5,6 +5,7 @@
 
 import 'dart:async';
 
+import 'package:_flutterfire_internals/_flutterfire_internals.dart';
 import 'package:cloud_firestore_platform_interface/cloud_firestore_platform_interface.dart';
 import 'package:cloud_firestore_platform_interface/src/internal/pointer.dart';
 import 'package:flutter/services.dart';
@@ -103,38 +104,37 @@ class MethodChannelDocumentReference extends DocumentReferencePlatform {
     late StreamController<DocumentSnapshotPlatform>
         controller; // ignore: close_sinks
 
-    StreamSubscription<dynamic>? snapshotStream;
+    StreamSubscription<dynamic>? snapshotStreamSubscription;
     controller = StreamController<DocumentSnapshotPlatform>.broadcast(
       onListen: () async {
         final observerId = await MethodChannelFirebaseFirestore.channel
             .invokeMethod<String>('DocumentReference#snapshots');
-        snapshotStream =
+        snapshotStreamSubscription =
             MethodChannelFirebaseFirestore.documentSnapshotChannel(observerId!)
-                .receiveBroadcastStream(
-                  <String, dynamic>{
-                    'reference': this,
-                    'includeMetadataChanges': includeMetadataChanges,
-                  },
-                )
-                .handleError(convertPlatformException)
-                .listen(
-                  (snapshot) {
-                    controller.add(
-                      DocumentSnapshotPlatform(
-                        firestore,
-                        snapshot['path'],
-                        <String, dynamic>{
-                          'data': snapshot['data'],
-                          'metadata': snapshot['metadata'],
-                        },
-                      ),
-                    );
-                  },
-                  onError: controller.addError,
-                );
+                .receiveGuardedBroadcastStream(
+          arguments: <String, dynamic>{
+            'reference': this,
+            'includeMetadataChanges': includeMetadataChanges,
+          },
+          onError: convertPlatformException,
+        ).listen(
+          (snapshot) {
+            controller.add(
+              DocumentSnapshotPlatform(
+                firestore,
+                snapshot['path'],
+                <String, dynamic>{
+                  'data': snapshot['data'],
+                  'metadata': snapshot['metadata'],
+                },
+              ),
+            );
+          },
+          onError: controller.addError,
+        );
       },
       onCancel: () {
-        snapshotStream?.cancel();
+        snapshotStreamSubscription?.cancel();
       },
     );
 

diff --git a/...e/cloud_firestore_platform_interface/lib/src/method_channel/method_channel_firestore.dart b/...e/cloud_firestore_platform_interface/lib/src/method_channel/method_channel_firestore.dart
@@ -8,6 +8,7 @@ import 'dart:async';
 // ignore: unnecessary_import
 import 'dart:typed_data';
 
+import 'package:_flutterfire_internals/_flutterfire_internals.dart';
 import 'package:cloud_firestore_platform_interface/cloud_firestore_platform_interface.dart';
 import 'package:cloud_firestore_platform_interface/src/method_channel/method_channel_load_bundle_task.dart';
 import 'package:cloud_firestore_platform_interface/src/method_channel/method_channel_query_snapshot.dart';
@@ -190,29 +191,26 @@ class MethodChannelFirebaseFirestore extends FirebaseFirestorePlatform {
 
   @override
   Stream<void> snapshotsInSync() {
-    StreamSubscription<dynamic>? snapshotStream;
+    StreamSubscription<dynamic>? snapshotStreamSubscription;
     late StreamController<void> controller; // ignore: close_sinks
 
     controller = StreamController<void>.broadcast(
       onListen: () async {
         final observerId = await MethodChannelFirebaseFirestore.channel
             .invokeMethod<String>('SnapshotsInSync#setup');
 
-        snapshotStream =
+        snapshotStreamSubscription =
             MethodChannelFirebaseFirestore.snapshotsInSyncChannel(observerId!)
-                .receiveBroadcastStream(
-                  <String, dynamic>{
-                    'firestore': this,
-                  },
-                )
-                .handleError(convertPlatformException)
-                .listen(
-                  (event) => controller.add(null),
-                  onError: controller.addError,
-                );
+                .receiveGuardedBroadcastStream(
+          arguments: <String, dynamic>{'firestore': this},
+          onError: convertPlatformException,
+        ).listen(
+          (event) => controller.add(null),
+          onError: controller.addError,
+        );
       },
       onCancel: () {
-        snapshotStream?.cancel();
+        snapshotStreamSubscription?.cancel();
       },
     );
 
@@ -233,8 +231,6 @@ class MethodChannelFirebaseFirestore extends FirebaseFirestorePlatform {
       'Transaction#create',
     );
 
-    StreamSubscription<dynamic> snapshotStream;
-
     Completer<T> completer = Completer();
 
     // Will be set by the `transactionHandler`.
@@ -245,12 +241,14 @@ class MethodChannelFirebaseFirestore extends FirebaseFirestorePlatform {
       const StandardMethodCodec(FirestoreMessageCodec()),
     );
 
-    snapshotStream = eventChannel.receiveBroadcastStream(
-      <String, dynamic>{
+    final snapshotStreamSubscription =
+        eventChannel.receiveGuardedBroadcastStream(
+      arguments: <String, dynamic>{
         'firestore': this,
         'timeout': timeout.inMilliseconds,
         'maxAttempts': maxAttempts,
       },
+      onError: convertPlatformException,
     ).listen(
       (event) async {
         if (event['error'] != null) {
@@ -304,9 +302,7 @@ class MethodChannelFirebaseFirestore extends FirebaseFirestorePlatform {
       },
     );
 
-    return completer.future.whenComplete(() {
-      snapshotStream.cancel();
-    });
+    return completer.future.whenComplete(snapshotStreamSubscription.cancel);
   }
 
   @override

diff --git a/..._firestore_platform_interface/lib/src/method_channel/method_channel_load_bundle_task.dart b/..._firestore_platform_interface/lib/src/method_channel/method_channel_load_bundle_task.dart
@@ -37,6 +37,8 @@ class MethodChannelLoadBundleTask extends LoadBundleTaskPlatform {
           }
         }
       } catch (exception) {
+        // TODO this should be refactored to use `convertPlatformException`,
+        // then change receiveBroadcastStream -> receiveGuardedBroadedStream
         if (exception is! Exception || exception is! PlatformException) {
           rethrow;
         }

diff --git a/...store/cloud_firestore_platform_interface/lib/src/method_channel/method_channel_query.dart b/...store/cloud_firestore_platform_interface/lib/src/method_channel/method_channel_query.dart
@@ -5,6 +5,7 @@
 
 import 'dart:async';
 
+import 'package:_flutterfire_internals/_flutterfire_internals.dart';
 import 'package:cloud_firestore_platform_interface/cloud_firestore_platform_interface.dart';
 import 'package:cloud_firestore_platform_interface/src/internal/pointer.dart';
 import 'package:collection/collection.dart';
@@ -135,31 +136,30 @@ class MethodChannelQuery extends QueryPlatform {
     late StreamController<QuerySnapshotPlatform>
         controller; // ignore: close_sinks
 
-    StreamSubscription<dynamic>? snapshotStream;
+    StreamSubscription<dynamic>? snapshotStreamSubscription;
+
     controller = StreamController<QuerySnapshotPlatform>.broadcast(
       onListen: () async {
         final observerId = await MethodChannelFirebaseFirestore.channel
             .invokeMethod<String>('Query#snapshots');
 
-        snapshotStream =
+        snapshotStreamSubscription =
             MethodChannelFirebaseFirestore.querySnapshotChannel(observerId!)
-                .receiveBroadcastStream(
-                  <String, dynamic>{
-                    'query': this,
-                    'includeMetadataChanges': includeMetadataChanges,
-                  },
-                )
-                .handleError(convertPlatformException)
-                .listen(
-                  (snapshot) {
-                    controller
-                        .add(MethodChannelQuerySnapshot(firestore, snapshot));
-                  },
-                  onError: controller.addError,
-                );
+                .receiveGuardedBroadcastStream(
+          arguments: <String, dynamic>{
+            'query': this,
+            'includeMetadataChanges': includeMetadataChanges,
+          },
+          onError: convertPlatformException,
+        ).listen(
+          (snapshot) {
+            controller.add(MethodChannelQuerySnapshot(firestore, snapshot));
+          },
+          onError: controller.addError,
+        );
       },
       onCancel: () {
-        snapshotStream?.cancel();
+        snapshotStreamSubscription?.cancel();
       },
     );
 

diff --git a/...pp_check_platform_interface/lib/src/method_channel/method_channel_firebase_app_check.dart b/...pp_check_platform_interface/lib/src/method_channel/method_channel_firebase_app_check.dart
@@ -5,6 +5,7 @@
 import 'dart:async';
 import 'dart:io';
 
+import 'package:_flutterfire_internals/_flutterfire_internals.dart';
 import 'package:firebase_core/firebase_core.dart';
 import 'package:flutter/foundation.dart';
 import 'package:flutter/services.dart';
@@ -22,7 +23,9 @@ class MethodChannelFirebaseAppCheck extends FirebaseAppCheckPlatform {
       'appName': app.name,
     }).then((channelName) {
       final events = EventChannel(channelName!, channel.codec);
-      events.receiveBroadcastStream().listen(
+      events
+          .receiveGuardedBroadcastStream(onError: convertPlatformException)
+          .listen(
         (arguments) {
           // ignore: close_sinks
           StreamController<String?> controller =

diff --git a/..._platform_interface/lib/src/method_channel/method_channel_firebase_app_installations.dart b/..._platform_interface/lib/src/method_channel/method_channel_firebase_app_installations.dart
@@ -4,6 +4,7 @@
 
 import 'dart:async';
 
+import 'package:_flutterfire_internals/_flutterfire_internals.dart';
 import 'package:firebase_core/firebase_core.dart';
 import 'package:firebase_app_installations_platform_interface/firebase_app_installations_platform_interface.dart';
 import 'package:flutter/services.dart';
@@ -37,9 +38,9 @@ class MethodChannelFirebaseAppInstallations
       'appName': app.name,
     }).then((channelName) {
       final events = EventChannel(channelName!, channel.codec);
+
       events
-          .receiveBroadcastStream()
-          .handleError(convertPlatformException)
+          .receiveGuardedBroadcastStream(onError: convertPlatformException)
           .listen(
             (Object? arguments) => controller.add((arguments as Map)['token']),
             onError: controller.addError,

diff --git a/...firebase_auth_platform_interface/lib/src/method_channel/method_channel_firebase_auth.dart b/...firebase_auth_platform_interface/lib/src/method_channel/method_channel_firebase_auth.dart
@@ -6,6 +6,7 @@
 import 'dart:async';
 import 'dart:io' show Platform;
 
+import 'package:_flutterfire_internals/_flutterfire_internals.dart';
 import 'package:firebase_auth_platform_interface/src/method_channel/method_channel_multi_factor.dart';
 import 'package:firebase_auth_platform_interface/src/method_channel/utils/convert_auth_provider.dart';
 import 'package:firebase_core/firebase_core.dart';
@@ -67,7 +68,9 @@ class MethodChannelFirebaseAuth extends FirebaseAuthPlatform {
       'appName': app.name,
     }).then((channelName) {
       final events = EventChannel(channelName!, channel.codec);
-      events.receiveBroadcastStream().listen(
+      events
+          .receiveGuardedBroadcastStream(onError: convertPlatformException)
+          .listen(
         (arguments) {
           _handleIdTokenChangesListener(app.name, arguments);
         },
@@ -78,7 +81,9 @@ class MethodChannelFirebaseAuth extends FirebaseAuthPlatform {
       'appName': app.name,
     }).then((channelName) {
       final events = EventChannel(channelName!, channel.codec);
-      events.receiveBroadcastStream().listen(
+      events
+          .receiveGuardedBroadcastStream(onError: convertPlatformException)
+          .listen(
         (arguments) {
           _handleAuthStateChangesListener(app.name, arguments);
         },
@@ -636,7 +641,7 @@ class MethodChannelFirebaseAuth extends FirebaseAuthPlatform {
           }));
 
       EventChannel(eventChannelName!)
-          .receiveBroadcastStream()
+          .receiveGuardedBroadcastStream(onError: convertPlatformException)
           .listen((arguments) {
         final name = arguments['name'];
         if (name == 'Auth#phoneVerificationCompleted') {

diff --git a/packages/firebase_auth/firebase_auth_platform_interface/pubspec.yaml b/packages/firebase_auth/firebase_auth_platform_interface/pubspec.yaml
@@ -11,6 +11,7 @@ environment:
   flutter: '>=1.9.1+hotfix.5'
 
 dependencies:
+  _flutterfire_internals: ^1.0.0
   collection: ^1.16.0
   firebase_core: ^1.10.0
   flutter:

diff --git a/...ase/firebase_database_platform_interface/lib/src/method_channel/method_channel_query.dart b/...ase/firebase_database_platform_interface/lib/src/method_channel/method_channel_query.dart
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import 'package:_flutterfire_internals/_flutterfire_internals.dart';
 import 'package:firebase_database_platform_interface/firebase_database_platform_interface.dart';
 import 'package:flutter/services.dart';
 
@@ -53,18 +54,13 @@ class MethodChannelQuery extends QueryPlatform {
       }),
     );
 
-    yield* EventChannel(channelName!)
-        .receiveBroadcastStream(<String, Object?>{
-          'eventType': eventTypeToString(eventType),
-        })
-        .map(
-          (event) =>
-              MethodChannelDatabaseEvent(ref, Map<String, dynamic>.from(event)),
-        )
-        .handleError(
-          convertPlatformException,
-          test: (err) => err is PlatformException,
-        );
+    yield* EventChannel(channelName!).receiveGuardedBroadcastStream(
+      arguments: <String, Object?>{'eventType': eventTypeToString(eventType)},
+      onError: convertPlatformException,
+    ).map(
+      (event) =>
+          MethodChannelDatabaseEvent(ref, Map<String, dynamic>.from(event)),
+    );
   }
 
   /// Gets the most up-to-date result for this query.