Skip to content

Commit

Permalink
Propagate user-level exceptions during client reset (#1581)
Browse files Browse the repository at this point in the history
  • Loading branch information
clementetb authored Jan 15, 2024
1 parent fd54a30 commit fcefa8a
Show file tree
Hide file tree
Showing 14 changed files with 112 additions and 89 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

### Enhancements
* [Sync] Added option to use managed WebSockets via OkHttp instead of Realm's built-in WebSocket client for Sync traffic (Only Android and JVM targets for now). Managed WebSockets offer improved support for proxies and firewalls that require authentication. This feature is currently opt-in and can be enabled by using `AppConfiguration.usePlatformNetworking()`. Managed WebSockets will become the default in a future version. (PR [#1528](https://github.com/realm/realm-kotlin/pull/1528)).
* `AutoClientResetFailed` exception now reports as the throwable cause any user exceptions that might occur during a client reset. (Issue [#1580](https://github.com/realm/realm-kotlin/issues/1580))

### Fixed
* Cache notification callback JNI references at startup to ensure that symbols can be resolved in core callbacks. (Issue [#1577](https://github.com/realm/realm-kotlin/issues/1577))
Expand All @@ -28,6 +29,7 @@
### Internal
* Update to Ktor 2.3.4.
* Updated to CMake 3.27.7
* Updated to Realm Core 13.25.0, commit 71f94d75e25bfc8913fcd93ae8de550b57577a4a.


## 1.13.1-SNAPSHOT (YYYY-MM-DD)
Expand Down
6 changes: 3 additions & 3 deletions dependencies.list
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
# Version of MongoDB Realm used by integration tests
# See https://github.com/realm/ci/packages/147854 for available versions
MONGODB_REALM_SERVER=2023-11-07
MONGODB_REALM_SERVER=2023-12-15

# `BAAS` and `BAAS-UI` projects commit hashes matching MONGODB_REALM_SERVER image version
# note that the MONGODB_REALM_SERVER image is a nightly build, find the matching commits
# for that date within the following repositories:
# https://github.com/10gen/baas/
# https://github.com/10gen/baas-ui/
REALM_BAAS_GIT_HASH=41fa6cdbca47826c20a64f756e21b2c184393e90
REALM_BAAS_UI_GIT_HASH=b97a27ac858e0e8126aeb63f6ff9734d11029a91
REALM_BAAS_GIT_HASH=47d9f6170ab1ac2aa64e7b5046e85247f3ac6d30
REALM_BAAS_UI_GIT_HASH=49157ef4a6af1c1de4dfbad5d7d02543776b25eb
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ expect enum class ErrorCode : CodeDescription {
RLM_ERR_TLS_HANDSHAKE_FAILED,
RLM_ERR_WRONG_SYNC_TYPE,
RLM_ERR_SYNC_WRITE_NOT_ALLOWED,
RLM_ERR_SYNC_LOCAL_CLOCK_BEFORE_EPOCH,
RLM_ERR_SYSTEM_ERROR,
RLM_ERR_LOGIC,
RLM_ERR_NOT_SUPPORTED,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,21 +29,23 @@ data class SyncError constructor(
val isFatal: Boolean,
val isUnrecognizedByClient: Boolean,
val isClientResetRequested: Boolean,
val compensatingWrites: Array<CoreCompensatingWriteInfo>
val compensatingWrites: Array<CoreCompensatingWriteInfo>,
val userError: Throwable?,
) {
// Constructs an SyncError out from a simple code. There are some situations (SyncSessionTransferCompletionCallback)
// where we receive an error code rather than a full SyncErrorCode, wrapping the code
// simplifies the error handling logic.
constructor(
error: CoreError
error: CoreError,
) : this(
errorCode = error,
originalFilePath = null,
recoveryFilePath = null,
isFatal = false,
isUnrecognizedByClient = false,
isClientResetRequested = false,
compensatingWrites = emptyArray()
compensatingWrites = emptyArray(),
userError = null,
)

// Constructor used by JNI so we avoid creating too many objects on the JNI side.
Expand All @@ -56,14 +58,16 @@ data class SyncError constructor(
isFatal: Boolean,
isUnrecognizedByClient: Boolean,
isClientResetRequested: Boolean,
compensatingWrites: Array<CoreCompensatingWriteInfo>
compensatingWrites: Array<CoreCompensatingWriteInfo>,
userError: Throwable?,
) : this(
CoreError(categoryFlags, value, message),
originalFilePath,
recoveryFilePath,
isFatal,
isUnrecognizedByClient,
isClientResetRequested,
compensatingWrites
compensatingWrites,
userError,
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ actual enum class ErrorCode(override val description: String, override val nativ
RLM_ERR_TLS_HANDSHAKE_FAILED("TLSHandshakeFailed", realm_errno_e.RLM_ERR_TLS_HANDSHAKE_FAILED),
RLM_ERR_WRONG_SYNC_TYPE("WrongSyncType", realm_errno_e.RLM_ERR_WRONG_SYNC_TYPE),
RLM_ERR_SYNC_WRITE_NOT_ALLOWED("SyncWriteNotAllowed", realm_errno_e.RLM_ERR_SYNC_WRITE_NOT_ALLOWED),
RLM_ERR_SYNC_LOCAL_CLOCK_BEFORE_EPOCH("SyncLocalClockBeforeEpoch", realm_errno_e.RLM_ERR_SYNC_LOCAL_CLOCK_BEFORE_EPOCH),
RLM_ERR_SYSTEM_ERROR("SystemError", realm_errno_e.RLM_ERR_SYSTEM_ERROR),
RLM_ERR_LOGIC("Logic", realm_errno_e.RLM_ERR_LOGIC),
RLM_ERR_NOT_SUPPORTED("NotSupported", realm_errno_e.RLM_ERR_NOT_SUPPORTED),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ actual enum class ErrorCode(
RLM_ERR_TLS_HANDSHAKE_FAILED("TLSHandshakeFailed", realm_errno.RLM_ERR_TLS_HANDSHAKE_FAILED),
RLM_ERR_WRONG_SYNC_TYPE("WrongSyncType", realm_errno.RLM_ERR_WRONG_SYNC_TYPE),
RLM_ERR_SYNC_WRITE_NOT_ALLOWED("SyncWriteNotAllowed", realm_errno.RLM_ERR_SYNC_WRITE_NOT_ALLOWED),
RLM_ERR_SYNC_LOCAL_CLOCK_BEFORE_EPOCH("SyncLocalClockBeforeEpoch", realm_errno.RLM_ERR_SYNC_LOCAL_CLOCK_BEFORE_EPOCH),
RLM_ERR_SYSTEM_ERROR("SystemError", realm_errno.RLM_ERR_SYSTEM_ERROR),
RLM_ERR_LOGIC("Logic", realm_errno.RLM_ERR_LOGIC),
RLM_ERR_NOT_SUPPORTED("NotSupported", realm_errno.RLM_ERR_NOT_SUPPORTED),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,9 @@ private fun throwOnError() {
errorCodeNativeValue = error.error.value.toInt(),
messageNativeValue = error.message?.toKString(),
path = error.path?.toKString(),
userError = error.usercode_error?.asStableRef<Throwable>()?.get()
userError = error.user_code_error?.asStableRef<Throwable>()?.get()
).also {
error.usercode_error?.let { disposeUserData<Throwable>(it) }
error.user_code_error?.let { disposeUserData<Throwable>(it) }
realm_clear_last_error()
}
}
Expand Down Expand Up @@ -638,9 +638,9 @@ actual object RealmInterop {
errorCodeNativeValue = err.error.value.toInt(),
messageNativeValue = err.message?.toKString(),
path = err.path?.toKString(),
userError = err.usercode_error?.asStableRef<Throwable>()?.get()
userError = err.user_code_error?.asStableRef<Throwable>()?.get()
)
err.usercode_error?.let { disposeUserData<Throwable>(it) }
err.user_code_error?.let { disposeUserData<Throwable>(it) }
} else {
realm_release(realm)
}
Expand Down Expand Up @@ -2534,8 +2534,11 @@ actual object RealmInterop {
isFatal = is_fatal,
isUnrecognizedByClient = is_unrecognized_by_client,
isClientResetRequested = is_client_reset_requested,
compensatingWrites = compensatingWrites
)
compensatingWrites = compensatingWrites,
userError = user_code_error?.asStableRef<Throwable>()?.get()
).also {
user_code_error?.let { disposeUserData<Throwable>(it) }
}
}
val errorCallback = safeUserData<SyncErrorCallback>(userData)
val session = CPointerWrapper<RealmSyncSessionT>(realm_clone(syncSession))
Expand Down Expand Up @@ -2565,16 +2568,10 @@ actual object RealmInterop {
realm_wrapper.realm_sync_config_set_before_client_reset_handler(
syncConfig.cptr(),
staticCFunction { userData, beforeRealm ->
val beforeCallback = safeUserData<SyncBeforeClientResetHandler>(userData)
val beforeDb = CPointerWrapper<FrozenRealmT>(beforeRealm, false)

// Check if exceptions have been thrown, return true if all went as it should
try {
beforeCallback.onBeforeReset(beforeDb)
stableUserDataWithErrorPropagation<SyncBeforeClientResetHandler>(userData) {
val beforeDb = CPointerWrapper<FrozenRealmT>(beforeRealm, false)
onBeforeReset(beforeDb)
true
} catch (e: Throwable) {
println(e.message)
false
}
},
StableRef.create(beforeHandler).asCPointer(),
Expand All @@ -2591,22 +2588,19 @@ actual object RealmInterop {
realm_wrapper.realm_sync_config_set_after_client_reset_handler(
syncConfig.cptr(),
staticCFunction { userData, beforeRealm, afterRealm, didRecover ->
val afterCallback = safeUserData<SyncAfterClientResetHandler>(userData)
val beforeDb = CPointerWrapper<FrozenRealmT>(beforeRealm, false)
stableUserDataWithErrorPropagation<SyncAfterClientResetHandler>(userData) {
val beforeDb = CPointerWrapper<FrozenRealmT>(beforeRealm, false)

// afterRealm is wrapped inside a ThreadSafeReference so the pointer needs to be resolved
val afterRealmPtr = realm_wrapper.realm_from_thread_safe_reference(afterRealm, null)
val afterDb = CPointerWrapper<LiveRealmT>(afterRealmPtr, false)
// afterRealm is wrapped inside a ThreadSafeReference so the pointer needs to be resolved
val afterRealmPtr = realm_wrapper.realm_from_thread_safe_reference(afterRealm, null)
val afterDb = CPointerWrapper<LiveRealmT>(afterRealmPtr, false)

// Check if exceptions have been thrown, return true if all went as it should
try {
afterCallback.onAfterReset(beforeDb, afterDb, didRecover)
true
} catch (e: Throwable) {
println(e.message)
false
} finally {
realm_wrapper.realm_close(afterRealmPtr)
try {
onAfterReset(beforeDb, afterDb, didRecover)
true
} finally {
realm_wrapper.realm_close(afterRealmPtr)
}
}
},
StableRef.create(afterHandler).asCPointer(),
Expand Down
2 changes: 1 addition & 1 deletion packages/external/core
Submodule core updated 74 files
+27 −0 .github/workflows/bindgen-unit-tests.yml
+36 −4 CHANGELOG.md
+1 −1 Package.swift
+2 −2 bindgen/.mocharc.json
+1 −1 bindgen/spec.yml
+1 −1 bindgen/src/bound-model.ts
+2 −2 dependencies.list
+101 −49 evergreen/config.yml
+22 −14 evergreen/install_baas.sh
+17 −20 evergreen/setup_baas_host_local.sh
+126 −0 evergreen/wait_for_remote_baas.sh
+4,213 −0 package-lock.json
+1 −1 package.json
+18 −2 src/realm.h
+11 −4 src/realm/alloc_slab.cpp
+1 −0 src/realm/error_codes.cpp
+1 −0 src/realm/error_codes.h
+1 −0 src/realm/error_codes.hpp
+4 −1 src/realm/group.cpp
+5 −3 src/realm/group.hpp
+19 −2 src/realm/obj.cpp
+1 −0 src/realm/obj.hpp
+21 −0 src/realm/object-store/c_api/app.cpp
+4 −4 src/realm/object-store/c_api/config.cpp
+14 −14 src/realm/object-store/c_api/error.cpp
+3 −3 src/realm/object-store/c_api/error.hpp
+3 −2 src/realm/object-store/c_api/sync.cpp
+3 −3 src/realm/object-store/c_api/types.hpp
+3 −1 src/realm/object-store/impl/realm_coordinator.cpp
+22 −19 src/realm/object-store/sync/sync_manager.cpp
+36 −52 src/realm/object-store/sync/sync_session.cpp
+5 −7 src/realm/object-store/sync/sync_session.hpp
+3 −1 src/realm/sync/changeset_encoder.cpp
+13 −9 src/realm/sync/client.cpp
+3 −0 src/realm/sync/config.hpp
+6 −2 src/realm/sync/history.hpp
+2 −4 src/realm/sync/instruction_applier.cpp
+14 −52 src/realm/sync/instructions.hpp
+90 −63 src/realm/sync/noinst/client_history_impl.cpp
+20 −18 src/realm/sync/noinst/client_history_impl.hpp
+13 −8 src/realm/sync/noinst/client_impl_base.cpp
+2 −2 src/realm/sync/noinst/client_impl_base.hpp
+36 −113 src/realm/sync/noinst/client_reset.cpp
+3 −12 src/realm/sync/noinst/client_reset.hpp
+3 −5 src/realm/sync/noinst/client_reset_operation.cpp
+226 −102 src/realm/sync/noinst/client_reset_recovery.cpp
+8 −191 src/realm/sync/noinst/client_reset_recovery.hpp
+3 −3 src/realm/sync/noinst/migration_store.cpp
+4 −4 src/realm/sync/noinst/migration_store.hpp
+0 −1 src/realm/sync/noinst/server/server_history.cpp
+118 −99 src/realm/sync/subscriptions.cpp
+35 −34 src/realm/sync/subscriptions.hpp
+1 −1 src/realm/util/backtrace.cpp
+87 −10 src/realm/util/file.cpp
+9 −3 src/realm/util/file.hpp
+13 −70 src/realm/util/file_mapper.cpp
+1 −1 src/realm/util/future.hpp
+1 −2 test/object-store/benchmarks/client_reset.cpp
+81 −3 test/object-store/c_api/c_api.cpp
+56 −69 test/object-store/sync/client_reset.cpp
+286 −105 test/object-store/sync/flx_sync.cpp
+19 −1 test/object-store/sync/user.cpp
+13 −3 test/object-store/util/sync/baas_admin_api.cpp
+6 −29 test/object-store/util/sync/sync_test_utils.cpp
+1 −1 test/object-store/util/sync/sync_test_utils.hpp
+37 −0 test/object-store/util/test_utils.hpp
+365 −12 test/test_client_reset.cpp
+12 −5 test/test_file.cpp
+9 −6 test/test_links.cpp
+5 −4 test/test_sync.cpp
+16 −37 test/test_sync_subscriptions.cpp
+9 −2 test/test_unresolved_links.cpp
+4 −1 test/tsan.suppress
+3 −0 test/util/spawned_process.cpp
74 changes: 31 additions & 43 deletions packages/jni-swig-stub/src/main/jni/realm_api_helpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,20 +34,26 @@ jobject wrap_pointer(JNIEnv* jenv, jlong pointer, jboolean managed = false) {
managed);
}

inline jboolean jni_check_exception(JNIEnv *jenv = get_env(), bool registrable_callback_error = false) {
inline jboolean jni_check_exception(JNIEnv *jenv = get_env()) {
// FIXME https://github.com/realm/realm-kotlin/issues/665 This function is catching and swallowing
// the exception. This behavior could leave the SDK in an illegal state.
if (jenv->ExceptionCheck()) {
if (registrable_callback_error) {
// setting the user code error is only propagated on certain callbacks.
jthrowable exception = jenv->ExceptionOccurred();
jenv->ExceptionClear();
realm_register_user_code_callback_error(jenv->NewGlobalRef(exception));
} else {
// Print the exception stacktrace in stderr.
jenv->ExceptionDescribe();
jenv->ExceptionClear();
}
// Print the exception stacktrace in stderr.
jenv->ExceptionDescribe();
jenv->ExceptionClear();
return false;
}
return true;
}

inline jboolean jni_check_exception_for_callback(JNIEnv *jenv = get_env()) {
if (jenv->ExceptionCheck()) {
// setting the user code error is only propagated on certain callbacks.
jthrowable exception = jenv->ExceptionOccurred();
jenv->ExceptionClear();
// This global ref would be released once core has propagated the exception back
// see: create_java_exception and convert_to_jvm_sync_error
realm_register_user_code_callback_error(jenv->NewGlobalRef(exception));
return false;
}
return true;
Expand Down Expand Up @@ -79,10 +85,10 @@ inline jobject create_java_exception(JNIEnv *jenv, realm_error_t error) {
jint(error.error),
error_message,
error_path,
static_cast<jobject>(error.usercode_error)
static_cast<jobject>(error.user_code_error)
);
if (error.usercode_error) {
(jenv)->DeleteGlobalRef(static_cast<jobject>(error.usercode_error));
if (error.user_code_error) {
(jenv)->DeleteGlobalRef(static_cast<jobject>(error.user_code_error));
}
jni_check_exception(jenv);
return jenv->PopLocalFrame(exception);
Expand Down Expand Up @@ -111,15 +117,6 @@ inline std::string get_exception_message(JNIEnv *env) {
return env->GetStringUTFChars(message, NULL);
}

inline void system_out_println(JNIEnv *env, std::string message) {
jclass system_class = env->FindClass("java/lang/System");
jfieldID field_id = env->GetStaticFieldID(system_class, "out", "Ljava/io/PrintStream;");
jobject system_out = env->GetStaticObjectField(system_class, field_id);
jclass print_stream_class = env->FindClass("java/io/PrintStream");
jmethodID method_id = env->GetMethodID(print_stream_class, "println", "(Ljava/lang/String;)V");
env->CallVoidMethod(system_out, method_id, to_jstring(env, message));
}

void
realm_changed_callback(void* userdata) {
auto env = get_env(true);
Expand Down Expand Up @@ -161,7 +158,7 @@ bool migration_callback(void *userdata, realm_t *old_realm, realm_t *new_realm,
wrap_pointer(env, reinterpret_cast<jlong>(new_realm), false),
wrap_pointer(env, reinterpret_cast<jlong>(schema))
);
bool failed = jni_check_exception(env, true);
bool failed = jni_check_exception_for_callback(env);
env->PopLocalFrame(NULL);
return failed;
}
Expand Down Expand Up @@ -559,7 +556,7 @@ bool realm_should_compact_callback(void* userdata, uint64_t total_bytes, uint64_

jobject callback = static_cast<jobject>(userdata);
jboolean result = env->CallBooleanMethod(callback, java_should_compact_method, jlong(total_bytes), jlong(used_bytes));
return jni_check_exception(env, true) && result;
return jni_check_exception_for_callback(env) && result;
}

bool realm_data_initialization_callback(void* userdata, realm_t*) {
Expand All @@ -569,7 +566,7 @@ bool realm_data_initialization_callback(void* userdata, realm_t*) {

jobject callback = static_cast<jobject>(userdata);
env->CallVoidMethod(callback, java_data_init_method);
return jni_check_exception(env, true);
return jni_check_exception_for_callback(env);
}

static void send_request_via_jvm_transport(JNIEnv *jenv, jobject network_transport, const realm_http_request_t request, jobject j_response_callback) {
Expand Down Expand Up @@ -1001,7 +998,7 @@ jobject convert_to_jvm_sync_error(JNIEnv* jenv, const realm_sync_error_t& error)
static JavaMethod sync_error_constructor(jenv,
JavaClassGlobalDef::sync_error(),
"<init>",
"(IILjava/lang/String;Ljava/lang/String;Ljava/lang/String;ZZZ[Lio/realm/kotlin/internal/interop/sync/CoreCompensatingWriteInfo;)V");
"(IILjava/lang/String;Ljava/lang/String;Ljava/lang/String;ZZZ[Lio/realm/kotlin/internal/interop/sync/CoreCompensatingWriteInfo;Ljava/lang/Throwable;)V");

jint category = static_cast<jint>(error.status.categories);
jint value = static_cast<jint>(error.status.error);
Expand Down Expand Up @@ -1089,10 +1086,14 @@ jobject convert_to_jvm_sync_error(JNIEnv* jenv, const realm_sync_error_t& error)
is_fatal,
is_unrecognized_by_client,
is_client_reset_requested,
j_compensating_write_info_array
j_compensating_write_info_array,
static_cast<jobject>(error.user_code_error)
);

jni_check_exception(jenv);
if(error.user_code_error) {
jenv->DeleteGlobalRef(static_cast<jobject>(error.user_code_error));
}
return jenv->PopLocalFrame(result);
}

Expand Down Expand Up @@ -1192,14 +1193,7 @@ before_client_reset(void* userdata, realm_t* before_realm) {
env->PushLocalFrame(1);
jobject before_pointer = wrap_pointer(env, reinterpret_cast<jlong>(before_realm), false);
env->CallVoidMethod(static_cast<jobject>(userdata), java_before_callback_function, before_pointer);

bool result = true;
if (env->ExceptionCheck()) {
std::string exception_message = get_exception_message(env);
std::string message_template = "An error has occurred in the 'onBefore' callback: ";
system_out_println(env, message_template.append(exception_message));
result = false;
}
bool result = jni_check_exception_for_callback(env);
env->PopLocalFrame(NULL);
return result;
}
Expand All @@ -1222,13 +1216,7 @@ after_client_reset(void* userdata, realm_t* before_realm,
jobject after_pointer = wrap_pointer(env, reinterpret_cast<jlong>(after_realm_ptr), false);
env->CallVoidMethod(static_cast<jobject>(userdata), java_after_callback_function, before_pointer, after_pointer, did_recover);
realm_close(after_realm_ptr);
bool result = true;
if (env->ExceptionCheck()) {
std::string exception_message = get_exception_message(env);
std::string message_template = "An error has occurred in the 'onAfter' callback: ";
system_out_println(env, message_template.append(exception_message));
result = false;
}
bool result = jni_check_exception_for_callback(env);
env->PopLocalFrame(NULL);
return result;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,11 @@ import io.realm.kotlin.mongodb.internal.createMessageFromSyncError
*/
public class ClientResetRequiredException constructor(
private val appPointer: RealmAppPointer,
error: SyncError
) : Throwable(message = createMessageFromSyncError(error.errorCode)) {
error: SyncError,
) : Throwable(
message = createMessageFromSyncError(error.errorCode),
cause = error.userError,
) {

/**
* Path to the original (local) copy of the realm when the Client Reset event was triggered.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,11 @@ interface AppAdmin {
*/
suspend fun countDocuments(clazz: String): Int

/**
* Delete documents of a given type.
*/
suspend fun deleteDocuments(database: String, clazz: String, query: String): JsonObject?

fun closeClient()
}

Expand Down Expand Up @@ -191,6 +196,11 @@ class AppAdminImpl(
app.countDocuments(clazz)
}

override suspend fun deleteDocuments(database: String, clazz: String, query: String): JsonObject? =
baasClient.run {
app.deleteDocument(database, clazz, query)
}

override fun closeClient() {
baasClient.closeClient()
}
Expand Down
Loading

0 comments on commit fcefa8a

Please sign in to comment.