Handle user errors in client reset callbacks

CHANGELOG.md

@@ -9,6 +9,7 @@
 ### Fixed
 * Fixed warnings being emitted by the realm generator requesting that `xyz.g.dart` be included with `part 'xyz.g.dart';` for `xyz.dart` files that import `realm` but don't have realm models defined. Those should not need generated parts and including the part file would have resulted in an empty file with `// ignore_for_file: type=lint` being generated. (PR [#1443](https://github.com/realm/realm-dart/pull/1443))
 * Updated the minimum required CMake version for Flutter on Linux to 3.19. (Issue [#1381](https://github.com/realm/realm-dart/issues/1381))
+* Errors in user-provided client reset callbacks, such as `RecoverOrDiscardUnsyncedChangesHandler.onBeforeReset/onAfterDiscard` would not be correctly propagated and the client reset exception would contain a message like `A fatal error occurred during client reset: 'User-provided callback failed'` but no details about the actual error. Now `SyncError` has an `innerError` field which contains the original error thrown in the callback. (PR [#1447](https://github.com/realm/realm-dart/pull/1447))
 
 ### Compatibility
 * Realm Studio: 13.0.0 or later.

lib/src/configuration.dart

@@ -636,14 +636,15 @@ class ClientResetError extends SyncError {
   ClientResetError._(
     String message,
     SyncErrorCode code,
-    this._app, {
+    this._app,
+    Object? innerError, {
     this.backupFilePath,
     this.originalFilePath,
-  }) : super._(message, code);
+  }) : super._(message, code, innerError);
 
   @override
   String toString() {
-    return "ClientResetError message: $message";
+    return "ClientResetError message: $message${innerError == null ? '' : ", inner error: '$innerError'"}";
   }
 
   /// Initiates the client reset process.
@@ -670,7 +671,7 @@ class SyncError extends RealmError {
   /// The code that describes this error.
   final SyncErrorCode code;
 
-  SyncError._(String message, this.code) : super(message);
+  SyncError._(String message, this.code, this.innerError) : super(message);
 
   /// The numeric code value indicating the type of the sync error.
   @Deprecated("Errors of SyncError subclasses will be created base on the error code. Error codes won't be returned anymore.")
@@ -688,14 +689,17 @@ class SyncError extends RealmError {
   @Deprecated("SyncError constructor is deprecated and will be removed in the future")
   SyncError(String message, this.category, int codeValue, {this.detailedMessage})
       : code = SyncErrorCode.fromInt(codeValue),
+        innerError = null,
         super(message);
 
   /// Creates a specific type of [SyncError] instance based on the [category] and the [code] supplied.
   @Deprecated("This method is deprecated and will be removed in the future")
   static SyncError create(String message, SyncErrorCategory category, int code, {bool isFatal = false}) {
-    return SyncError._(message, SyncErrorCode.fromInt(code));
+    return SyncError._(message, SyncErrorCode.fromInt(code), null);
   }
 
+  final Object? innerError;
+
   @override
   String toString() {
     return "Sync Error: $message";
@@ -731,9 +735,10 @@ final class CompensatingWriteError extends SyncError {
   late final List<CompensatingWriteInfo>? compensatingWrites;
 
   CompensatingWriteError._(
-    String message, {
+    String message,
+    Object? innerError, {
     this.compensatingWrites,
-  }) : super._(message, SyncErrorCode.compensatingWrite);
+  }) : super._(message, SyncErrorCode.compensatingWrite, innerError);
 
   @override
   String toString() {
@@ -752,16 +757,18 @@ extension SyncErrorInternal on SyncError {
           error.message,
           errorCode,
           app,
+          error.userError,
           originalFilePath: error.originalFilePath,
           backupFilePath: error.backupFilePath,
         ),
       SyncErrorCode.clientReset =>
-        ClientResetError._(error.message, errorCode, app, originalFilePath: error.originalFilePath, backupFilePath: error.backupFilePath),
+        ClientResetError._(error.message, errorCode, app, error.userError, originalFilePath: error.originalFilePath, backupFilePath: error.backupFilePath),
       SyncErrorCode.compensatingWrite => CompensatingWriteError._(
           error.message,
+          error.userError,
           compensatingWrites: error.compensatingWrites,
         ),
-      _ => SyncError._(error.message, errorCode),
+      _ => SyncError._(error.message, errorCode, error.userError),
     };
   }
 }

lib/src/native/realm_bindings.dart

@@ -3587,22 +3587,22 @@ class RealmLibrary {
           .asFunction<void Function(ffi.Pointer<ffi.Void>)>();
 
   void realm_dart_invoke_unlock_callback(
-    bool success,
+    ffi.Pointer<ffi.Void> error,
     ffi.Pointer<ffi.Void> unlockFunc,
   ) {
     return _realm_dart_invoke_unlock_callback(
-      success,
+      error,
       unlockFunc,
     );
   }
 
   late final _realm_dart_invoke_unlock_callbackPtr = _lookup<
-          ffi
-          .NativeFunction<ffi.Void Function(ffi.Bool, ffi.Pointer<ffi.Void>)>>(
-      'realm_dart_invoke_unlock_callback');
+      ffi.NativeFunction<
+          ffi.Void Function(ffi.Pointer<ffi.Void>,
+              ffi.Pointer<ffi.Void>)>>('realm_dart_invoke_unlock_callback');
   late final _realm_dart_invoke_unlock_callback =
-      _realm_dart_invoke_unlock_callbackPtr
-          .asFunction<void Function(bool, ffi.Pointer<ffi.Void>)>();
+      _realm_dart_invoke_unlock_callbackPtr.asFunction<
+          void Function(ffi.Pointer<ffi.Void>, ffi.Pointer<ffi.Void>)>();
 
   ffi.Pointer<ffi.Char> realm_dart_library_version() {
     return _realm_dart_library_version();
@@ -11126,8 +11126,8 @@ class _SymbolAddresses {
       get realm_dart_initializeDartApiDL =>
           _library._realm_dart_initializeDartApiDLPtr;
   ffi.Pointer<
-          ffi
-          .NativeFunction<ffi.Void Function(ffi.Bool, ffi.Pointer<ffi.Void>)>>
+          ffi.NativeFunction<
+              ffi.Void Function(ffi.Pointer<ffi.Void>, ffi.Pointer<ffi.Void>)>>
       get realm_dart_invoke_unlock_callback =>
           _library._realm_dart_invoke_unlock_callbackPtr;
   ffi.Pointer<ffi.NativeFunction<ffi.Pointer<ffi.Char> Function()>>

lib/src/native/realm_core.dart

@@ -587,14 +587,13 @@ class _RealmCore {
   }
 
   static void _guardSynchronousCallback(FutureOr<void> Function() callback, Pointer<Void> unlockCallbackFunc) async {
-    bool success = true;
+    Pointer<Void> user_error = nullptr;
     try {
       await callback();
     } catch (error) {
-      success = false;
-      _realmLib.realm_register_user_code_callback_error(error.toPersistentHandle());
+      user_error = error.toPersistentHandle();
     } finally {
-      _realmLib.realm_dart_invoke_unlock_callback(success, unlockCallbackFunc);
+      _realmLib.realm_dart_invoke_unlock_callback(user_error, unlockCallbackFunc);
     }
   }
 
@@ -3189,6 +3188,16 @@ extension on Pointer<Void> {
 
     return object;
   }
+
+  Object? toUserCodeError() {
+    if (this != nullptr) {
+      final result = toObject(isPersistent: true);
+      _realmLib.realm_dart_delete_persistent_handle(this);
+      return result;
+    }
+
+    return null;
+  }
 }
 
 extension on Pointer<Utf8> {
@@ -3222,6 +3231,7 @@ extension on realm_sync_error {
     return SyncErrorDetails(
       message,
       status.error,
+      user_code_error.toUserCodeError(),
       isFatal: is_fatal,
       isClientResetRequested: is_client_reset_requested,
       originalFilePath: userInfoMap?[originalFilePathKey],
@@ -3267,7 +3277,7 @@ extension on Pointer<realm_sync_error_compensating_write_info> {
 extension on Pointer<realm_error_t> {
   SyncError toSyncError() {
     final message = ref.message.cast<Utf8>().toDartString();
-    final details = SyncErrorDetails(message, ref.error);
+    final details = SyncErrorDetails(message, ref.error, ref.user_code_error.toUserCodeError());
     return SyncErrorInternal.createSyncError(details);
   }
 }
@@ -3432,9 +3442,12 @@ class SyncErrorDetails {
   final String? originalFilePath;
   final String? backupFilePath;
   final List<CompensatingWriteInfo>? compensatingWrites;
+  final Object? userError;
+
   SyncErrorDetails(
     this.message,
-    this.code, {
+    this.code,
+    this.userError, {
     this.path,
     this.isFatal = false,
     this.isClientResetRequested = false,
@@ -3447,12 +3460,6 @@ class SyncErrorDetails {
 extension on realm_error {
   LastError toLastError() {
     final message = this.message.cast<Utf8>().toRealmDartString();
-    Object? userError;
-    if (error == realm_errno.RLM_ERR_CALLBACK && user_code_error != nullptr) {
-      userError = user_code_error.toObject(isPersistent: true);
-      _realmLib.realm_dart_delete_persistent_handle(user_code_error);
-    }
-
-    return LastError(error, message, userError);
+    return LastError(error, message, user_code_error.toUserCodeError());
   }
 }

src/realm_dart.cpp

@@ -110,18 +110,18 @@ RLM_API void realm_dart_userdata_async_free(void* userdata) {
     });
 }
 
-RLM_API void realm_dart_invoke_unlock_callback(bool success, void* unlockFunc) {
-    auto castFunc = (reinterpret_cast<realm::util::UniqueFunction<void(bool)>*>(unlockFunc));
-    (*castFunc)(success);
+RLM_API void realm_dart_invoke_unlock_callback(realm_userdata_t error, void* unlockFunc) {
+    auto castFunc = (reinterpret_cast<realm::util::UniqueFunction<void(realm_userdata_t)>*>(unlockFunc));
+    (*castFunc)(error);
 }
 
 // Stamped into the library by the build system (see prepare-release.yml)
-// Keep this method as it is written and do not format it. 
+// Keep this method as it is written and do not format it.
 // We have a github workflow that looks for and replaces this string as it is written here.
 RLM_API const char* realm_dart_library_version() { return "1.6.1"; }
 
 //for debugging only
-// RLM_API void realm_dart_gc() { 
+// RLM_API void realm_dart_gc() {
 //     Dart_ExecuteInternalCommand_DL("gc-now", nullptr);
 // }
 

src/realm_dart.h

@@ -46,7 +46,7 @@ typedef struct realm_dart_userdata_async* realm_dart_userdata_async_t;
 RLM_API realm_dart_userdata_async_t realm_dart_userdata_async_new(Dart_Handle handle, void* callback, realm_scheduler_t* scheduler);
 RLM_API void realm_dart_userdata_async_free(void* userdata);
 
-RLM_API void realm_dart_invoke_unlock_callback(bool success, void* unlockFunc);
+RLM_API void realm_dart_invoke_unlock_callback(realm_userdata_t error, void* unlockFunc);
 
 RLM_API const char* realm_dart_library_version();
 

src/realm_dart_sync.cpp

@@ -181,16 +181,16 @@ RLM_API void realm_dart_sync_on_subscription_state_changed_callback(realm_userda
     });
 }
 
-bool invoke_dart_and_await_result(realm::util::UniqueFunction<void(realm::util::UniqueFunction<void(bool)>*)>* userCallback)
+bool invoke_dart_and_await_result(realm::util::UniqueFunction<void(realm::util::UniqueFunction<void(realm_userdata_t)>*)>* userCallback)
 {
     std::condition_variable condition;
     std::mutex mutex;
-    bool success = false;
+    realm_userdata_t user_error = nullptr;
     bool completed = false;
 
-    realm::util::UniqueFunction unlockFunc = [&](bool result) {
+    realm::util::UniqueFunction unlockFunc = [&](realm_userdata_t error) {
         std::unique_lock lock(mutex);
-        success = result;
+        user_error = error;
         completed = true;
         condition.notify_one();
     };
@@ -199,13 +199,18 @@ bool invoke_dart_and_await_result(realm::util::UniqueFunction<void(realm::util::
     (*userCallback)(&unlockFunc);
     condition.wait(lock, [&]() { return completed; });
 
-    return success;
+    if (user_error != nullptr) {
+        realm_register_user_code_callback_error(user_error);
+        return false;
+    }
+
+    return true;
 }
 
 RLM_API bool realm_dart_sync_before_reset_handler_callback(realm_userdata_t userdata, realm_t* realm)
 {
     auto ud = reinterpret_cast<realm_dart_userdata_async_t>(userdata);
-    realm::util::UniqueFunction userCallback = [ud, realm](realm::util::UniqueFunction<void(bool)>* unlockFunc) {
+    realm::util::UniqueFunction userCallback = [ud, realm](realm::util::UniqueFunction<void(realm_userdata_t)>* unlockFunc) {
         ud->scheduler->invoke([ud, realm, unlockFunc]() {
             (reinterpret_cast<realm_sync_before_client_reset_begin_func_t>(ud->dart_callback))(ud->handle, realm, unlockFunc);
         });
@@ -216,7 +221,7 @@ RLM_API bool realm_dart_sync_before_reset_handler_callback(realm_userdata_t user
 RLM_API bool realm_dart_sync_after_reset_handler_callback(realm_userdata_t userdata, realm_t* before_realm, realm_thread_safe_reference_t* after_realm, bool did_recover)
 {
     auto ud = reinterpret_cast<realm_dart_userdata_async_t>(userdata);
-    realm::util::UniqueFunction userCallback = [ud, before_realm, after_realm, did_recover](realm::util::UniqueFunction<void(bool)>* unlockFunc) {
+    realm::util::UniqueFunction userCallback = [ud, before_realm, after_realm, did_recover](realm::util::UniqueFunction<void(realm_userdata_t)>* unlockFunc) {
         ud->scheduler->invoke([ud, before_realm, after_realm, did_recover, unlockFunc]() {
             (reinterpret_cast<realm_sync_after_client_reset_begin_func_t>(ud->dart_callback))(ud->handle, before_realm, after_realm, did_recover, unlockFunc);
         });

test/client_reset_test.dart

@@ -165,7 +165,10 @@ Future<void> main([List<String>? args]) async {
       await triggerClientReset(realm);
 
       final clientResetFuture = onManualResetFallback.future.wait(defaultWaitTimeout, "onManualResetFallback is not reported.");
-      await expectLater(clientResetFuture, throws<ClientResetError>());
+      await expectLater(
+          clientResetFuture,
+          throwsA(isA<ClientResetError>().having((e) => e.innerError?.toString(), 'innerError', 'Exception: This fails!').having((e) => e.toString(), 'message',
+              "ClientResetError message: A fatal error occurred during client reset: 'User-provided callback failed', inner error: 'Exception: This fails!'")));
     });
 
     baasTest('$clientResetHandlerType.onManualResetFallback invoked when throw in onAfterReset', (appConfig) async {
@@ -174,7 +177,7 @@ Future<void> main([List<String>? args]) async {
 
       final onManualResetFallback = Completer<void>();
       void onAfterReset(Realm beforeResetRealm, Realm afterResetRealm) {
-        throw Exception("This fails!");
+        throw Exception("This fails too!");
       }
 
       final config = Configuration.flexibleSync(user, getSyncSchema(),
@@ -191,7 +194,12 @@ Future<void> main([List<String>? args]) async {
       await triggerClientReset(realm);
 
       final clientResetFuture = onManualResetFallback.future.wait(defaultWaitTimeout, "onManualResetFallback is not reported.");
-      await expectLater(clientResetFuture, throws<ClientResetError>());
+      await expectLater(
+          clientResetFuture,
+          throwsA(isA<ClientResetError>().having((e) => e.innerError?.toString(), 'innerError', 'Exception: This fails too!').having(
+              (e) => e.toString(),
+              'message',
+              "ClientResetError message: A fatal error occurred during client reset: 'User-provided callback failed', inner error: 'Exception: This fails too!'")));
     });
 
     baasTest('$clientResetHandlerType.onBeforeReset and onAfterReset are invoked', (appConfig) async {
@@ -466,6 +474,8 @@ Future<void> main([List<String>? args]) async {
     expect(onBeforeResetOccurred, 1);
 
     expect(clientResetErrorOnManualFallback.message, isNotEmpty);
+    expect(clientResetErrorOnManualFallback.innerError, isNotNull);
+    expect(clientResetErrorOnManualFallback.innerError.toString(), 'Exception: Cause onManualResetFallback');
   });
 
   // 1. userA adds [task0, task1, task2] and syncs it, then disconnects