[query] - Accept errorCallback for query.on()

Author: KkevinLi

src/app/database/index.ts

@@ -2,20 +2,7 @@ import * as firebase from "../../firebase";
 import { nextPushId } from "./util/NextPushId";
 
 export namespace database {
-  export interface DataSnapshot {
-    // child(path: string): DataSnapshot;
-    exists(): boolean;
-    // exportVal(): any;
-    // forEach(action: (a: DataSnapshot) => boolean): boolean;
-    // getPriority(): string | number | null;
-    // hasChild(path: string): boolean;
-    // hasChildren(): boolean;
-    key: string | null;
-    // numChildren(): number;
-    // ref: Reference;
-    // toJSON(): Object | null;
-    val(): any;
-  }
+  export type DataSnapshot = firebase.DataSnapshot;
 
   export class Query implements firebase.Query {
     protected path: string;
@@ -29,12 +16,13 @@ export namespace database {
      * Listens for data changes at a particular location
      * @param eventType One of the following strings: "value", "child_added", "child_changed", "child_removed", or "child_moved."
      * @param callback A callback that fires when the specified event occurs. The callback will be passed a DataSnapshot.
+     * @param cancelCallbackOrContext A callback that fires when an error occurs. The callback will be passed an error object.
      * @returns The provided callback function is returned unmodified.
      */
     public on(eventType: string, callback: (a: DataSnapshot | null, b?: string) => any,
-      cancelCallbackOrContext?: Object | null, context?: Object | null): (a: DataSnapshot | null, b?: string) => Function {
+      cancelCallbackOrContext?: (a: Error | null) => any, context?: Object | null): (a: DataSnapshot | null, b?: string) => Function {
 
-      this.queryObject.on(eventType, callback);
+      this.queryObject.on(eventType, callback, cancelCallbackOrContext);
 
       return callback; // According to firebase doc we just return the callback given
     }

src/firebase.android.ts

@@ -1640,25 +1640,19 @@ class Query implements QueryBase {
     this.query = this.dbRef;
   }
 
-  on(eventType: string, callback: (a: any, b?: string) => any): Function {
-    const onValueEvent = result => {
-      callback(result);
-    };
-
+  on(eventType: string, callback: (a: DataSnapshot, b?: string) => any, cancelCallbackOrContext?: (a: Error | null) => any): Function {
     try {
       if (firebase.instance === null) {
         throw new Error("Run init() first!");
       }
-      const listener = this.createEventListener(eventType, onValueEvent);
+      const listener = this.createEventListener(eventType, callback, cancelCallbackOrContext);
 
       if (eventType === "value") {
         this.query.addValueEventListener(listener as com.google.firebase.database.ValueEventListener);
       } else if (eventType === "child_added" || eventType === "child_changed" || eventType === "child_removed" || eventType === "child_moved") {
         this.query.addChildEventListener(listener as com.google.firebase.database.ChildEventListener);
       } else {
-        callback({
-          error: "Invalid eventType.  Use one of the following: 'value', 'child_added', 'child_changed', 'child_removed', or 'child_moved'"
-        });
+        throw new Error(`${eventType} is not a valid eventType. Use one of the following: 'value', 'child_added', 'child_changed', 'child_removed', or 'child_moved'`);
       }
       // Add listener to our map which keeps track of eventType: child/value events
       if (!Query.eventListenerMap.has(eventType)) {
@@ -1667,6 +1661,9 @@ class Query implements QueryBase {
       Query.eventListenerMap.get(eventType).push(listener); // We need to keep track of the listeners to fully remove them when calling off
     } catch (ex) {
       console.error("Error in firebase.on: " + ex);
+      if (cancelCallbackOrContext !== undefined) {
+        cancelCallbackOrContext(ex);
+      }
     } finally {
       return callback;
     }
@@ -1693,7 +1690,7 @@ class Query implements QueryBase {
         firebase.instance.child(this.path).addListenerForSingleValueEvent(listener);
       }
       catch (ex) {
-        console.log("Error in firebase.once: " + ex);
+        console.error("Error in firebase.once: " + ex);
         reject(ex);
       }
     });
@@ -1776,7 +1773,7 @@ class Query implements QueryBase {
   * to specific events (Android is more generic value / child - which includes all events add, change, remove etc).
   * Similar to firebase._addObserver but I do not want to listen for every event
   */
-  private createEventListener(eventType: string, callback): com.google.firebase.database.ValueEventListener | com.google.firebase.database.ChildEventListener {
+  private createEventListener(eventType: string, callback, cancelCallback?): com.google.firebase.database.ValueEventListener | com.google.firebase.database.ChildEventListener {
     let listener;
 
     if (eventType === "value") {
@@ -1785,17 +1782,17 @@ class Query implements QueryBase {
           callback(nativeSnapshotToWebSnapshot(snapshot));
         },
         onCancelled: (databaseError: com.google.firebase.database.DatabaseError) => {
-          callback({
-            error: databaseError.getMessage()
-          });
+          if (cancelCallback !== undefined) {
+            cancelCallback(new Error(databaseError.getMessage()));
+          }
         }
       });
     } else if (eventType === "child_added" || eventType === "child_changed" || eventType === "child_removed" || eventType === "child_moved") {
       listener = new com.google.firebase.database.ChildEventListener({
         onCancelled: (databaseError: com.google.firebase.database.DatabaseError) => {
-          callback({
-            error: databaseError.getMessage()
-          });
+           if (cancelCallback !== undefined) {
+            cancelCallback(new Error(databaseError.getMessage()));
+          }
         },
         onChildAdded: (snapshot: com.google.firebase.database.DataSnapshot, previousChildKey: string) => {
           if (eventType === "child_added") {

src/firebase.d.ts

@@ -549,7 +549,7 @@ export interface OnDisconnect {
 
 // WebAPI Query
 export interface Query {
-  on(eventType: string, callback: (a: any, b?: string) => any): Function;
+  on(eventType: string, callback: (a: any, b?: string) => any, cancelCallbackOrContext?: (a: Error | null) => any): Function;
 
   once(eventType: string): Promise<DataSnapshot>;
 

src/firebase.ios.ts

@@ -1402,30 +1402,29 @@ class Query implements QueryBase {
     this.query = this.dbRef;
   }
 
-  on(eventType: string, callback: (a: any, b?: string) => any): Function {
-    const onValueEvent = result => {
-      callback(result);
-    };
-      try {
-        if (eventType === "value" || eventType === "child_added" || eventType === "child_changed"
-          || eventType === "child_removed" || eventType === "child_moved") {
-          const firDataEventType = this.eventToFIRDataEventType(eventType);
-          const firDatabaseHandle = this.attachEventObserver(this.query, firDataEventType, onValueEvent);
-          if (!Query.eventListenerMap.has(eventType)) {
-            Query.eventListenerMap.set(eventType, []);
-          }
-          Query.eventListenerMap.get(eventType).push(firDatabaseHandle); // We need to keep track of the listeners to fully remove them when calling off
-        } else {
-          callback({
-            error: "Invalid eventType.  Use one of the following: 'value', 'child_added', 'child_changed', 'child_removed', or 'child_moved'"
-          });
+  on(eventType: string, callback: (a: any, b?: string) => any, cancelCallbackOrContext?: (a: Error | null) => any): Function {
+    try {
+      if (eventType === "value" || eventType === "child_added" || eventType === "child_changed"
+        || eventType === "child_removed" || eventType === "child_moved") {
+        const firDataEventType = this.eventToFIRDataEventType(eventType);
+        const firDatabaseHandle = this.attachEventObserver(this.query, firDataEventType, callback, cancelCallbackOrContext);
+        if (!Query.eventListenerMap.has(eventType)) {
+          Query.eventListenerMap.set(eventType, []);
         }
-      } catch (ex) {
-        console.log("Error in firebase.on: " + ex);
+        Query.eventListenerMap.get(eventType).push(firDatabaseHandle); // We need to keep track of the listeners to fully remove them when calling off
+      } else {
+        throw new Error(`${eventType} is not a valid eventType. Use one of the following: 'value', 'child_added', 'child_changed', 'child_removed', or 'child_moved'`);
       }
-      finally {
-        return callback;
+    } catch (ex) {
+      // TODO: Make custom errors
+      console.error("Error in firebase.on: " + ex);
+      if (cancelCallbackOrContext !== undefined) {
+        cancelCallbackOrContext(ex);
       }
+    }
+    finally {
+      return callback;
+    }
   }
 
   once(eventType: string): Promise<DataSnapshot> {
@@ -1443,7 +1442,7 @@ class Query implements QueryBase {
             });
           });
       } catch (ex) {
-        console.log("Error in firebase.once: " + ex);
+        console.error("Error in firebase.once: " + ex);
         reject(ex);
       }
     });
@@ -1548,16 +1547,16 @@ class Query implements QueryBase {
   * to specific events (Android is more generic value / child - which includes all events add, change, remove etc).
   * Similar to firebase._addObserver but I do not want to listen for every event
   */
-  private attachEventObserver(dbRef: FIRDatabaseQuery | FIRDatabaseReference, firEventType: FIRDataEventType, callback): number {
+  private attachEventObserver(dbRef: FIRDatabaseQuery | FIRDatabaseReference, firEventType: FIRDataEventType, callback, cancelCallback): number {
     const listener = dbRef.observeEventTypeWithBlockWithCancelBlock(
       firEventType,
       snapshot => {
         callback(nativeSnapshotToWebSnapshot(snapshot));
       },
       firebaseError => {
-        callback({
-          error: firebaseError.localizedDescription
-        });
+        if (cancelCallback !== undefined) {
+          cancelCallback(new Error(firebaseError.localizedDescription));
+        }
       });
     return listener;
   }