Skip to content

Commit

Permalink
Propagate error code in case of client reset errors in the callback (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
nicola-cab authored Dec 7, 2023
1 parent c3ed40b commit 3975f43
Show file tree
Hide file tree
Showing 8 changed files with 112 additions and 31 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
([PR #7161](https://github.com/realm/realm-core/pull/7161), since v12.3.0).
* If the very first open of a flexible sync Realm triggered a client reset, the configuration had an initial subscriptions callback, both before and after reset callbacks, and the initial subscription callback began a read transaction without ending it (which is normally going to be the case), opening the frozen Realm for the after reset callback would trigger a BadVersion exception ([PR #7161](https://github.com/realm/realm-core/pull/7161), since v12.3.0).
* Changesets have wrong timestamps if the local clock lags behind 2015-01-01T00:00:00Z. The sync client now throws an exception if that happens. ([PR #7180](https://github.com/realm/realm-core/pull/7180))
* Allow propagation of user code exceptions happening during client reset callbacks, retrievable via `realm_sync_error_t` in `realm_sync_config_set_error_handler` in the C-API. ([#7098](https://github.com/realm/realm-core/issues/7098), since v11.16.0)

### Breaking changes
* Update existing std exceptions thrown by the Sync Client to use Realm exceptions. ([PR #7141](https://github.com/realm/realm-core/pull/7141/files))
Expand Down
5 changes: 3 additions & 2 deletions src/realm.h
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ typedef struct realm_error {
const char* message;
// When error is RLM_ERR_CALLBACK this is an opaque pointer to an SDK-owned error object
// thrown by user code inside a callback with realm_register_user_code_callback_error(), otherwise null.
void* usercode_error;
void* user_code_error;
const char* path;
} realm_error_t;

Expand Down Expand Up @@ -3408,6 +3408,7 @@ typedef struct realm_sync_error {

realm_sync_error_compensating_write_info_t* compensating_writes;
size_t compensating_writes_length;
void* user_code_error;
} realm_sync_error_t;

/**
Expand Down Expand Up @@ -3878,7 +3879,7 @@ RLM_API void realm_sync_session_handle_error_for_testing(const realm_sync_sessio
/**
* In case of exception thrown in user code callbacks, this api will allow the sdk to store the user code exception
* and retrieve a it later via realm_get_last_error.
* Most importantly the SDK is responsible to handle the memory pointed by usercode_error.
* Most importantly the SDK is responsible to handle the memory pointed by user_code_error.
* @param usercode_error pointer representing whatever object the SDK treats as exception/error.
*/
RLM_API void realm_register_user_code_callback_error(realm_userdata_t usercode_error) RLM_API_NOEXCEPT;
Expand Down
8 changes: 4 additions & 4 deletions src/realm/object-store/c_api/config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ RLM_API void realm_config_set_migration_function(realm_config_t* config, realm_m
realm_t r2{new_realm};
realm_schema_t sch{&schema};
if (!(func)(userdata, &r1, &r2, &sch)) {
throw CallbackFailed{ErrorStorage::get_thread_local()->get_and_clear_usercode_error()};
throw CallbackFailed{ErrorStorage::get_thread_local()->get_and_clear_user_code_error()};
}
};
config->migration_function = std::move(migration_func);
Expand All @@ -123,7 +123,7 @@ RLM_API void realm_config_set_data_initialization_function(realm_config_t* confi
auto init_func = [=](SharedRealm realm) {
realm_t r{realm};
if (!(func)(userdata, &r)) {
throw CallbackFailed{ErrorStorage::get_thread_local()->get_and_clear_usercode_error()};
throw CallbackFailed{ErrorStorage::get_thread_local()->get_and_clear_user_code_error()};
}
};
config->initialization_function = std::move(init_func);
Expand All @@ -144,8 +144,8 @@ RLM_API void realm_config_set_should_compact_on_launch_function(realm_config_t*
if (func) {
auto should_func = [=](uint64_t total_bytes, uint64_t used_bytes) -> bool {
auto result = func(userdata, total_bytes, used_bytes);
if (auto usercode_error = ErrorStorage::get_thread_local()->get_and_clear_usercode_error())
throw CallbackFailed{usercode_error};
if (auto user_code_error = ErrorStorage::get_thread_local()->get_and_clear_user_code_error())
throw CallbackFailed{user_code_error};
return result;
};
config->should_compact_on_launch_function = std::move(should_func);
Expand Down
28 changes: 14 additions & 14 deletions src/realm/object-store/c_api/error.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ namespace realm::c_api {
ErrorStorage::ErrorStorage(std::exception_ptr ptr) noexcept
: m_err(none)
, m_message_buf()
, m_usercode_error(nullptr)
, m_user_code_error(nullptr)
{
assign(std::move(ptr));
}

ErrorStorage::ErrorStorage(const ErrorStorage& other)
: m_err(other.m_err)
, m_message_buf(other.m_message_buf)
, m_usercode_error(other.m_usercode_error)
, m_user_code_error(other.m_user_code_error)
{
if (m_err) {
m_err->message = m_message_buf.c_str();
Expand All @@ -34,7 +34,7 @@ ErrorStorage& ErrorStorage::operator=(const ErrorStorage& other)
{
m_err = other.m_err;
m_message_buf = other.m_message_buf;
m_usercode_error = other.m_usercode_error;
m_user_code_error = other.m_user_code_error;
if (m_err) {
m_err->message = m_message_buf.c_str();
}
Expand All @@ -44,7 +44,7 @@ ErrorStorage& ErrorStorage::operator=(const ErrorStorage& other)
ErrorStorage::ErrorStorage(ErrorStorage&& other)
: m_err(std::move(other.m_err))
, m_message_buf(std::move(other.m_message_buf))
, m_usercode_error(std::move(other.m_usercode_error))
, m_user_code_error(std::move(other.m_user_code_error))
{
if (m_err) {
m_err->message = m_message_buf.c_str();
Expand All @@ -56,7 +56,7 @@ ErrorStorage& ErrorStorage::operator=(ErrorStorage&& other)
{
m_err = std::move(other.m_err);
m_message_buf = std::move(other.m_message_buf);
m_usercode_error = std::move(other.m_usercode_error);
m_user_code_error = std::move(other.m_user_code_error);
if (m_err) {
m_err->message = m_message_buf.c_str();
}
Expand All @@ -83,7 +83,7 @@ void ErrorStorage::assign(std::exception_ptr eptr) noexcept
}

m_err.emplace();
m_err->usercode_error = nullptr;
m_err->user_code_error = nullptr;
m_err->path = nullptr;
auto populate_error = [&](const std::exception& ex, ErrorCodes::Error error_code) {
m_err->error = realm_errno_e(error_code);
Expand All @@ -108,7 +108,7 @@ void ErrorStorage::assign(std::exception_ptr eptr) noexcept
catch (const Exception& ex) {
populate_error(ex, ex.code());
if (ex.code() == ErrorCodes::CallbackFailed) {
m_err->usercode_error = static_cast<const CallbackFailed&>(ex).usercode_error;
m_err->user_code_error = static_cast<const CallbackFailed&>(ex).user_code_error;
}
if (ErrorCodes::error_categories(ex.code()).test(ErrorCategory::file_access)) {
auto& file_access_error = static_cast<const FileAccessError&>(ex);
Expand Down Expand Up @@ -168,15 +168,15 @@ bool ErrorStorage::clear() noexcept
return ret;
}

void ErrorStorage::set_usercode_error(void* usercode_error)
void ErrorStorage::set_user_code_error(void* user_code_error)
{
m_usercode_error = usercode_error;
m_user_code_error = user_code_error;
}

void* ErrorStorage::get_and_clear_usercode_error()
void* ErrorStorage::get_and_clear_user_code_error()
{
auto ret = m_usercode_error;
m_usercode_error = nullptr;
auto ret = m_user_code_error;
m_user_code_error = nullptr;
return ret;
}

Expand Down Expand Up @@ -247,7 +247,7 @@ RLM_EXPORT bool realm_wrap_exceptions(void (*func)()) noexcept
});
}

RLM_API void realm_register_user_code_callback_error(void* usercode_error) noexcept
RLM_API void realm_register_user_code_callback_error(void* user_code_error) noexcept
{
realm::c_api::ErrorStorage::get_thread_local()->set_usercode_error(usercode_error);
realm::c_api::ErrorStorage::get_thread_local()->set_user_code_error(user_code_error);
}
6 changes: 3 additions & 3 deletions src/realm/object-store/c_api/error.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,14 @@ class ErrorStorage {
bool get_as_realm_error_t(realm_error_t* out) const noexcept;
bool clear() noexcept;

void set_usercode_error(void* usercode_error);
void* get_and_clear_usercode_error();
void set_user_code_error(void* user_code_error);
void* get_and_clear_user_code_error();

private:
util::Optional<realm_error_t> m_err;
std::string m_message_buf;
std::string m_path_buf;
void* m_usercode_error;
void* m_user_code_error;
};

} // namespace realm::c_api
5 changes: 3 additions & 2 deletions src/realm/object-store/c_api/sync.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ RLM_API void realm_sync_config_set_error_handler(realm_sync_config_t* config, re
c_error.server_requests_action = static_cast<realm_sync_error_action_e>(error.server_requests_action);
c_error.c_original_file_path_key = error.c_original_file_path_key;
c_error.c_recovery_file_path_key = error.c_recovery_file_path_key;
c_error.user_code_error = ErrorStorage::get_thread_local()->get_and_clear_user_code_error();

std::vector<realm_sync_error_user_info_t> c_user_info;
c_user_info.reserve(error.user_info.size());
Expand Down Expand Up @@ -388,7 +389,7 @@ RLM_API void realm_sync_config_set_before_client_reset_handler(realm_sync_config
auto cb = [callback, userdata = SharedUserdata(userdata, FreeUserdata(userdata_free))](SharedRealm before_realm) {
realm_t r1{before_realm};
if (!callback(userdata.get(), &r1)) {
throw CallbackFailed();
throw CallbackFailed{};
}
};
config->notify_before_client_reset = std::move(cb);
Expand All @@ -404,7 +405,7 @@ RLM_API void realm_sync_config_set_after_client_reset_handler(realm_sync_config_
realm_t r1{before_realm};
auto tsr = realm_t::thread_safe_reference(std::move(after_realm));
if (!callback(userdata.get(), &r1, &tsr, did_recover)) {
throw CallbackFailed();
throw CallbackFailed{};
}
};
config->notify_after_client_reset = std::move(cb);
Expand Down
6 changes: 3 additions & 3 deletions src/realm/object-store/c_api/types.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,17 @@ class CallbackFailed : public RuntimeError {
public:
// SDK-provided opaque error value when error == RLM_ERR_CALLBACK with a callout to
// realm_register_user_code_callback_error()
void* usercode_error{nullptr};
void* user_code_error{nullptr};

CallbackFailed()
: RuntimeError(ErrorCodes::CallbackFailed, "User-provided callback failed")
{
}

CallbackFailed(void* str)
explicit CallbackFailed(void* error)
: CallbackFailed()
{
usercode_error = str;
user_code_error = error;
}
};

Expand Down
84 changes: 81 additions & 3 deletions test/object-store/c_api/c_api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,7 @@ TEST_CASE("C API (non-database)", "[c_api]") {
REQUIRE(c_err.message == status.reason());
REQUIRE(c_err.categories == RLM_ERR_CAT_RUNTIME);
REQUIRE(c_err.path == nullptr);
REQUIRE(c_err.usercode_error == nullptr);
REQUIRE(c_err.user_code_error == nullptr);
}

#if REALM_ENABLE_SYNC
Expand Down Expand Up @@ -1255,8 +1255,8 @@ TEST_CASE("C API - schema", "[c_api]") {
CHECK(realm_get_last_error(&_err));
CHECK(_err.error == RLM_ERR_CALLBACK);
CHECK(std::string{_err.message} == "User-provided callback failed");
REQUIRE(_err.usercode_error); // this is the error registered inside the callback
auto ex = (MyExceptionWrapper*)_err.usercode_error;
REQUIRE(_err.user_code_error); // this is the error registered inside the callback
auto ex = (MyExceptionWrapper*)_err.user_code_error;
try {
std::rethrow_exception(ex->m_ptr);
}
Expand Down Expand Up @@ -5532,6 +5532,84 @@ TEST_CASE("C API - client reset", "[sync][pbs][c_api][client reset][baas]") {
REQUIRE(before_client_reset_counter.load() == 1);
REQUIRE(after_client_reset_counter.load() == 0);
}

SECTION("Simulate failure during client reset with expection in the user code callback") {
error_handler_counter.store(0);
baas_client_stop.store(false);

struct ErrorState {
uintptr_t target_user_code_data = static_cast<uintptr_t>(random_int());
std::optional<uintptr_t> observed_user_code_data;
};
ErrorState state;
realm_sync_config_set_error_handler(
local_sync_config,
[](realm_userdata_t uncast_state, realm_sync_session_t*, const realm_sync_error_t sync_error) {
REQUIRE(sync_error.c_original_file_path_key);
REQUIRE(sync_error.c_recovery_file_path_key);
REQUIRE(sync_error.is_client_reset_requested);
// Callback in `realm_sync_config_set_before_client_reset_handler` fails, so
// a synthetic error is created with no action.
// Since this is a failure triggered by some exception in the user code
// an opaque ptr should have passed back to this callback in order to let
// the SDK re-throw the excpetion.
REQUIRE(sync_error.server_requests_action == RLM_SYNC_ERROR_ACTION_NO_ACTION);
ResetRealmFiles::instance().reset_realm(sync_error.c_original_file_path_key);
auto state = static_cast<ErrorState*>(uncast_state);
state->observed_user_code_data = reinterpret_cast<uintptr_t>(sync_error.user_code_error);
error_handler_counter.fetch_add(1);
baas_client_stop.store(true);
},
&state, nullptr);

SECTION("before reset exception") {
realm_sync_config_set_before_client_reset_handler(
local_sync_config,
[](realm_userdata_t uncast_state, realm_t*) -> bool {
auto state = static_cast<ErrorState*>(uncast_state);
realm_register_user_code_callback_error(
reinterpret_cast<void*>(state->target_user_code_data));
return false;
},
&state, nullptr);

make_reset(local_config, remote_config)
->on_post_reset([&](SharedRealm) {
util::EventLoop::main().run_until([&] {
return baas_client_stop.load();
});
})
->run();
}
SECTION("After reset exception") {
realm_sync_config_set_before_client_reset_handler(
local_sync_config,
[](realm_userdata_t, realm_t*) -> bool {
return true;
},
nullptr, nullptr);

realm_sync_config_set_after_client_reset_handler(
local_sync_config,
[](realm_userdata_t uncast_state, realm_t*, realm_thread_safe_reference_t*, bool) -> bool {
auto state = static_cast<ErrorState*>(uncast_state);
realm_register_user_code_callback_error(
reinterpret_cast<void*>(state->target_user_code_data));
return false;
},
&state, nullptr);

make_reset(local_config, remote_config)
->on_post_reset([&](SharedRealm) {
util::EventLoop::main().run_until([&] {
return baas_client_stop.load();
});
})
->run();
}
REQUIRE(error_handler_counter.load() == 1);
REQUIRE(state.observed_user_code_data == state.target_user_code_data);
}
}
}

Expand Down

0 comments on commit 3975f43

Please sign in to comment.