From 1a378c708befc5e703e9baddf8d57b4c90884dce Mon Sep 17 00:00:00 2001 From: Mikkel Ravn Date: Fri, 15 Dec 2017 22:03:10 +0100 Subject: [PATCH 1/5] Basic error event communication --- .../database/FirebaseDatabasePlugin.java | 21 +++++++--- .../firebase_database/example/lib/main.dart | 13 +++++- .../ios/Classes/FirebaseDatabasePlugin.m | 17 +++++++- .../lib/src/firebase_database.dart | 35 ++++++++++------ .../lib/ui/firebase_list.dart | 21 +++++++--- .../lib/ui/firebase_sorted_list.dart | 20 +++++++--- .../lib/ui/utils/stream_subscriber_mixin.dart | 4 +- .../test/firebase_database_test.dart | 40 +++++++++++++++++++ 8 files changed, 135 insertions(+), 36 deletions(-) diff --git a/packages/firebase_database/android/src/main/java/io/flutter/plugins/firebase/database/FirebaseDatabasePlugin.java b/packages/firebase_database/android/src/main/java/io/flutter/plugins/firebase/database/FirebaseDatabasePlugin.java index a8ac10ce2bab..59a942d3dd59 100644 --- a/packages/firebase_database/android/src/main/java/io/flutter/plugins/firebase/database/FirebaseDatabasePlugin.java +++ b/packages/firebase_database/android/src/main/java/io/flutter/plugins/firebase/database/FirebaseDatabasePlugin.java @@ -179,7 +179,12 @@ private void sendEvent(String eventType, DataSnapshot snapshot, String previousC } @Override - public void onCancelled(DatabaseError error) {} + public void onCancelled(DatabaseError error) { + Map arguments = new HashMap<>(); + arguments.put("handle", handle); + arguments.put("error", asMap(error)); + channel.invokeMethod("Error", arguments); + } @Override public void onChildAdded(DataSnapshot snapshot, String previousChildName) { @@ -371,11 +376,7 @@ public void onComplete( Map completionMap = new HashMap<>(); completionMap.put("transactionKey", arguments.get("transactionKey")); if (databaseError != null) { - Map errorMap = new HashMap<>(); - errorMap.put("code", databaseError.getCode()); - errorMap.put("message", databaseError.getMessage()); - errorMap.put("details", databaseError.getDetails()); - completionMap.put("error", errorMap); + completionMap.put("error", asMap(databaseError)); } completionMap.put("committed", committed); if (dataSnapshot != null) { @@ -445,4 +446,12 @@ public void onComplete( } } } + + private static Map asMap(DatabaseError error) { + Map map = new HashMap<>(); + map.put("code", error.getCode()); + map.put("message", error.getMessage()); + map.put("details", error.getDetails()); + return map; + } } diff --git a/packages/firebase_database/example/lib/main.dart b/packages/firebase_database/example/lib/main.dart index 3ba755c86f8e..cb99bb827b7c 100755 --- a/packages/firebase_database/example/lib/main.dart +++ b/packages/firebase_database/example/lib/main.dart @@ -40,6 +40,7 @@ class _MyHomePageState extends State { String _kTestKey = 'Hello'; String _kTestValue = 'world!'; + DatabaseError _error; @override void initState() { @@ -49,12 +50,19 @@ class _MyHomePageState extends State { _counterRef.keepSynced(true); _counterSubscription = _counterRef.onValue.listen((Event event) { setState(() { + _error = null; _counter = event.snapshot.value ?? 0; }); + }, onError: (DatabaseError error) { + setState(() { + _error = error; + }); }); _messagesSubscription = _messagesRef.limitToLast(10).onChildAdded.listen((Event event) { print('Child added: ${event.snapshot.value}'); + }, onError: (DatabaseError error) { + print('Error: ${error.code} ${error.message}'); }); } @@ -96,10 +104,11 @@ class _MyHomePageState extends State { children: [ new Flexible( child: new Center( - // ignore: prefer_const_constructors - child: new Text( + child: _error == null ? new Text( 'Button tapped $_counter time${ _counter == 1 ? '' : 's' }.\n\n' 'This includes all devices, ever.', + ) : new Text( + 'Error retrieving button tap count:\n${_error.message}' ), ), ), diff --git a/packages/firebase_database/ios/Classes/FirebaseDatabasePlugin.m b/packages/firebase_database/ios/Classes/FirebaseDatabasePlugin.m index aa35794c4bcd..c9d7947e3752 100644 --- a/packages/firebase_database/ios/Classes/FirebaseDatabasePlugin.m +++ b/packages/firebase_database/ios/Classes/FirebaseDatabasePlugin.m @@ -11,6 +11,7 @@ @interface FLTFirebaseDatabasePlugin () @interface NSError (FlutterError) @property(readonly, nonatomic) FlutterError *flutterError; +@property(readonly, nonatomic) NSDictionary *dictionary; @end @implementation NSError (FlutterError) @@ -19,6 +20,14 @@ - (FlutterError *)flutterError { message:self.domain details:self.localizedDescription]; } + +- (NSDictionary *)dictionary { + return @{ + @"code" : @(self.code), + @"message" : self.domain ?: [NSNull null], + @"details" : self.localizedDescription ?: [NSNull null], + }; +} @end FIRDatabaseReference *getReference(NSDictionary *arguments) { @@ -252,7 +261,7 @@ - (void)handleMethodCall:(FlutterMethodCall *)call result:(FlutterResult)result // Invoke transaction completion on the Dart side. result(@{ @"transactionKey" : call.arguments[@"transactionKey"], - @"error" : error.flutterError ?: [NSNull null], + @"error" : error.dictionary ?: [NSNull null], @"committed" : [NSNumber numberWithBool:committed], @"snapshot" : @{@"key" : snapshot.key ?: [NSNull null], @"value" : snapshot.value} }); @@ -271,6 +280,12 @@ - (void)handleMethodCall:(FlutterMethodCall *)call result:(FlutterResult)result }, @"previousSiblingKey" : previousSiblingKey ?: [NSNull null], }]; + } withCancelBlock:^(NSError *error) { + [self.channel invokeMethod:@"Error" + arguments:@{ + @"handle" : [NSNumber numberWithUnsignedInteger:handle], + @"error" : error.dictionary, + }]; }]; result([NSNumber numberWithUnsignedInteger:handle]); } else if ([@"Query#removeObserver" isEqualToString:call.method]) { diff --git a/packages/firebase_database/lib/src/firebase_database.dart b/packages/firebase_database/lib/src/firebase_database.dart index 4ab151e94420..22b45075d680 100644 --- a/packages/firebase_database/lib/src/firebase_database.dart +++ b/packages/firebase_database/lib/src/firebase_database.dart @@ -20,19 +20,28 @@ class FirebaseDatabase { FirebaseDatabase._() { _channel.setMethodCallHandler((MethodCall call) async { - if (call.method == 'Event') { - final Event event = new Event._(call.arguments); - _observers[call.arguments['handle']].add(event); - } else if (call.method == 'DoTransaction') { - final MutableData mutableData = - new MutableData.private(call.arguments['snapshot']); - final MutableData updated = - await _transactions[call.arguments['transactionKey']](mutableData); - return {'value': updated.value}; - } else { - throw new MissingPluginException( - '${call.method} method not implemented on the Dart side.', - ); + switch (call.method) { + case 'Event': + print('Got Event'); + final Event event = new Event._(call.arguments); + _observers[call.arguments['handle']].add(event); + return null; + case 'Error': + print('Got Error'); + final DatabaseError error = new DatabaseError._(call.arguments['error']); + print('Interpreted Error'); + _observers[call.arguments['handle']].addError(error); + return null; + case 'DoTransaction': + final MutableData mutableData = + new MutableData.private(call.arguments['snapshot']); + final MutableData updated = await _transactions[ + call.arguments['transactionKey']](mutableData); + return {'value': updated.value}; + default: + throw new MissingPluginException( + '${call.method} method not implemented on the Dart side.', + ); } }); } diff --git a/packages/firebase_database/lib/ui/firebase_list.dart b/packages/firebase_database/lib/ui/firebase_list.dart index f68c2b0f48cf..4a2b6d3dcd42 100644 --- a/packages/firebase_database/lib/ui/firebase_list.dart +++ b/packages/firebase_database/lib/ui/firebase_list.dart @@ -6,13 +6,14 @@ import 'dart:collection'; import 'package:meta/meta.dart'; -import '../firebase_database.dart' show DataSnapshot, Event, Query; +import '../firebase_database.dart' show DatabaseError, DataSnapshot, Event, Query; import 'utils/stream_subscriber_mixin.dart'; typedef void ChildCallback(int index, DataSnapshot snapshot); typedef void ChildMovedCallback( int fromIndex, int toIndex, DataSnapshot snapshot); typedef void ValueCallback(DataSnapshot snapshot); +typedef void ErrorCallback(DatabaseError error); /// Sorts the results of `query` on the client side using `DataSnapshot.key`. class FirebaseList extends ListBase @@ -24,13 +25,14 @@ class FirebaseList extends ListBase this.onChildChanged, this.onChildMoved, this.onValue, + this.onError, }) { assert(query != null); - listen(query.onChildAdded, _onChildAdded); - listen(query.onChildRemoved, _onChildRemoved); - listen(query.onChildChanged, _onChildChanged); - listen(query.onChildMoved, _onChildMoved); - listen(query.onValue, _onValue); + listen(query.onChildAdded, _onChildAdded, onError: _onError); + listen(query.onChildRemoved, _onChildRemoved, onError: _onError); + listen(query.onChildChanged, _onChildChanged, onError: _onError); + listen(query.onChildMoved, _onChildMoved, onError: _onError); + listen(query.onValue, _onValue, onError: _onError); } /// Database query used to populate the list @@ -51,6 +53,9 @@ class FirebaseList extends ListBase /// Called when the data of the list has finished loading final ValueCallback onValue; + /// Called when an error is reported (e.g. permission denied) + final ErrorCallback onError; + // ListBase implementation final List _snapshots = []; @@ -127,4 +132,8 @@ class FirebaseList extends ListBase void _onValue(Event event) { onValue(event.snapshot); } + + void _onError(DatabaseError error) { + onError?.call(error); + } } diff --git a/packages/firebase_database/lib/ui/firebase_sorted_list.dart b/packages/firebase_database/lib/ui/firebase_sorted_list.dart index bd6c96b44705..299ca9f74983 100644 --- a/packages/firebase_database/lib/ui/firebase_sorted_list.dart +++ b/packages/firebase_database/lib/ui/firebase_sorted_list.dart @@ -6,8 +6,8 @@ import 'dart:collection'; import 'package:meta/meta.dart'; -import '../firebase_database.dart' show DataSnapshot, Event, Query; -import 'firebase_list.dart' show ChildCallback, ValueCallback; +import '../firebase_database.dart' show DatabaseError, DataSnapshot, Event, Query; +import 'firebase_list.dart' show ChildCallback, ErrorCallback, ValueCallback; import 'utils/stream_subscriber_mixin.dart'; /// Sorts the results of `query` on the client side using to the `comparator`. @@ -26,13 +26,14 @@ class FirebaseSortedList extends ListBase this.onChildRemoved, this.onChildChanged, this.onValue, + this.onError, }) { assert(query != null); assert(comparator != null); - listen(query.onChildAdded, _onChildAdded); - listen(query.onChildRemoved, _onChildRemoved); - listen(query.onChildChanged, _onChildChanged); - listen(query.onValue, _onValue); + listen(query.onChildAdded, _onChildAdded, onError: _onError); + listen(query.onChildRemoved, _onChildRemoved, onError: _onError); + listen(query.onChildChanged, _onChildChanged, onError: _onError); + listen(query.onValue, _onValue, onError: _onError); } /// Database query used to populate the list @@ -53,6 +54,9 @@ class FirebaseSortedList extends ListBase /// Called when the data of the list has finished loading final ValueCallback onValue; + /// Called when an error is reported (e.g. permission denied) + final ErrorCallback onError; + // ListBase implementation final List _snapshots = []; @@ -108,4 +112,8 @@ class FirebaseSortedList extends ListBase void _onValue(Event event) { onValue(event.snapshot); } + + void _onError(DatabaseError error) { + onError?.call(error); + } } diff --git a/packages/firebase_database/lib/ui/utils/stream_subscriber_mixin.dart b/packages/firebase_database/lib/ui/utils/stream_subscriber_mixin.dart index 1e890fe278a1..388934c802a8 100644 --- a/packages/firebase_database/lib/ui/utils/stream_subscriber_mixin.dart +++ b/packages/firebase_database/lib/ui/utils/stream_subscriber_mixin.dart @@ -10,9 +10,9 @@ abstract class StreamSubscriberMixin { List> _subscriptions = >[]; /// Listens to a stream and saves it to the list of subscriptions. - void listen(Stream stream, void onData(T data)) { + void listen(Stream stream, void onData(T data), { Function onError }) { if (stream != null) { - _subscriptions.add(stream.listen(onData)); + _subscriptions.add(stream.listen(onData, onError: onError)); } } diff --git a/packages/firebase_database/test/firebase_database_test.dart b/packages/firebase_database/test/firebase_database_test.dart index 45feb850fb91..37d0e10ab2c1 100755 --- a/packages/firebase_database/test/firebase_database_test.dart +++ b/packages/firebase_database/test/firebase_database_test.dart @@ -285,6 +285,46 @@ void main() { ], ); }); + test('observing error events', () async { + mockHandleId = 99; + const int errorCode = 12; + const String errorDetails = 'Some details'; + final Query query = database.reference().child('some path'); + Future simulateError(String errorMessage) async { + await BinaryMessages.handlePlatformMessage( + channel.name, + channel.codec.encodeMethodCall( + new MethodCall('Error', { + 'handle': 99, + 'error': { + 'code': errorCode, + 'message': errorMessage, + 'details': errorDetails, + }, + }), + ), + (_) {}, + ); + } + + final AsyncQueue errors = new AsyncQueue(); + + // Subscribe and allow subscription to complete. + final StreamSubscription subscription = + query.onValue.listen((_) {}, onError: errors.add); + await new Future.delayed(const Duration(seconds: 0)); + + await simulateError('Bad foo'); + await simulateError('Bad bar'); + final DatabaseError error1 = await errors.remove(); + final DatabaseError error2 = await errors.remove(); + expect(error1.code, errorCode); + expect(error1.message, 'Bad foo'); + expect(error1.details, errorDetails); + expect(error2.code, errorCode); + expect(error2.message, 'Bad bar'); + expect(error2.details, errorDetails); + }); test('observing value events', () async { mockHandleId = 87; final String path = 'foo'; From c3c42719c7582ec8e616ce09411474f4ece77772 Mon Sep 17 00:00:00 2001 From: Mikkel Ravn Date: Fri, 15 Dec 2017 23:54:32 +0100 Subject: [PATCH 2/5] Remove debug output --- packages/firebase_database/lib/src/firebase_database.dart | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/firebase_database/lib/src/firebase_database.dart b/packages/firebase_database/lib/src/firebase_database.dart index 22b45075d680..5d8b51650920 100644 --- a/packages/firebase_database/lib/src/firebase_database.dart +++ b/packages/firebase_database/lib/src/firebase_database.dart @@ -22,14 +22,11 @@ class FirebaseDatabase { _channel.setMethodCallHandler((MethodCall call) async { switch (call.method) { case 'Event': - print('Got Event'); final Event event = new Event._(call.arguments); _observers[call.arguments['handle']].add(event); return null; case 'Error': - print('Got Error'); final DatabaseError error = new DatabaseError._(call.arguments['error']); - print('Interpreted Error'); _observers[call.arguments['handle']].addError(error); return null; case 'DoTransaction': From c118594aac80b5da29676d19ebc133d7e9d4999d Mon Sep 17 00:00:00 2001 From: Mikkel Ravn Date: Sun, 17 Dec 2017 23:26:43 +0100 Subject: [PATCH 3/5] Fix formatting --- packages/firebase_database/example/lib/main.dart | 14 ++++++++------ .../ios/Classes/FirebaseDatabasePlugin.m | 6 +++--- .../lib/src/firebase_database.dart | 3 ++- .../lib/ui/firebase_sorted_list.dart | 3 ++- .../lib/ui/utils/stream_subscriber_mixin.dart | 2 +- .../test/firebase_database_test.dart | 3 ++- 6 files changed, 18 insertions(+), 13 deletions(-) diff --git a/packages/firebase_database/example/lib/main.dart b/packages/firebase_database/example/lib/main.dart index cb99bb827b7c..60acc5545d47 100755 --- a/packages/firebase_database/example/lib/main.dart +++ b/packages/firebase_database/example/lib/main.dart @@ -104,12 +104,14 @@ class _MyHomePageState extends State { children: [ new Flexible( child: new Center( - child: _error == null ? new Text( - 'Button tapped $_counter time${ _counter == 1 ? '' : 's' }.\n\n' - 'This includes all devices, ever.', - ) : new Text( - 'Error retrieving button tap count:\n${_error.message}' - ), + child: _error == null + ? new Text( + 'Button tapped $_counter time${ _counter == 1 ? '' : 's' }.\n\n' + 'This includes all devices, ever.', + ) + : new Text( + 'Error retrieving button tap count:\n${_error.message}', + ), ), ), new ListTile( diff --git a/packages/firebase_database/ios/Classes/FirebaseDatabasePlugin.m b/packages/firebase_database/ios/Classes/FirebaseDatabasePlugin.m index c9d7947e3752..6e0ac7d71ee1 100644 --- a/packages/firebase_database/ios/Classes/FirebaseDatabasePlugin.m +++ b/packages/firebase_database/ios/Classes/FirebaseDatabasePlugin.m @@ -268,8 +268,7 @@ - (void)handleMethodCall:(FlutterMethodCall *)call result:(FlutterResult)result }]; } else if ([@"Query#observe" isEqualToString:call.method]) { FIRDataEventType eventType = parseEventType(call.arguments[@"eventType"]); - __block FIRDatabaseHandle handle = [getQuery(call.arguments) - observeEventType:eventType + __block FIRDatabaseHandle handle = [getQuery(call.arguments) observeEventType:eventType andPreviousSiblingKeyWithBlock:^(FIRDataSnapshot *snapshot, NSString *previousSiblingKey) { [self.channel invokeMethod:@"Event" arguments:@{ @@ -280,7 +279,8 @@ - (void)handleMethodCall:(FlutterMethodCall *)call result:(FlutterResult)result }, @"previousSiblingKey" : previousSiblingKey ?: [NSNull null], }]; - } withCancelBlock:^(NSError *error) { + } + withCancelBlock:^(NSError *error) { [self.channel invokeMethod:@"Error" arguments:@{ @"handle" : [NSNumber numberWithUnsignedInteger:handle], diff --git a/packages/firebase_database/lib/src/firebase_database.dart b/packages/firebase_database/lib/src/firebase_database.dart index 5d8b51650920..bf09655363cd 100644 --- a/packages/firebase_database/lib/src/firebase_database.dart +++ b/packages/firebase_database/lib/src/firebase_database.dart @@ -26,7 +26,8 @@ class FirebaseDatabase { _observers[call.arguments['handle']].add(event); return null; case 'Error': - final DatabaseError error = new DatabaseError._(call.arguments['error']); + final DatabaseError error = + new DatabaseError._(call.arguments['error']); _observers[call.arguments['handle']].addError(error); return null; case 'DoTransaction': diff --git a/packages/firebase_database/lib/ui/firebase_sorted_list.dart b/packages/firebase_database/lib/ui/firebase_sorted_list.dart index 299ca9f74983..4ee02cfa7010 100644 --- a/packages/firebase_database/lib/ui/firebase_sorted_list.dart +++ b/packages/firebase_database/lib/ui/firebase_sorted_list.dart @@ -6,7 +6,8 @@ import 'dart:collection'; import 'package:meta/meta.dart'; -import '../firebase_database.dart' show DatabaseError, DataSnapshot, Event, Query; +import '../firebase_database.dart' + show DatabaseError, DataSnapshot, Event, Query; import 'firebase_list.dart' show ChildCallback, ErrorCallback, ValueCallback; import 'utils/stream_subscriber_mixin.dart'; diff --git a/packages/firebase_database/lib/ui/utils/stream_subscriber_mixin.dart b/packages/firebase_database/lib/ui/utils/stream_subscriber_mixin.dart index 388934c802a8..5bba816d30f6 100644 --- a/packages/firebase_database/lib/ui/utils/stream_subscriber_mixin.dart +++ b/packages/firebase_database/lib/ui/utils/stream_subscriber_mixin.dart @@ -10,7 +10,7 @@ abstract class StreamSubscriberMixin { List> _subscriptions = >[]; /// Listens to a stream and saves it to the list of subscriptions. - void listen(Stream stream, void onData(T data), { Function onError }) { + void listen(Stream stream, void onData(T data), {Function onError}) { if (stream != null) { _subscriptions.add(stream.listen(onData, onError: onError)); } diff --git a/packages/firebase_database/test/firebase_database_test.dart b/packages/firebase_database/test/firebase_database_test.dart index 37d0e10ab2c1..a0f4fa1494aa 100755 --- a/packages/firebase_database/test/firebase_database_test.dart +++ b/packages/firebase_database/test/firebase_database_test.dart @@ -307,7 +307,8 @@ void main() { ); } - final AsyncQueue errors = new AsyncQueue(); + final AsyncQueue errors = + new AsyncQueue(); // Subscribe and allow subscription to complete. final StreamSubscription subscription = From 556242fc5cee9ee2b2a8b9c26bfee3b6ac48d6ca Mon Sep 17 00:00:00 2001 From: Mikkel Ravn Date: Sun, 17 Dec 2017 23:31:03 +0100 Subject: [PATCH 4/5] Fix analyzer issue --- packages/firebase_database/test/firebase_database_test.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/firebase_database/test/firebase_database_test.dart b/packages/firebase_database/test/firebase_database_test.dart index a0f4fa1494aa..d89b8c8d9e95 100755 --- a/packages/firebase_database/test/firebase_database_test.dart +++ b/packages/firebase_database/test/firebase_database_test.dart @@ -319,6 +319,7 @@ void main() { await simulateError('Bad bar'); final DatabaseError error1 = await errors.remove(); final DatabaseError error2 = await errors.remove(); + subscription.cancel(); expect(error1.code, errorCode); expect(error1.message, 'Bad foo'); expect(error1.details, errorDetails); From c183ed1ef2017c4d1c0980454b130fcdfd19b46c Mon Sep 17 00:00:00 2001 From: Mikkel Ravn Date: Sun, 17 Dec 2017 23:37:38 +0100 Subject: [PATCH 5/5] Fix formatting --- packages/firebase_database/lib/ui/firebase_list.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/firebase_database/lib/ui/firebase_list.dart b/packages/firebase_database/lib/ui/firebase_list.dart index 4a2b6d3dcd42..d67bf52bad68 100644 --- a/packages/firebase_database/lib/ui/firebase_list.dart +++ b/packages/firebase_database/lib/ui/firebase_list.dart @@ -6,7 +6,8 @@ import 'dart:collection'; import 'package:meta/meta.dart'; -import '../firebase_database.dart' show DatabaseError, DataSnapshot, Event, Query; +import '../firebase_database.dart' + show DatabaseError, DataSnapshot, Event, Query; import 'utils/stream_subscriber_mixin.dart'; typedef void ChildCallback(int index, DataSnapshot snapshot);