forward exceptions from user-provided property converters (#298)

Author: vaind

objectbox/lib/src/native/bindings/data_visitor.dart

@@ -34,9 +34,25 @@ Pointer<NativeFunction<obx_data_visitor>> dataVisitor(
 
 @pragma('vm:prefer-inline')
 Pointer<NativeFunction<obx_data_visitor>> objectCollector<T>(
-        List<T> list, Store store, EntityDefinition<T> entity) =>
+        List<T> list,
+        Store store,
+        EntityDefinition<T> entity,
+        ObjectCollectorError outError) =>
     dataVisitor((Pointer<Uint8> data, int size) {
-      list.add(entity.objectFromFB(
-          store, InternalStoreAccess.reader(store).access(data, size)));
-      return true;
+      try {
+        list.add(entity.objectFromFB(
+            store, InternalStoreAccess.reader(store).access(data, size)));
+        return true;
+      } catch (e) {
+        outError.error = e;
+        return false;
+      }
     });
+
+class ObjectCollectorError {
+  Object? error;
+
+  void throwIfError() {
+    if (error != null) throw error!;
+  }
+}

objectbox/lib/src/native/query/query.dart

@@ -738,12 +738,18 @@ class Query<T> {
   /// results. Note: [offset] and [limit] are respected, if set.
   T? findFirst() {
     T? result;
+    Object? error;
     final visitor = dataVisitor((Pointer<Uint8> data, int size) {
-      result = _entity.objectFromFB(
-          _store, InternalStoreAccess.reader(_store).access(data, size));
+      try {
+        result = _entity.objectFromFB(
+            _store, InternalStoreAccess.reader(_store).access(data, size));
+      } catch (e) {
+        error = e;
+      }
       return false; // we only want to visit the first element
     });
     checkObx(C.query_visit(_ptr, visitor, nullptr));
+    if (error != null) throw error!;
     reachabilityFence(this);
     return result;
   }
@@ -756,17 +762,22 @@ class Query<T> {
   /// higher than one, otherwise the check for non-unique result won't work.
   T? findUnique() {
     T? result;
-    Exception? error;
+    Object? error;
     final visitor = dataVisitor((Pointer<Uint8> data, int size) {
       if (result == null) {
-        result = _entity.objectFromFB(
-            _store, InternalStoreAccess.reader(_store).access(data, size));
+        try {
+          result = _entity.objectFromFB(
+              _store, InternalStoreAccess.reader(_store).access(data, size));
+          return true;
+        } catch (e) {
+          error = e;
+          return false;
+        }
       } else {
         error = UniqueViolationException(
             'Query findUnique() matched more than one object');
         return false;
       }
-      return true;
     });
     checkObx(C.query_visit(_ptr, visitor, nullptr));
     reachabilityFence(this);
@@ -790,8 +801,10 @@ class Query<T> {
   /// Finds Objects matching the query.
   List<T> find() {
     final result = <T>[];
-    final collector = objectCollector(result, _store, _entity);
+    final errorWrapper = ObjectCollectorError();
+    final collector = objectCollector(result, _store, _entity, errorWrapper);
     checkObx(C.query_visit(_ptr, collector, nullptr));
+    errorWrapper.throwIfError();
     reachabilityFence(this);
     return result;
   }

objectbox/test/box_test.dart

@@ -1,8 +1,9 @@
 import 'dart:io';
 import 'dart:typed_data';
 
-import 'package:test/test.dart';
 import 'package:objectbox/objectbox.dart';
+import 'package:test/test.dart';
+
 import 'entity.dart';
 import 'entity2.dart';
 import 'test_env.dart';
@@ -631,4 +632,27 @@ void main() {
             box.get(1);
             box2.get(1);
           }));
+
+  test('throwing in converters', () {
+    late Box<ThrowingInConverters> box = store.box();
+
+    box.put(ThrowingInConverters());
+    box.put(ThrowingInConverters(throwOnGet: true));
+    expect(() => box.put(ThrowingInConverters(throwOnPut: true)),
+        ThrowingInConverters.throwsIn('Getter'));
+
+    expect(
+        () => box.putMany([
+              ThrowingInConverters(),
+              ThrowingInConverters(),
+              ThrowingInConverters(throwOnPut: true)
+            ]),
+        ThrowingInConverters.throwsIn('Getter'));
+
+    expect(box.count(), 2);
+
+    box.get(1);
+    expect(() => box.get(2), ThrowingInConverters.throwsIn('Setter'));
+    expect(() => box.getAll(), ThrowingInConverters.throwsIn('Setter'));
+  });
 }

objectbox/test/entity2.dart

@@ -1,4 +1,5 @@
 import 'package:objectbox/objectbox.dart';
+import 'package:test/test.dart';
 
 // Testing a model for entities in multiple files is generated properly
 @Entity()
@@ -35,3 +36,24 @@ class TreeNode {
 
   TreeNode(this.path);
 }
+
+/// Test how DB operations behave if property converters throw.
+@Entity()
+class ThrowingInConverters {
+  int id = 0;
+
+  final bool throwOnGet;
+  final bool throwOnPut;
+
+  ThrowingInConverters({this.throwOnGet = false, this.throwOnPut = false});
+
+  int get value =>
+      throwOnPut ? throw Exception('Getter invoked, e.g. box.put()') : 1;
+
+  set value(int val) {
+    if (throwOnGet) throw Exception('Setter invoked, e.g. box.get())');
+  }
+
+  static Matcher throwsIn(String op) =>
+      throwsA(predicate((Exception e) => e.toString().contains('$op invoked')));
+}

objectbox/test/objectbox-model.json

@@ -491,9 +491,38 @@
         }
       ],
       "relations": []
+    },
+    {
+      "id": "10:8814538095619551454",
+      "lastPropertyId": "4:1497692645133575154",
+      "name": "ThrowingInConverters",
+      "properties": [
+        {
+          "id": "1:4741681947876978320",
+          "name": "id",
+          "type": 6,
+          "flags": 1
+        },
+        {
+          "id": "2:4138824249670147679",
+          "name": "throwOnGet",
+          "type": 1
+        },
+        {
+          "id": "3:7057161000152955585",
+          "name": "throwOnPut",
+          "type": 1
+        },
+        {
+          "id": "4:1497692645133575154",
+          "name": "value",
+          "type": 6
+        }
+      ],
+      "relations": []
     }
   ],
-  "lastEntityId": "9:5417142555323669520",
+  "lastEntityId": "10:8814538095619551454",
   "lastIndexId": "19:3009172190024929732",
   "lastRelationId": "1:2155747579134420981",
   "lastSequenceId": "0:0",

objectbox/test/query_test.dart

@@ -2,6 +2,7 @@ import 'package:collection/collection.dart';
 import 'package:test/test.dart';
 
 import 'entity.dart';
+import 'entity2.dart';
 import 'objectbox.g.dart';
 import 'test_env.dart';
 
@@ -869,4 +870,18 @@ void main() {
           '| Link RelatedEntityA via standalone Relation 1 (from entity 1 to 4) with conditions: tInt == 11',
         ].join('\n'));
   });
+
+  test('throwing in converters', () {
+    late Box<ThrowingInConverters> box = env.store.box();
+
+    box.put(ThrowingInConverters(throwOnGet: true));
+    box.put(ThrowingInConverters());
+
+    final query = box.query().build();
+    expect(query.count(), 2);
+    expect(query.findIds().length, 2);
+
+    expect(query.findFirst, ThrowingInConverters.throwsIn('Setter'));
+    expect(query.find, ThrowingInConverters.throwsIn('Setter'));
+  });
 }