Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Exceptions inside Query.snapshots() and more now have a stack trace that correctly points to the invocation of the throwing method #9639

Merged
merged 2 commits into from Oct 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
27 changes: 25 additions & 2 deletions packages/_flutterfire_internals/lib/src/exception.dart
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
});
}
}
Expand Up @@ -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';
Expand Down Expand Up @@ -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();
},
);

Expand Down
Expand Up @@ -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';
Expand Down Expand Up @@ -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();
},
);

Expand All @@ -233,8 +231,6 @@ class MethodChannelFirebaseFirestore extends FirebaseFirestorePlatform {
'Transaction#create',
);

StreamSubscription<dynamic> snapshotStream;

Completer<T> completer = Completer();

// Will be set by the `transactionHandler`.
Expand All @@ -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) {
Expand Down Expand Up @@ -304,9 +302,7 @@ class MethodChannelFirebaseFirestore extends FirebaseFirestorePlatform {
},
);

return completer.future.whenComplete(() {
snapshotStream.cancel();
});
return completer.future.whenComplete(snapshotStreamSubscription.cancel);
}

@override
Expand Down
Expand Up @@ -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;
}
Expand Down
Expand Up @@ -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';
Expand Down Expand Up @@ -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();
},
);

Expand Down
Expand Up @@ -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';
Expand All @@ -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 =
Expand Down
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand Down
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
},
Expand All @@ -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);
},
Expand Down Expand Up @@ -636,7 +641,7 @@ class MethodChannelFirebaseAuth extends FirebaseAuthPlatform {
}));

EventChannel(eventChannelName!)
.receiveBroadcastStream()
.receiveGuardedBroadcastStream(onError: convertPlatformException)
.listen((arguments) {
final name = arguments['name'];
if (name == 'Auth#phoneVerificationCompleted') {
Expand Down
Expand Up @@ -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:
Expand Down
Expand Up @@ -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';

Expand Down Expand Up @@ -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.
Expand Down