Skip to content

Commit

Permalink
RCORE-2150 Include originating client reset error message when report…
Browse files Browse the repository at this point in the history
…ing auto client reset failures (#7761)

* Added originating client reset error to reason strings of AutoClientResetFailed errors

* Updated changelog
  • Loading branch information
Michael Wilkerson-Barker authored Jun 3, 2024
1 parent 352b981 commit e246b7d
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 25 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

### Enhancements
* <New feature description> (PR [#????](https://github.com/realm/realm-core/pull/????))
* None.
* Include the originating client reset error in AutoClientResetFailure errors. ([#7761](https://github.com/realm/realm-core/pull/7761))

### Fixed
* <How do the end-user experience this issue? what was the impact?> ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?)
Expand Down
28 changes: 15 additions & 13 deletions src/realm/object-store/sync/sync_session.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -429,8 +429,10 @@ void SyncSession::download_fresh_realm(const sync::SessionErrorInfo& error_info)
auto mode = config(&SyncConfig::client_resync_mode);
if (mode == ClientResyncMode::Recover) {
handle_fresh_realm_downloaded(
nullptr, {ErrorCodes::RuntimeError,
"A client reset is required but the server does not permit recovery for this client"});
nullptr,
{ErrorCodes::RuntimeError,
"A client reset is required but the server does not permit recovery for this client"},
error_info);
return;
}
}
Expand Down Expand Up @@ -467,7 +469,7 @@ void SyncSession::download_fresh_realm(const sync::SessionErrorInfo& error_info)
catch (...) {
// Failed to open the fresh path after attempting to delete it, so we
// just can't do automatic recovery.
handle_fresh_realm_downloaded(nullptr, exception_to_status());
handle_fresh_realm_downloaded(nullptr, exception_to_status(), error_info);
return;
}

Expand Down Expand Up @@ -535,10 +537,10 @@ void SyncSession::download_fresh_realm(const sync::SessionErrorInfo& error_info)
// it immediately
fresh_sync_session->force_close();
if (subs.is_ok()) {
self->handle_fresh_realm_downloaded(db, std::move(error_info), std::move(subs.get_value()));
self->handle_fresh_realm_downloaded(db, Status::OK(), error_info, std::move(subs.get_value()));
}
else {
self->handle_fresh_realm_downloaded(nullptr, std::move(subs.get_status()));
self->handle_fresh_realm_downloaded(nullptr, std::move(subs.get_status()), error_info);
}
});
}
Expand All @@ -549,18 +551,18 @@ void SyncSession::download_fresh_realm(const sync::SessionErrorInfo& error_info)
fresh_sync_session->force_close();
if (auto strong_self = weak_self.lock()) {
if (status.is_ok()) {
strong_self->handle_fresh_realm_downloaded(db, std::move(error_info));
strong_self->handle_fresh_realm_downloaded(db, Status::OK(), error_info);
}
else {
strong_self->handle_fresh_realm_downloaded(nullptr, std::move(status));
strong_self->handle_fresh_realm_downloaded(nullptr, std::move(status), error_info);
}
}
});
}
fresh_sync_session->revive_if_needed();
}

void SyncSession::handle_fresh_realm_downloaded(DBRef db, StatusWith<sync::SessionErrorInfo> error_info,
void SyncSession::handle_fresh_realm_downloaded(DBRef db, Status result, const sync::SessionErrorInfo& cr_error_info,
std::optional<sync::SubscriptionSet> new_subs)
{
util::CheckedUniqueLock lock(m_state_mutex);
Expand All @@ -571,15 +573,16 @@ void SyncSession::handle_fresh_realm_downloaded(DBRef db, StatusWith<sync::Sessi
// - unable to write the fresh copy to the file system
// - during download of the fresh copy, the fresh copy itself is reset
// - in FLX mode there was a problem fulfilling the previously active subscription
if (!error_info.is_ok()) {
if (error_info.get_status() == ErrorCodes::OperationAborted) {
if (!result.is_ok()) {
if (result == ErrorCodes::OperationAborted) {
return;
}
lock.unlock();

sync::SessionErrorInfo synthetic(
Status{ErrorCodes::AutoClientResetFailed,
util::format("A fatal error occurred during client reset: '%1'", error_info.get_status())},
util::format("A fatal error occurred during '%1' client reset for %2: '%3'",
cr_error_info.server_requests_action, cr_error_info.status, result)},
sync::IsFatal{true});
handle_error(synthetic);
return;
Expand All @@ -598,8 +601,7 @@ void SyncSession::handle_fresh_realm_downloaded(DBRef db, StatusWith<sync::Sessi
m_client_reset_fresh_copy = db;
CompletionCallbacks callbacks;
// Save the client reset error for when the original sync session is revived
REALM_ASSERT(error_info.is_ok()); // required if we get here
m_client_reset_error = std::move(error_info.get_value());
m_client_reset_error = cr_error_info;

std::swap(m_completion_callbacks, callbacks);
// always swap back, even if advance_state throws
Expand Down
2 changes: 1 addition & 1 deletion src/realm/object-store/sync/sync_session.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ class SyncSession : public std::enable_shared_from_this<SyncSession> {

void download_fresh_realm(const sync::SessionErrorInfo& error_info)
REQUIRES(!m_config_mutex, !m_state_mutex, !m_connection_state_mutex);
void handle_fresh_realm_downloaded(DBRef db, StatusWith<sync::SessionErrorInfo> error_info,
void handle_fresh_realm_downloaded(DBRef db, Status result, const sync::SessionErrorInfo& cr_error_info,
std::optional<sync::SubscriptionSet> new_subs = std::nullopt)
REQUIRES(!m_state_mutex, !m_config_mutex, !m_connection_state_mutex);
void handle_error(sync::SessionErrorInfo) REQUIRES(!m_state_mutex, !m_config_mutex, !m_connection_state_mutex);
Expand Down
12 changes: 11 additions & 1 deletion src/realm/sync/noinst/client_impl_base.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2345,11 +2345,21 @@ Status Session::receive_ident_message(SaltedFileIdent client_file_ident)
// if a client reset happens, it will take care of setting the file ident
// and if not, we do it here
bool did_client_reset = false;

// Save some of the client reset info for reporting to the client if an error occurs.
Status cr_status(Status::OK()); // Start with no client reset
ProtocolErrorInfo::Action cr_action = ProtocolErrorInfo::Action::NoAction;
if (auto& cr_config = get_client_reset_config()) {
cr_status = cr_config->error;
cr_action = cr_config->action;
}

try {
did_client_reset = client_reset_if_needed();
}
catch (const std::exception& e) {
auto err_msg = util::format("A fatal error occurred during client reset: '%1'", e.what());
auto err_msg = util::format("A fatal error occurred during '%1' client reset for %2: '%3'", cr_action,
cr_status, e.what());
logger.error(err_msg.c_str());
SessionErrorInfo err_info(Status{ErrorCodes::AutoClientResetFailed, err_msg}, IsFatal{true});
suspend(err_info);
Expand Down
10 changes: 6 additions & 4 deletions test/object-store/sync/flx_sync.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -908,6 +908,8 @@ TEST_CASE("flx: client reset", "[sync][flx][client reset][baas]") {
REQUIRE(before_reset_count == 1);
REQUIRE(after_reset_count == 0);
REQUIRE(sync_error.status == ErrorCodes::AutoClientResetFailed);
REQUIRE(sync_error.status.reason().find(
"SyncClientResetRequired: Bad client file identifier (IDENT)") != std::string::npos);
REQUIRE(sync_error.is_client_reset_requested());
local_realm->refresh();
auto table = local_realm->read_group().get_table("class_TopLevel");
Expand Down Expand Up @@ -1133,6 +1135,8 @@ TEST_CASE("flx: client reset", "[sync][flx][client reset][baas]") {
auto sync_error = wait_for_future(std::move(err_future)).get();
INFO(sync_error.status);
CHECK(sync_error.status == ErrorCodes::AutoClientResetFailed);
REQUIRE(sync_error.status.reason().find(
"SyncClientResetRequired: Bad client file identifier (IDENT)") != std::string::npos);
})
->run();
}
Expand Down Expand Up @@ -1420,8 +1424,7 @@ TEST_CASE("flx: client reset", "[sync][flx][client reset][baas]") {
REQUIRE(!ref);
REQUIRE(error);
REQUIRE_THROWS_CONTAINING(std::rethrow_exception(error),
"A fatal error occurred during client reset: 'Client reset cannot recover when "
"classes have been removed: {AddedClass}'");
"'Client reset cannot recover when classes have been removed: {AddedClass}'");
});
error_future.get();
CHECK(before_reset_count == 1);
Expand All @@ -1441,8 +1444,7 @@ TEST_CASE("flx: client reset", "[sync][flx][client reset][baas]") {
REQUIRE(error);
REQUIRE_THROWS_CONTAINING(
std::rethrow_exception(error),
"A fatal error occurred during client reset: 'The following changes cannot be "
"made in additive-only schema mode:\n"
"'The following changes cannot be made in additive-only schema mode:\n"
"- Property 'TopLevel._id' has been changed from 'object id' to 'uuid'.\nIf your app is running in "
"development mode, you can delete the realm and restart the app to update your schema.'");
});
Expand Down
11 changes: 6 additions & 5 deletions test/test_client_reset.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -730,12 +730,12 @@ TEST(ClientReset_DoNotRecoverSchema)
// schema change is not allowed and so fails with a client reset error.
{
Session::Config session_config;
std::string error_msg = "Some bad client file identifier (IDENT)";
{
Session::Config::ClientReset cr_config{
ClientResyncMode::DiscardLocal,
sg_fresh1,
{ErrorCodes::SyncClientResetRequired, "Bad client file identifier (IDENT)"},
sync::ProtocolErrorInfo::Action::ClientReset};
Session::Config::ClientReset cr_config{ClientResyncMode::DiscardLocal,
sg_fresh1,
{ErrorCodes::SyncClientResetRequired, error_msg},
sync::ProtocolErrorInfo::Action::ClientReset};
session_config.client_reset_config = std::move(cr_config);
}

Expand All @@ -746,6 +746,7 @@ TEST(ClientReset_DoNotRecoverSchema)
return;
REALM_ASSERT(error_info);
CHECK_EQUAL(error_info->status, ErrorCodes::AutoClientResetFailed);
CHECK(error_info->status.reason().find(error_msg) != std::string::npos);
bowl.add_stone();
};
Session session = fixture.make_session(path_1, server_path_2, std::move(session_config));
Expand Down

0 comments on commit e246b7d

Please sign in to comment.