diff --git a/CHANGELOG.md b/CHANGELOG.md index f2a5f9474..7ad4670c9 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/lib/src/configuration.dart b/lib/src/configuration.dart index c63242407..16659997e 100644 --- a/lib/src/configuration.dart +++ b/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? 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), }; } } diff --git a/lib/src/native/realm_bindings.dart b/lib/src/native/realm_bindings.dart index 08c276089..446ea5bd6 100644 --- a/lib/src/native/realm_bindings.dart +++ b/lib/src/native/realm_bindings.dart @@ -3587,22 +3587,22 @@ class RealmLibrary { .asFunction)>(); void realm_dart_invoke_unlock_callback( - bool success, + ffi.Pointer error, ffi.Pointer unlockFunc, ) { return _realm_dart_invoke_unlock_callback( - success, + error, unlockFunc, ); } late final _realm_dart_invoke_unlock_callbackPtr = _lookup< - ffi - .NativeFunction)>>( - 'realm_dart_invoke_unlock_callback'); + ffi.NativeFunction< + ffi.Void Function(ffi.Pointer, + ffi.Pointer)>>('realm_dart_invoke_unlock_callback'); late final _realm_dart_invoke_unlock_callback = - _realm_dart_invoke_unlock_callbackPtr - .asFunction)>(); + _realm_dart_invoke_unlock_callbackPtr.asFunction< + void Function(ffi.Pointer, ffi.Pointer)>(); ffi.Pointer 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.NativeFunction< + ffi.Void Function(ffi.Pointer, ffi.Pointer)>> get realm_dart_invoke_unlock_callback => _library._realm_dart_invoke_unlock_callbackPtr; ffi.Pointer Function()>> diff --git a/lib/src/native/realm_core.dart b/lib/src/native/realm_core.dart index 999153456..134d1db08 100644 --- a/lib/src/native/realm_core.dart +++ b/lib/src/native/realm_core.dart @@ -587,14 +587,13 @@ class _RealmCore { } static void _guardSynchronousCallback(FutureOr Function() callback, Pointer unlockCallbackFunc) async { - bool success = true; + Pointer 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 { 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 { @@ -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 { extension on Pointer { SyncError toSyncError() { final message = ref.message.cast().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? 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().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()); } } diff --git a/src/realm_dart.cpp b/src/realm_dart.cpp index 1de4e9b2f..199c6149b 100644 --- a/src/realm_dart.cpp +++ b/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*>(unlockFunc)); - (*castFunc)(success); +RLM_API void realm_dart_invoke_unlock_callback(realm_userdata_t error, void* unlockFunc) { + auto castFunc = (reinterpret_cast*>(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); // } diff --git a/src/realm_dart.h b/src/realm_dart.h index 1f43b2fa1..76e41f70c 100644 --- a/src/realm_dart.h +++ b/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(); diff --git a/src/realm_dart_sync.cpp b/src/realm_dart_sync.cpp index ba148f972..0249f55a8 100644 --- a/src/realm_dart_sync.cpp +++ b/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*)>* userCallback) +bool invoke_dart_and_await_result(realm::util::UniqueFunction*)>* 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(userdata); - realm::util::UniqueFunction userCallback = [ud, realm](realm::util::UniqueFunction* unlockFunc) { + realm::util::UniqueFunction userCallback = [ud, realm](realm::util::UniqueFunction* unlockFunc) { ud->scheduler->invoke([ud, realm, unlockFunc]() { (reinterpret_cast(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(userdata); - realm::util::UniqueFunction userCallback = [ud, before_realm, after_realm, did_recover](realm::util::UniqueFunction* unlockFunc) { + realm::util::UniqueFunction userCallback = [ud, before_realm, after_realm, did_recover](realm::util::UniqueFunction* unlockFunc) { ud->scheduler->invoke([ud, before_realm, after_realm, did_recover, unlockFunc]() { (reinterpret_cast(ud->dart_callback))(ud->handle, before_realm, after_realm, did_recover, unlockFunc); }); diff --git a/test/client_reset_test.dart b/test/client_reset_test.dart index fb18c624e..56391748f 100644 --- a/test/client_reset_test.dart +++ b/test/client_reset_test.dart @@ -165,7 +165,10 @@ Future main([List? args]) async { await triggerClientReset(realm); final clientResetFuture = onManualResetFallback.future.wait(defaultWaitTimeout, "onManualResetFallback is not reported."); - await expectLater(clientResetFuture, throws()); + await expectLater( + clientResetFuture, + throwsA(isA().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 main([List? args]) async { final onManualResetFallback = Completer(); 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 main([List? args]) async { await triggerClientReset(realm); final clientResetFuture = onManualResetFallback.future.wait(defaultWaitTimeout, "onManualResetFallback is not reported."); - await expectLater(clientResetFuture, throws()); + await expectLater( + clientResetFuture, + throwsA(isA().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 main([List? 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