feat(*): refactor error handling to preserve stack traces on platform…

… exceptions (#8156) Co-authored-by: Mike Diarmid <mike.diarmid@gmail.com>
firebase · Feb 24, 2022 · 6ac77d9 · 6ac77d9
1 parent 960a75a
commit 6ac77d9
Show file tree

Hide file tree

Showing 47 changed files with 548 additions and 403 deletions.
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
@@ -40,8 +40,8 @@ class MethodChannelDocumentReference extends DocumentReferencePlatform {
           },
         },
       );
-    } catch (e) {
-      throw convertPlatformException(e);
+    } catch (e, stack) {
+      convertPlatformException(e, stack);
     }
   }
 
@@ -56,8 +56,8 @@ class MethodChannelDocumentReference extends DocumentReferencePlatform {
           'data': data,
         },
       );
-    } catch (e) {
-      throw convertPlatformException(e);
+    } catch (e, stack) {
+      convertPlatformException(e, stack);
     }
   }
 
@@ -77,8 +77,8 @@ class MethodChannelDocumentReference extends DocumentReferencePlatform {
       );
 
       return DocumentSnapshotPlatform(firestore, _pointer.path, data!);
-    } catch (e) {
-      throw convertPlatformException(e);
+    } catch (e, stack) {
+      convertPlatformException(e, stack);
     }
   }
 
@@ -89,8 +89,8 @@ class MethodChannelDocumentReference extends DocumentReferencePlatform {
         'DocumentReference#delete',
         <String, dynamic>{'firestore': firestore, 'reference': this},
       );
-    } catch (e) {
-      throw convertPlatformException(e);
+    } catch (e, stack) {
+      convertPlatformException(e, stack);
     }
   }
 
@@ -111,24 +111,27 @@ class MethodChannelDocumentReference extends DocumentReferencePlatform {
         snapshotStream =
             MethodChannelFirebaseFirestore.documentSnapshotChannel(observerId!)
                 .receiveBroadcastStream(
-          <String, dynamic>{
-            'reference': this,
-            'includeMetadataChanges': includeMetadataChanges,
-          },
-        ).listen((snapshot) {
-          controller.add(
-            DocumentSnapshotPlatform(
-              firestore,
-              snapshot['path'],
-              <String, dynamic>{
-                'data': snapshot['data'],
-                'metadata': snapshot['metadata'],
-              },
-            ),
-          );
-        }, onError: (error, stack) {
-          controller.addError(convertPlatformException(error), stack);
-        });
+                  <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,
+                );
       },
       onCancel: () {
         snapshotStream?.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
@@ -107,16 +107,20 @@ class MethodChannelFirebaseFirestore extends FirebaseFirestorePlatform {
         FirebaseFirestorePlatform.instance,
         data!,
       );
-    } catch (e) {
+    } catch (e, stack) {
       if (e.toString().contains('Named query has not been found')) {
-        throw FirebaseException(
+        Error.throwWithStackTrace(
+          FirebaseException(
             plugin: 'cloud_firestore',
             code: 'non-existent-named-query',
-            message:
-                'Named query has not been found. Please check it has been loaded properly via loadBundle().');
+            message: 'Named query has not been found. '
+                'Please check it has been loaded properly via loadBundle().',
+          ),
+          stack,
+        );
       }
 
-      throw convertPlatformException(e);
+      convertPlatformException(e, stack);
     }
   }
 
@@ -130,8 +134,8 @@ class MethodChannelFirebaseFirestore extends FirebaseFirestorePlatform {
           .invokeMethod<void>('Firestore#clearPersistence', <String, dynamic>{
         'firestore': this,
       });
-    } catch (e) {
-      throw convertPlatformException(e);
+    } catch (e, stack) {
+      convertPlatformException(e, stack);
     }
   }
 
@@ -160,8 +164,8 @@ class MethodChannelFirebaseFirestore extends FirebaseFirestorePlatform {
           .invokeMethod<void>('Firestore#disableNetwork', <String, dynamic>{
         'firestore': this,
       });
-    } catch (e) {
-      throw convertPlatformException(e);
+    } catch (e, stack) {
+      convertPlatformException(e, stack);
     }
   }
 
@@ -177,8 +181,8 @@ class MethodChannelFirebaseFirestore extends FirebaseFirestorePlatform {
           .invokeMethod<void>('Firestore#enableNetwork', <String, dynamic>{
         'firestore': this,
       });
-    } catch (e) {
-      throw convertPlatformException(e);
+    } catch (e, stack) {
+      convertPlatformException(e, stack);
     }
   }
 
@@ -195,14 +199,15 @@ class MethodChannelFirebaseFirestore extends FirebaseFirestorePlatform {
         snapshotStream =
             MethodChannelFirebaseFirestore.snapshotsInSyncChannel(observerId!)
                 .receiveBroadcastStream(
-          <String, dynamic>{
-            'firestore': this,
-          },
-        ).listen((event) {
-          controller.add(null);
-        }, onError: (error, stack) {
-          controller.addError(convertPlatformException(error), stack);
-        });
+                  <String, dynamic>{
+                    'firestore': this,
+                  },
+                )
+                .handleError(convertPlatformException)
+                .listen(
+                  (event) => controller.add(null),
+                  onError: controller.addError,
+                );
       },
       onCancel: () {
         snapshotStream?.cancel();
@@ -306,8 +311,8 @@ class MethodChannelFirebaseFirestore extends FirebaseFirestorePlatform {
       await channel.invokeMethod<void>('Firestore#terminate', <String, dynamic>{
         'firestore': this,
       });
-    } catch (e) {
-      throw convertPlatformException(e);
+    } catch (e, stack) {
+      convertPlatformException(e, stack);
     }
   }
 
@@ -318,8 +323,8 @@ class MethodChannelFirebaseFirestore extends FirebaseFirestorePlatform {
           'Firestore#waitForPendingWrites', <String, dynamic>{
         'firestore': this,
       });
-    } catch (e) {
-      throw convertPlatformException(e);
+    } catch (e, stack) {
+      convertPlatformException(e, stack);
     }
   }
 }
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
@@ -106,8 +106,8 @@ class MethodChannelQuery extends QueryPlatform {
       );
 
       return MethodChannelQuerySnapshot(firestore, data!);
-    } catch (e) {
-      throw convertPlatformException(e);
+    } catch (e, stack) {
+      convertPlatformException(e, stack);
     }
   }
 
@@ -145,15 +145,19 @@ class MethodChannelQuery extends QueryPlatform {
         snapshotStream =
             MethodChannelFirebaseFirestore.querySnapshotChannel(observerId!)
                 .receiveBroadcastStream(
-          <String, dynamic>{
-            'query': this,
-            'includeMetadataChanges': includeMetadataChanges,
-          },
-        ).listen((snapshot) {
-          controller.add(MethodChannelQuerySnapshot(firestore, snapshot));
-        }, onError: (error, stack) {
-          controller.addError(convertPlatformException(error), stack);
-        });
+                  <String, dynamic>{
+                    'query': this,
+                    'includeMetadataChanges': includeMetadataChanges,
+                  },
+                )
+                .handleError(convertPlatformException)
+                .listen(
+                  (snapshot) {
+                    controller
+                        .add(MethodChannelQuerySnapshot(firestore, snapshot));
+                  },
+                  onError: controller.addError,
+                );
       },
       onCancel: () {
         snapshotStream?.cancel();

diff --git a/...cloud_firestore_platform_interface/lib/src/method_channel/method_channel_write_batch.dart b/...cloud_firestore_platform_interface/lib/src/method_channel/method_channel_write_batch.dart
@@ -48,8 +48,8 @@ class MethodChannelWriteBatch extends WriteBatchPlatform {
         'firestore': _firestore,
         'writes': _writes,
       });
-    } catch (e) {
-      throw convertPlatformException(e);
+    } catch (e, stack) {
+      convertPlatformException(e, stack);
     }
   }
 

diff --git a/..._firestore/cloud_firestore_platform_interface/lib/src/method_channel/utils/exception.dart b/..._firestore/cloud_firestore_platform_interface/lib/src/method_channel/utils/exception.dart
@@ -9,12 +9,15 @@ import 'package:flutter/services.dart';
 /// Catches a [PlatformException] and returns an [Exception].
 ///
 /// If the [Exception] is a [PlatformException], a [FirebaseException] is returned.
-Exception convertPlatformException(Object exception) {
+Never convertPlatformException(Object exception, StackTrace stackTrace) {
   if (exception is! Exception || exception is! PlatformException) {
-    throw exception;
+    Error.throwWithStackTrace(exception, stackTrace);
   }
 
-  return platformExceptionToFirebaseException(exception);
+  Error.throwWithStackTrace(
+    platformExceptionToFirebaseException(exception),
+    stackTrace,
+  );
 }
 
 /// Converts a [PlatformException] into a [FirebaseException].

diff --git a/...ud_functions_platform_interface/lib/src/method_channel/method_channel_https_callable.dart b/...ud_functions_platform_interface/lib/src/method_channel/method_channel_https_callable.dart
@@ -36,7 +36,7 @@ class MethodChannelHttpsCallable extends HttpsCallablePlatform {
         return result;
       }
     } catch (e, s) {
-      throw convertPlatformException(e, s);
+      convertPlatformException(e, s);
     }
   }
 }
diff --git a/..._functions/cloud_functions_platform_interface/lib/src/method_channel/utils/exception.dart b/..._functions/cloud_functions_platform_interface/lib/src/method_channel/utils/exception.dart
@@ -11,12 +11,15 @@ import '../../../cloud_functions_platform_interface.dart';
 /// Catches a [PlatformException] and returns an [Exception].
 ///
 /// If the [Exception] is a [PlatformException], a [FirebaseFunctionsException] is returned.
-Exception convertPlatformException(Object exception, [StackTrace? stackTrace]) {
+Never convertPlatformException(Object exception, StackTrace stackTrace) {
   if (exception is! Exception || exception is! PlatformException) {
-    throw exception;
+    Error.throwWithStackTrace(exception, stackTrace);
   }
 
-  return platformExceptionToFirebaseFunctionsException(exception, stackTrace);
+  Error.throwWithStackTrace(
+    platformExceptionToFirebaseFunctionsException(exception, stackTrace),
+    stackTrace,
+  );
 }
 
 /// Converts a [PlatformException] into a [FirebaseFunctionsException].
@@ -25,8 +28,9 @@ Exception convertPlatformException(Object exception, [StackTrace? stackTrace]) {
 /// the `details` of the exception exist. Firebase returns specific codes and
 /// messages which can be converted into user friendly exceptions.
 FirebaseException platformExceptionToFirebaseFunctionsException(
-    PlatformException platformException,
-    [StackTrace? stackTrace]) {
+  PlatformException platformException,
+  StackTrace stackTrace,
+) {
   Map<String, dynamic>? details = platformException.details != null
       ? Map<String, dynamic>.from(platformException.details)
       : null;
@@ -41,8 +45,9 @@ FirebaseException platformExceptionToFirebaseFunctionsException(
   }
 
   return FirebaseFunctionsException(
-      code: code,
-      message: message!,
-      details: additionalData,
-      stackTrace: stackTrace);
+    code: code,
+    message: message!,
+    details: additionalData,
+    stackTrace: stackTrace,
+  );
 }
diff --git a/...unctions/cloud_functions_platform_interface/test/method_channel/utils/exception_test.dart b/...unctions/cloud_functions_platform_interface/test/method_channel/utils/exception_test.dart
@@ -17,8 +17,10 @@ void main() {
     test('should throw any exception', () async {
       AssertionError assertionError = AssertionError();
 
-      expect(() => convertPlatformException(assertionError),
-          throwsA(isA<AssertionError>()));
+      expect(
+        () => convertPlatformException(assertionError, StackTrace.empty),
+        throwsA(isA<AssertionError>()),
+      );
     });
 
     test(
@@ -30,11 +32,14 @@ void main() {
       );
 
       expect(
-          convertPlatformException(platformException),
+        () => convertPlatformException(platformException, StackTrace.empty),
+        throwsA(
           isA<FirebaseFunctionsException>()
               .having((e) => e.code, 'code', 'unknown')
               .having((e) => e.message, 'message', testMessage)
-              .having((e) => e.details, 'details', isNull));
+              .having((e) => e.details, 'details', isNull),
+        ),
+      );
     });
 
     test('should override code and message if provided to additional details',
@@ -46,11 +51,14 @@ void main() {
           details: {'code': code, 'message': testMessage});
 
       expect(
-          convertPlatformException(platformException),
+        () => convertPlatformException(platformException, StackTrace.empty),
+        throwsA(
           isA<FirebaseFunctionsException>()
               .having((e) => e.code, 'code', code)
               .having((e) => e.message, 'message', testMessage)
-              .having((e) => e.details, 'details', isNull));
+              .having((e) => e.details, 'details', isNull),
+        ),
+      );
     });
 
     test('should provide additionalData as details', () async {
@@ -60,15 +68,18 @@ void main() {
           details: {'additionalData': testAdditionalData});
 
       expect(
-          convertPlatformException(platformException),
+        () => convertPlatformException(platformException, StackTrace.empty),
+        throwsA(
           isA<FirebaseFunctionsException>()
               .having((e) => e.code, 'code', 'unknown')
               .having((e) => e.message, 'message', testMessage)
               .having(
                   (e) => e.details,
                   'details',
                   isA<Map<String, dynamic>>()
-                      .having((e) => e['foo'], 'additionalData', 'bar')));
+                      .having((e) => e['foo'], 'additionalData', 'bar')),
+        ),
+      );
     });
   });
 }