From 18f3c2a933ce4fcebb2e3712ac10c0f147a6846c Mon Sep 17 00:00:00 2001
From: Simon Binder <oss@simonbinder.eu>
Date: Tue, 7 Mar 2023 20:41:28 +0100
Subject: [PATCH] Only serialize remote exceptions when needed

---
 drift/CHANGELOG.md                      |  2 ++
 drift/lib/src/remote/communication.dart |  9 ++----
 drift/lib/src/remote/protocol.dart      |  9 ++++--
 drift/test/isolate_test.dart            | 38 +++++++++++++++++++++++++
 drift/test/remote_test.dart             |  9 ++++++
 5 files changed, 58 insertions(+), 9 deletions(-)

diff --git a/drift/CHANGELOG.md b/drift/CHANGELOG.md
index 255999f3..f5129374 100644
--- a/drift/CHANGELOG.md
+++ b/drift/CHANGELOG.md
@@ -7,6 +7,8 @@
   visible alias for the rowid.
 - After opening a database with a higher schema version than the current one set
   in the database class, the schema version in the database will now be downgraded.
+- When using a drift isolate in the same engine group, errors on the remote end are
+  reported as-is instead of wrapping them in a `DriftRemoteException`.
 
 ## 2.5.0
 
diff --git a/drift/lib/src/remote/communication.dart b/drift/lib/src/remote/communication.dart
index 0462c615..98ab7c48 100644
--- a/drift/lib/src/remote/communication.dart
+++ b/drift/lib/src/remote/communication.dart
@@ -88,12 +88,9 @@ class DriftCommunication {
     } else if (msg is ErrorResponse) {
       final requestId = msg.requestId;
       final request = _pendingRequests.remove(requestId);
-      final backgroundTrace = msg.stackTrace != null
-          ? StackTrace.fromString(msg.stackTrace!)
-          : null;
 
       request?.completeWithError(
-          DriftRemoteException._(msg.error, backgroundTrace));
+          DriftRemoteException._(msg.error, msg.stackTrace));
       _pendingRequests.remove(msg.requestId);
     } else if (msg is Request) {
       _incomingRequests.add(msg);
@@ -145,14 +142,14 @@ class DriftCommunication {
   }
 
   /// Sends an erroneous response for a [Request].
-  void respondError(Request request, dynamic error, [StackTrace? trace]) {
+  void respondError(Request request, Object error, [StackTrace? trace]) {
     // sending a message while closed will throw, so don't even try.
     if (isClosed) return;
 
     if (error is CancellationException) {
       _send(CancelledResponse(request.id));
     } else {
-      _send(ErrorResponse(request.id, error.toString(), trace.toString()));
+      _send(ErrorResponse(request.id, error, trace));
     }
   }
 
diff --git a/drift/lib/src/remote/protocol.dart b/drift/lib/src/remote/protocol.dart
index d30fa43b..1c455ac2 100644
--- a/drift/lib/src/remote/protocol.dart
+++ b/drift/lib/src/remote/protocol.dart
@@ -38,7 +38,7 @@ class DriftProtocol {
         _tag_Response_error,
         message.requestId,
         message.error.toString(),
-        message.stackTrace,
+        message.stackTrace?.toString(),
       ];
     } else if (message is SuccessResponse) {
       return [
@@ -63,7 +63,10 @@ class DriftProtocol {
       case _tag_Request:
         return Request(id, decodePayload(message[2]));
       case _tag_Response_error:
-        return ErrorResponse(id, message[2] as Object, message[3] as String);
+        final stringTrace = message[3] as String?;
+
+        return ErrorResponse(id, message[2] as Object,
+            stringTrace != null ? StackTrace.fromString(stringTrace) : null);
       case _tag_Response_success:
         return SuccessResponse(id, decodePayload(message[2]));
       case _tag_Response_cancelled:
@@ -303,7 +306,7 @@ class SuccessResponse extends Message {
 class ErrorResponse extends Message {
   final int requestId;
   final Object error;
-  final String? stackTrace;
+  final StackTrace? stackTrace;
 
   ErrorResponse(this.requestId, this.error, [this.stackTrace]);
 
diff --git a/drift/test/isolate_test.dart b/drift/test/isolate_test.dart
index 454749f7..1bea9145 100644
--- a/drift/test/isolate_test.dart
+++ b/drift/test/isolate_test.dart
@@ -409,6 +409,44 @@ void _runTests(FutureOr<DriftIsolate> Function() spawner, bool terminateIsolate,
         emitsInOrder([anything]));
     database.markTablesUpdated({database.users});
   });
+
+  test('can see parameters in exception', () async {
+    final duplicateCategory =
+        CategoriesCompanion.insert(description: 'has unique constraint');
+    await database.categories.insertOne(duplicateCategory);
+
+    if (serialize) {
+      // We can't serialize exceptions, so expect a string error
+      await expectLater(
+        () => database.categories.insertOne(duplicateCategory),
+        throwsA(
+          isA<DriftRemoteException>().having(
+            (e) => e.remoteCause,
+            'remoteCause',
+            allOf(
+              contains('SqliteException'),
+              contains('parameters: has unique constraint'),
+            ),
+          ),
+        ),
+      );
+    } else {
+      await expectLater(
+        () => database.categories.insertOne(duplicateCategory),
+        throwsA(
+          isA<DriftRemoteException>().having(
+            (e) => e.remoteCause,
+            'remoteCause',
+            isA<SqliteException>()
+                .having((e) => e.causingStatement, 'causingStatement',
+                    'INSERT INTO "categories" ("desc") VALUES (?)')
+                .having((e) => e.parametersToStatement, 'parametersToStatement',
+                    ['has unique constraint']),
+          ),
+        ),
+      );
+    }
+  });
 }
 
 DatabaseConnection _backgroundConnection() {
diff --git a/drift/test/remote_test.dart b/drift/test/remote_test.dart
index 41993d61..b65f5be4 100644
--- a/drift/test/remote_test.dart
+++ b/drift/test/remote_test.dart
@@ -140,6 +140,15 @@ void main() {
       Uint8List(12),
     ]));
 
+    when(executor.runInsert(any, any)).thenAnswer(
+        (realInvocation) => Future.error(UnimplementedError('error!')));
+    await expectLater(
+      db.categories
+          .insertOne(CategoriesCompanion.insert(description: 'description')),
+      throwsA(isA<DriftRemoteException>().having(
+          (e) => e.remoteCause, 'remoteCause', 'UnimplementedError: error!')),
+    );
+
     await db.close();
   });