Don't duplicate error messages for SyncError (#6774)

SystemError is used in two places: as a base class for SyncError, and for
filesystem errors. Filesystem errors don't include the actual error message and
need it appended, but sync errors do and appending the message from the error
code just results in it being duplicated.

CHANGELOG.md

@@ -5,7 +5,7 @@
 
 ### Fixed
 * <How do the end-user experience this issue? what was the impact?> ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?)
-* None.
+* Sync errors included the error message twice ([PR #6774](https://github.com/realm/realm-core/pull/6774), since v13.16.0).
 
 ### Breaking changes
 * None.

src/realm/exceptions.cpp

@@ -103,6 +103,11 @@ RuntimeError::RuntimeError(ErrorCodes::Error code, std::string_view msg)
 {
     REALM_ASSERT(ErrorCodes::error_categories(code).test(ErrorCategory::runtime_error));
 }
+RuntimeError::RuntimeError(Status&& status)
+    : Exception(std::move(status))
+{
+    REALM_ASSERT(ErrorCodes::error_categories(to_status().code()).test(ErrorCategory::runtime_error));
+}
 RuntimeError::~RuntimeError() noexcept = default;
 
 InvalidArgument::InvalidArgument(std::string_view msg)

src/realm/exceptions.hpp

@@ -109,6 +109,7 @@ struct LogicError : Exception {
 
 struct RuntimeError : Exception {
     RuntimeError(ErrorCodes::Error code, std::string_view msg);
+    RuntimeError(Status&& status);
     ~RuntimeError() noexcept override;
 };
 
@@ -355,14 +356,12 @@ class FileAccessError : public RuntimeError {
 
 struct SystemError : RuntimeError {
     SystemError(std::error_code err, std::string_view msg)
-        : RuntimeError(ErrorCodes::SystemError,
-                       util::format("%1 (SystemError %2: %3)", msg, err.value(), err.message()))
+        : RuntimeError(make_status(err, msg, false))
     {
-        const_cast<Status&>(to_status()).set_std_error_code(err);
     }
 
     SystemError(int err_no, std::string_view msg)
-        : SystemError(std::error_code(err_no, std::generic_category()), msg)
+        : RuntimeError(make_status(std::error_code(err_no, std::generic_category()), msg, true))
     {
     }
 
@@ -377,6 +376,12 @@ struct SystemError : RuntimeError {
     {
         return get_system_error().category();
     }
+
+private:
+    static Status make_status(std::error_code err, std::string_view msg, bool msg_is_prefix)
+    {
+        return Status(err, msg_is_prefix ? util::format("%1: %2 (%3)", msg, err.message(), err.value()) : msg);
+    }
 };
 
 namespace query_parser {

test/object-store/sync/app.cpp

@@ -2470,7 +2470,7 @@ TEST_CASE("app: sync integration", "[sync][app][baas]") {
         auto partition = random_string(100);
         auto user1 = test_session.app()->current_user();
         SyncTestFile r_config(user1, partition, schema);
-        // Overrride the default
+        // Override the default
         r_config.sync_config->error_handler = [&](std::shared_ptr<SyncSession>, SyncError error) {
             if (error.get_system_error() == sync::make_error_code(realm::sync::ProtocolError::bad_authentication)) {
                 util::format(std::cerr, "Websocket redirect test: User logged out\n");
@@ -2771,8 +2771,7 @@ TEST_CASE("app: sync integration", "[sync][app][baas]") {
                 sync_error_handler_called.store(true);
                 REQUIRE(error.get_system_error() ==
                         sync::make_error_code(realm::sync::ProtocolError::bad_authentication));
-                REQUIRE_THAT(std::string(error.reason()),
-                             Catch::Matchers::ContainsSubstring("Unable to refresh the user access token."));
+                REQUIRE(error.reason() == "Unable to refresh the user access token.");
             };
             auto r = Realm::get_shared_realm(config);
             timed_wait_for([&] {
@@ -2872,8 +2871,7 @@ TEST_CASE("app: sync integration", "[sync][app][baas]") {
                 sync_error_handler_called.store(true);
                 REQUIRE(error.get_system_error() ==
                         sync::make_error_code(realm::sync::ProtocolError::bad_authentication));
-                REQUIRE_THAT(std::string(error.reason()),
-                             Catch::Matchers::ContainsSubstring("Unable to refresh the user access token."));
+                REQUIRE(error.reason() == "Unable to refresh the user access token.");
             };
 
             auto transport = static_cast<SynchronousTestTransport*>(session.transport());
@@ -3103,9 +3101,8 @@ TEST_CASE("app: sync integration", "[sync][app][baas]") {
 
         auto error = wait_for_future(std::move(pf.future), std::chrono::minutes(5)).get();
         REQUIRE(error.get_system_error() == make_error_code(sync::ProtocolError::limits_exceeded));
-        REQUIRE_THAT(std::string(error.reason()),
-                     Catch::Matchers::ContainsSubstring("Sync websocket closed because the server received a message "
-                                                        "that was too large: read limited at 16777217 bytes"));
+        REQUIRE(error.reason() == "Sync websocket closed because the server received a message that was too large: "
+                                  "read limited at 16777217 bytes");
         REQUIRE(error.is_client_reset_requested());
         REQUIRE(error.server_requests_action == sync::ProtocolErrorInfo::Action::ClientReset);
     }

test/test_util_file.cpp

@@ -309,12 +309,12 @@ TEST(Utils_File_SystemErrorMessage)
     std::error_code err = std::make_error_code(std::errc::too_many_files_open);
     std::string_view message = "my message";
 #ifdef _WIN32
-    std::string expected = util::format("%1 (SystemError %2: %3)", message, err.value(), "too many files open");
+    const char* expected = "my message: too many files open (%1)";
 #else
-    std::string expected = util::format("%1 (SystemError %2: %3)", message, err.value(), "Too many open files");
+    const char* expected = "my message: Too many open files (%1)";
 #endif
-    CHECK_THROW_CONTAINING_MESSAGE(throw SystemError(err, message), expected);
-    CHECK_THROW_CONTAINING_MESSAGE(throw SystemError(err.value(), message), expected);
+    CHECK_THROW_CONTAINING_MESSAGE(throw SystemError(err, message), message);
+    CHECK_THROW_CONTAINING_MESSAGE(throw SystemError(err.value(), message), util::format(expected, err.value()));
 }
 
 #endif

test/util/unit_test.cpp

@@ -1034,7 +1034,7 @@ void SimpleReporter::fail(const TestContext& context, const char* file, long lin
     if (m_report_progress) {
         m_error_messages.push_back(msg);
     }
-    context.thread_context.report_logger.info(msg.c_str());
+    context.thread_context.report_logger.info("%1", msg.c_str());
 }
 
 void SimpleReporter::thread_end(const ThreadContext& context)