From bc487096035a25e4d80e9b81e17d912d004047dd Mon Sep 17 00:00:00 2001 From: Michael Wilkerson-Barker Date: Mon, 27 Nov 2023 16:40:26 -0500 Subject: [PATCH] Fix a bunch of throw statements to use Realm exceptions (#7141) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix a bunch of throw statements to use Realm exceptions * check correct exception in test * clang-format and replaced a couple of std::exceptions in SyncManager * Updated changelog --------- Co-authored-by: Jonathan Reams Co-authored-by: Jørgen Edelbo --- CHANGELOG.md | 4 +- .../impl/emscripten/network_transport.cpp | 3 +- .../object-store/sync/impl/sync_file.cpp | 23 ++++--- src/realm/object-store/sync/sync_manager.cpp | 8 ++- .../sync/noinst/client_reset_recovery.cpp | 4 +- src/realm/sync/noinst/migration_store.cpp | 5 +- .../sync/noinst/sync_metadata_schema.cpp | 65 +++++++++++-------- src/realm/sync/subscriptions.cpp | 26 ++++---- test/test_sync_subscriptions.cpp | 2 +- 9 files changed, 84 insertions(+), 56 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e86c6ba5d4..4978c33fbef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,10 +6,10 @@ ### Fixed * ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?) -* None. +* Update existing std exceptions thrown by the Sync Client to use Realm exceptions. ([#6255](https://github.com/realm/realm-core/issues/6255), since v10.2.0) ### Breaking changes -* None. +* Update existing std exceptions thrown by the Sync Client to use Realm exceptions. ([PR #7141](https://github.com/realm/realm-core/pull/7141/files)) ### Compatibility * Fileformat: Generates files with format v23. Reads and automatically upgrade from fileformat v5. diff --git a/src/realm/object-store/sync/impl/emscripten/network_transport.cpp b/src/realm/object-store/sync/impl/emscripten/network_transport.cpp index 5344bfccc57..5616b80b473 100644 --- a/src/realm/object-store/sync/impl/emscripten/network_transport.cpp +++ b/src/realm/object-store/sync/impl/emscripten/network_transport.cpp @@ -77,7 +77,8 @@ static void error(emscripten_fetch_t* fetch) emscripten_fetch_get_response_headers(fetch, packed_headers.data(), packed_headers.size()); std::unique_ptr state(reinterpret_cast(fetch->userData)); - state->completion_block({fetch->status, 0, parse_headers(packed_headers), std::string(fetch->data, size_t(fetch->numBytes)), ErrorCodes::HTTPError}); + state->completion_block({fetch->status, 0, parse_headers(packed_headers), + std::string(fetch->data, size_t(fetch->numBytes)), ErrorCodes::HTTPError}); } void EmscriptenNetworkTransport::send_request_to_server( diff --git a/src/realm/object-store/sync/impl/sync_file.cpp b/src/realm/object-store/sync/impl/sync_file.cpp index 02da289074f..147d1f47e2e 100644 --- a/src/realm/object-store/sync/impl/sync_file.cpp +++ b/src/realm/object-store/sync/impl/sync_file.cpp @@ -61,7 +61,7 @@ uint8_t value_of_hex_digit(char hex_digit) return 10 + hex_digit - 'a'; } else { - throw std::invalid_argument("Cannot get the value of a character that isn't a hex digit."); + throw LogicError(ErrorCodes::InvalidArgument, "Cannot get the value of a character that isn't a hex digit."); } } @@ -82,7 +82,8 @@ bool character_is_unreserved(char character) char decoded_char_for(const std::string& percent_encoding, size_t index) { if (index + 2 >= percent_encoding.length()) { - throw std::invalid_argument("Malformed string: not enough characters after '%' before end of string."); + throw LogicError(ErrorCodes::InvalidArgument, + "Malformed string: not enough characters after '%' before end of string."); } REALM_ASSERT(percent_encoding[index] == '%'); return (16 * value_of_hex_digit(percent_encoding[index + 1])) + value_of_hex_digit(percent_encoding[index + 2]); @@ -126,7 +127,8 @@ std::string make_raw_string(const std::string& percent_encoded_string) else { // No need to decode. +1. if (!character_is_unreserved(current)) { - throw std::invalid_argument("Input string is invalid: contains reserved characters."); + throw LogicError(ErrorCodes::InvalidArgument, + "Input string is invalid: contains reserved characters."); } buffer.push_back(current); idx++; @@ -207,7 +209,9 @@ std::string reserve_unique_file_name(const std::string& path, const std::string& int fd = mkstemp(&path_buffer[0]); if (fd < 0) { int err = errno; - throw std::system_error(err, std::system_category()); + throw RuntimeError(ErrorCodes::FileOperationFailed, + util::format("Failed to make temporary path: %1 (%2)", + std::system_error(err, std::system_category()).what(), err)); } // Remove the file so we can use the name for our own file. #ifdef _WIN32 @@ -225,7 +229,8 @@ static std::string validate_and_clean_path(const std::string& path) REALM_ASSERT(path.length() > 0); std::string escaped_path = util::make_percent_encoded_string(path); if (filename_is_reserved(escaped_path)) - throw std::invalid_argument( + throw LogicError( + ErrorCodes::InvalidArgument, util::format("A path can't have an identifier reserved by the filesystem: '%1'", escaped_path)); return escaped_path; } @@ -419,10 +424,10 @@ std::string SyncFileManager::realm_file_path(const std::string& user_identity, } catch (const FileAccessError& e_hashed) { // hashed test path also failed, give up and report error to user. - throw std::logic_error(util::format("A valid realm path cannot be created for the " - "Realm identity '%1' at neither '%2' nor '%3'. %4", - realm_file_name, preferred_name_with_suffix, hashed_path, - e_hashed.what())); + throw LogicError(ErrorCodes::InvalidArgument, + util::format("A valid realm path cannot be created for the " + "Realm identity '%1' at neither '%2' nor '%3'. %4", + realm_file_name, preferred_name_with_suffix, hashed_path, e_hashed.what())); } } diff --git a/src/realm/object-store/sync/sync_manager.cpp b/src/realm/object-store/sync/sync_manager.cpp index eba75289246..03e4b999d4c 100644 --- a/src/realm/object-store/sync/sync_manager.cpp +++ b/src/realm/object-store/sync/sync_manager.cpp @@ -29,6 +29,8 @@ #include #include +#include + using namespace realm; using namespace realm::_impl; @@ -275,7 +277,8 @@ void SyncManager::set_logger_factory(SyncClientConfig::LoggerFactory factory) m_config.logger_factory = std::move(factory); if (m_sync_client) - throw std::logic_error("Cannot set the logger factory after creating the sync client"); + throw LogicError(ErrorCodes::IllegalOperation, + "Cannot set the logger factory after creating the sync client"); // Create a new logger using the new factory do_make_logger(); @@ -723,7 +726,8 @@ void SyncManager::set_session_multiplexing(bool allowed) return; // Already enabled, we can ignore if (m_sync_client) - throw std::logic_error("Cannot enable session multiplexing after creating the sync client"); + throw LogicError(ErrorCodes::IllegalOperation, + "Cannot enable session multiplexing after creating the sync client"); m_config.multiplex_sessions = allowed; } diff --git a/src/realm/sync/noinst/client_reset_recovery.cpp b/src/realm/sync/noinst/client_reset_recovery.cpp index cc0a9ff5347..715787079d3 100644 --- a/src/realm/sync/noinst/client_reset_recovery.cpp +++ b/src/realm/sync/noinst/client_reset_recovery.cpp @@ -237,8 +237,8 @@ InternDictKey InterningBuffer::get_interned_key(const std::string_view& str) con return key; } } - throw std::runtime_error( - util::format("InterningBuffer::get_interned_key(%1) did not contain the requested key", str)); + throw RuntimeError(ErrorCodes::InvalidArgument, + util::format("InterningBuffer::get_interned_key(%1) did not contain the requested key", str)); return {}; } diff --git a/src/realm/sync/noinst/migration_store.cpp b/src/realm/sync/noinst/migration_store.cpp index b2300105c73..5149b76a578 100644 --- a/src/realm/sync/noinst/migration_store.cpp +++ b/src/realm/sync/noinst/migration_store.cpp @@ -83,7 +83,8 @@ bool MigrationStore::load_data(bool read_only) // Load the metadata schema unless it was just created if (!m_migration_table) { if (*schema_version != c_schema_version) { - throw std::runtime_error("Invalid schema version for flexible sync migration store metadata"); + throw RuntimeError(ErrorCodes::UnsupportedFileFormatVersion, + "Invalid schema version for flexible sync migration store metadata"); } load_sync_metadata_schema(tr, &internal_tables); } @@ -381,4 +382,4 @@ std::optional MigrationStore::get_sentinel_subscription_set_version() return m_sentinel_subscription_set_version; } -} // namespace realm::sync \ No newline at end of file +} // namespace realm::sync diff --git a/src/realm/sync/noinst/sync_metadata_schema.cpp b/src/realm/sync/noinst/sync_metadata_schema.cpp index 7871a8eedc5..d82579caa73 100644 --- a/src/realm/sync/noinst/sync_metadata_schema.cpp +++ b/src/realm/sync/noinst/sync_metadata_schema.cpp @@ -38,7 +38,8 @@ void create_sync_metadata_schema(const TransactionRef& tr, std::vector found_tables; for (auto& table : *tables) { if (tr->has_table(table.name)) { - throw std::runtime_error( + throw RuntimeError( + ErrorCodes::RuntimeError, util::format("table %1 already existed when creating internal tables for sync", table.name)); } TableRef table_ref; @@ -64,18 +65,18 @@ void create_sync_metadata_schema(const TransactionRef& tr, std::vectoradd_column_list(*target_table_it->second, column.name); } else if (column.data_type == type_Link) { auto target_table_it = found_tables.find(column.target_table); if (target_table_it == found_tables.end()) { - throw std::runtime_error( - util::format("cannot link to non-existant table %1 from internal sync table %2", - column.target_table, table.name)); + throw LogicError(ErrorCodes::InvalidArgument, + util::format("cannot link to non-existant table %1 from internal sync table %2", + column.target_table, table.name)); } *column.key_out = table_ref->add_column(*target_table_it->second, column.name); } @@ -91,63 +92,75 @@ void load_sync_metadata_schema(const TransactionRef& tr, std::vectorget_table(table.name); if (!table_ref) { - throw std::runtime_error(util::format("could not find internal sync table %1", table.name)); + throw RuntimeError(ErrorCodes::RuntimeError, + util::format("could not find internal sync table %1", table.name)); } *table.key_out = table_ref->get_key(); if (table.pk_info) { auto pk_col = table_ref->get_primary_key_column(); if (auto pk_name = table_ref->get_column_name(pk_col); pk_name != table.pk_info->name) { - throw std::runtime_error(util::format( - "primary key name of sync internal table %1 does not match (stored: %2, defined: %3)", table.name, - pk_name, table.pk_info->name)); + throw RuntimeError( + ErrorCodes::RuntimeError, + util::format( + "primary key name of sync internal table %1 does not match (stored: %2, defined: %3)", + table.name, pk_name, table.pk_info->name)); } if (auto pk_type = table_ref->get_column_type(pk_col); pk_type != table.pk_info->data_type) { - throw std::runtime_error(util::format( - "primary key type of sync internal table %1 does not match (stored: %2, defined: %3)", table.name, - pk_type, table.pk_info->data_type)); + throw RuntimeError( + ErrorCodes::RuntimeError, + util::format( + "primary key type of sync internal table %1 does not match (stored: %2, defined: %3)", + table.name, pk_type, table.pk_info->data_type)); } if (auto is_nullable = table_ref->is_nullable(pk_col); is_nullable != table.pk_info->is_optional) { - throw std::runtime_error(util::format( - "primary key nullabilty of sync internal table %1 does not match (stored: %2, defined: %3)", - table.name, is_nullable, table.pk_info->is_optional)); + throw RuntimeError( + ErrorCodes::RuntimeError, + util::format( + "primary key nullabilty of sync internal table %1 does not match (stored: %2, defined: %3)", + table.name, is_nullable, table.pk_info->is_optional)); } *table.pk_info->key_out = pk_col; } else if (table.is_embedded && !table_ref->is_embedded()) { - throw std::runtime_error( - util::format("internal sync table %1 should be embedded, but is not", table.name)); + throw RuntimeError(ErrorCodes::RuntimeError, + util::format("internal sync table %1 should be embedded, but is not", table.name)); } if (table.columns.size() + size_t(table.pk_info ? 1 : 0) != table_ref->get_column_count()) { - throw std::runtime_error( + throw RuntimeError( + ErrorCodes::RuntimeError, util::format("sync internal table %1 has a different number of columns than its schema", table.name)); } for (auto& col : table.columns) { auto col_key = table_ref->get_column_key(col.name); if (!col_key) { - throw std::runtime_error( + throw RuntimeError( + ErrorCodes::RuntimeError, util::format("column %1 is missing in sync internal table %2", col.name, table.name)); } auto found_col_type = table_ref->get_column_type(col_key); if (found_col_type != col.data_type) { - throw std::runtime_error( + throw RuntimeError( + ErrorCodes::RuntimeError, util::format("column %1 in sync internal table %2 is the wrong type", col.name, table.name)); } if (col.is_optional != table_ref->is_nullable(col_key)) { - throw std::runtime_error( + throw RuntimeError( + ErrorCodes::RuntimeError, util::format("column %1 in sync internal table %2 has different nullabilty than in its schema", col.name, table.name)); } if (col.data_type == type_LinkList && table_ref->get_link_target(col_key)->get_name() != col.target_table) { - throw std::runtime_error( - util::format("column %1 in sync internal table %2 links to the wrong table %3", col.name, - table.name, table_ref->get_link_target(col_key)->get_name())); + throw RuntimeError(ErrorCodes::RuntimeError, + util::format("column %1 in sync internal table %2 links to the wrong table %3", + col.name, table.name, + table_ref->get_link_target(col_key)->get_name())); } *col.key_out = col_key; } diff --git a/src/realm/sync/subscriptions.cpp b/src/realm/sync/subscriptions.cpp index 6c65e9de27b..0b902f6ac0c 100644 --- a/src/realm/sync/subscriptions.cpp +++ b/src/realm/sync/subscriptions.cpp @@ -86,7 +86,8 @@ SubscriptionSet::State state_from_storage(int64_t value) case SubscriptionStateForStorage::Error: return SubscriptionSet::State::Error; default: - throw std::runtime_error(util::format("Invalid state for SubscriptionSet stored on disk: %1", value)); + throw RuntimeError(ErrorCodes::InvalidArgument, + util::format("Invalid state for SubscriptionSet stored on disk: %1", value)); } } @@ -206,7 +207,7 @@ std::shared_ptr SubscriptionSet::get_flx_subscription_s if (auto mgr = m_mgr.lock()) { return mgr; } - throw std::logic_error("Active SubscriptionSet without a SubscriptionStore"); + throw RuntimeError(ErrorCodes::BrokenInvariant, "Active SubscriptionSet without a SubscriptionStore"); } int64_t SubscriptionSet::version() const @@ -420,19 +421,21 @@ void MutableSubscriptionSet::update_state(State new_state, util::Optionalget_transact_stage() != DB::transact_Writing) { - throw std::logic_error("SubscriptionSet is not in a commitable state"); + throw RuntimeError(ErrorCodes::WrongTransactionState, "SubscriptionSet is not in a commitable state"); } auto mgr = get_flx_subscription_store(); // Throws @@ -690,7 +693,8 @@ SubscriptionStore::SubscriptionStore(DBRef db) } else { if (*schema_version != c_flx_schema_version) { - throw std::runtime_error("Invalid schema version for flexible sync metadata"); + throw RuntimeError(ErrorCodes::UnsupportedFileFormatVersion, + "Invalid schema version for flexible sync metadata"); } load_sync_metadata_schema(tr, &internal_tables); } diff --git a/test/test_sync_subscriptions.cpp b/test/test_sync_subscriptions.cpp index 916287b0da0..7398878d936 100644 --- a/test/test_sync_subscriptions.cpp +++ b/test/test_sync_subscriptions.cpp @@ -412,7 +412,7 @@ TEST(Sync_SubscriptionStoreRefreshSubscriptionSetInvalid) store.reset(); // Throws since the SubscriptionStore is gone. - CHECK_THROW(latest->refresh(), std::logic_error); + CHECK_THROW(latest->refresh(), RuntimeError); } TEST(Sync_SubscriptionStoreInternalSchemaMigration)