From 2f7adcb777cd6bc4e3b5b3dd03c975c725bacef7 Mon Sep 17 00:00:00 2001 From: Remi Rousselet Date: Thu, 6 Oct 2022 09:57:28 +0200 Subject: [PATCH] fix: Exceptions inside Query.snapshots() and more now have a stack trace that correctly points to the invocation of the throwing method (#9639) --- .../lib/src/exception.dart | 27 +++++++++- .../method_channel_document_reference.dart | 50 +++++++++---------- .../method_channel_firestore.dart | 36 ++++++------- .../method_channel_load_bundle_task.dart | 2 + .../method_channel/method_channel_query.dart | 34 ++++++------- .../method_channel_firebase_app_check.dart | 5 +- ...od_channel_firebase_app_installations.dart | 5 +- .../method_channel_firebase_auth.dart | 11 ++-- .../pubspec.yaml | 1 + .../method_channel/method_channel_query.dart | 20 +++----- 10 files changed, 109 insertions(+), 82 deletions(-) diff --git a/packages/_flutterfire_internals/lib/src/exception.dart b/packages/_flutterfire_internals/lib/src/exception.dart index a6426c178751..4d70bb6795dc 100644 --- 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 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/packages/cloud_firestore/cloud_firestore_platform_interface/lib/src/method_channel/method_channel_document_reference.dart b/packages/cloud_firestore/cloud_firestore_platform_interface/lib/src/method_channel/method_channel_document_reference.dart index bdb3874e7658..a24f3f22e744 100644 --- a/packages/cloud_firestore/cloud_firestore_platform_interface/lib/src/method_channel/method_channel_document_reference.dart +++ b/packages/cloud_firestore/cloud_firestore_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 controller; // ignore: close_sinks - StreamSubscription? snapshotStream; + StreamSubscription? snapshotStreamSubscription; controller = StreamController.broadcast( onListen: () async { final observerId = await MethodChannelFirebaseFirestore.channel .invokeMethod('DocumentReference#snapshots'); - snapshotStream = + snapshotStreamSubscription = MethodChannelFirebaseFirestore.documentSnapshotChannel(observerId!) - .receiveBroadcastStream( - { - 'reference': this, - 'includeMetadataChanges': includeMetadataChanges, - }, - ) - .handleError(convertPlatformException) - .listen( - (snapshot) { - controller.add( - DocumentSnapshotPlatform( - firestore, - snapshot['path'], - { - 'data': snapshot['data'], - 'metadata': snapshot['metadata'], - }, - ), - ); - }, - onError: controller.addError, - ); + .receiveGuardedBroadcastStream( + arguments: { + 'reference': this, + 'includeMetadataChanges': includeMetadataChanges, + }, + onError: convertPlatformException, + ).listen( + (snapshot) { + controller.add( + DocumentSnapshotPlatform( + firestore, + snapshot['path'], + { + 'data': snapshot['data'], + 'metadata': snapshot['metadata'], + }, + ), + ); + }, + onError: controller.addError, + ); }, onCancel: () { - snapshotStream?.cancel(); + snapshotStreamSubscription?.cancel(); }, ); diff --git a/packages/cloud_firestore/cloud_firestore_platform_interface/lib/src/method_channel/method_channel_firestore.dart b/packages/cloud_firestore/cloud_firestore_platform_interface/lib/src/method_channel/method_channel_firestore.dart index 87dda067e301..043ac2e9e259 100644 --- a/packages/cloud_firestore/cloud_firestore_platform_interface/lib/src/method_channel/method_channel_firestore.dart +++ b/packages/cloud_firestore/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,7 +191,7 @@ class MethodChannelFirebaseFirestore extends FirebaseFirestorePlatform { @override Stream snapshotsInSync() { - StreamSubscription? snapshotStream; + StreamSubscription? snapshotStreamSubscription; late StreamController controller; // ignore: close_sinks controller = StreamController.broadcast( @@ -198,21 +199,18 @@ class MethodChannelFirebaseFirestore extends FirebaseFirestorePlatform { final observerId = await MethodChannelFirebaseFirestore.channel .invokeMethod('SnapshotsInSync#setup'); - snapshotStream = + snapshotStreamSubscription = MethodChannelFirebaseFirestore.snapshotsInSyncChannel(observerId!) - .receiveBroadcastStream( - { - 'firestore': this, - }, - ) - .handleError(convertPlatformException) - .listen( - (event) => controller.add(null), - onError: controller.addError, - ); + .receiveGuardedBroadcastStream( + arguments: {'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 snapshotStream; - Completer completer = Completer(); // Will be set by the `transactionHandler`. @@ -245,12 +241,14 @@ class MethodChannelFirebaseFirestore extends FirebaseFirestorePlatform { const StandardMethodCodec(FirestoreMessageCodec()), ); - snapshotStream = eventChannel.receiveBroadcastStream( - { + final snapshotStreamSubscription = + eventChannel.receiveGuardedBroadcastStream( + arguments: { '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/packages/cloud_firestore/cloud_firestore_platform_interface/lib/src/method_channel/method_channel_load_bundle_task.dart b/packages/cloud_firestore/cloud_firestore_platform_interface/lib/src/method_channel/method_channel_load_bundle_task.dart index 8104a9fd6d03..9a48188cf826 100644 --- a/packages/cloud_firestore/cloud_firestore_platform_interface/lib/src/method_channel/method_channel_load_bundle_task.dart +++ b/packages/cloud_firestore/cloud_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/packages/cloud_firestore/cloud_firestore_platform_interface/lib/src/method_channel/method_channel_query.dart b/packages/cloud_firestore/cloud_firestore_platform_interface/lib/src/method_channel/method_channel_query.dart index 997a7086d71f..e24b1065d30c 100644 --- a/packages/cloud_firestore/cloud_firestore_platform_interface/lib/src/method_channel/method_channel_query.dart +++ b/packages/cloud_firestore/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 controller; // ignore: close_sinks - StreamSubscription? snapshotStream; + StreamSubscription? snapshotStreamSubscription; + controller = StreamController.broadcast( onListen: () async { final observerId = await MethodChannelFirebaseFirestore.channel .invokeMethod('Query#snapshots'); - snapshotStream = + snapshotStreamSubscription = MethodChannelFirebaseFirestore.querySnapshotChannel(observerId!) - .receiveBroadcastStream( - { - 'query': this, - 'includeMetadataChanges': includeMetadataChanges, - }, - ) - .handleError(convertPlatformException) - .listen( - (snapshot) { - controller - .add(MethodChannelQuerySnapshot(firestore, snapshot)); - }, - onError: controller.addError, - ); + .receiveGuardedBroadcastStream( + arguments: { + 'query': this, + 'includeMetadataChanges': includeMetadataChanges, + }, + onError: convertPlatformException, + ).listen( + (snapshot) { + controller.add(MethodChannelQuerySnapshot(firestore, snapshot)); + }, + onError: controller.addError, + ); }, onCancel: () { - snapshotStream?.cancel(); + snapshotStreamSubscription?.cancel(); }, ); diff --git a/packages/firebase_app_check/firebase_app_check_platform_interface/lib/src/method_channel/method_channel_firebase_app_check.dart b/packages/firebase_app_check/firebase_app_check_platform_interface/lib/src/method_channel/method_channel_firebase_app_check.dart index a975f903a968..fb5e08e8119f 100644 --- a/packages/firebase_app_check/firebase_app_check_platform_interface/lib/src/method_channel/method_channel_firebase_app_check.dart +++ b/packages/firebase_app_check/firebase_app_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 controller = diff --git a/packages/firebase_app_installations/firebase_app_installations_platform_interface/lib/src/method_channel/method_channel_firebase_app_installations.dart b/packages/firebase_app_installations/firebase_app_installations_platform_interface/lib/src/method_channel/method_channel_firebase_app_installations.dart index 857f0da523b8..f692aef90d71 100644 --- a/packages/firebase_app_installations/firebase_app_installations_platform_interface/lib/src/method_channel/method_channel_firebase_app_installations.dart +++ b/packages/firebase_app_installations/firebase_app_installations_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/packages/firebase_auth/firebase_auth_platform_interface/lib/src/method_channel/method_channel_firebase_auth.dart b/packages/firebase_auth/firebase_auth_platform_interface/lib/src/method_channel/method_channel_firebase_auth.dart index 206784901bfc..485db6e7e0d6 100644 --- a/packages/firebase_auth/firebase_auth_platform_interface/lib/src/method_channel/method_channel_firebase_auth.dart +++ b/packages/firebase_auth/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 index 450c99ec0163..85a4bca0f8dd 100644 --- 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/packages/firebase_database/firebase_database_platform_interface/lib/src/method_channel/method_channel_query.dart b/packages/firebase_database/firebase_database_platform_interface/lib/src/method_channel/method_channel_query.dart index 662731b0261f..ddbd9ecc0ce0 100755 --- a/packages/firebase_database/firebase_database_platform_interface/lib/src/method_channel/method_channel_query.dart +++ b/packages/firebase_database/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({ - 'eventType': eventTypeToString(eventType), - }) - .map( - (event) => - MethodChannelDatabaseEvent(ref, Map.from(event)), - ) - .handleError( - convertPlatformException, - test: (err) => err is PlatformException, - ); + yield* EventChannel(channelName!).receiveGuardedBroadcastStream( + arguments: {'eventType': eventTypeToString(eventType)}, + onError: convertPlatformException, + ).map( + (event) => + MethodChannelDatabaseEvent(ref, Map.from(event)), + ); } /// Gets the most up-to-date result for this query.