raise<ErrorCode>: correcty use format string

cpp/arcticdb/storage/azure/azure_mock_client.cpp

@@ -44,8 +44,8 @@ std::optional<Azure::Core::RequestFailedException> has_failure_trigger(const std
  auto error_code = blob_name.substr(start, blob_name.find_last_of('_') - start);
  auto status_code_string = blob_name.substr(blob_name.find_last_of('_') + 1);
  auto status_code = Azure::Core::Http::HttpStatusCode(std::stoi(status_code_string));
- auto error_message = fmt::format("Simulated Error, message: operation {}, error code {} statuscode {}",
- operation_to_string(operation), error_code, static_cast<int>(status_code));
+ auto error_message = fmt::format("Simulated Error, message: operation {}, blob name {} error code {} statuscode {}",
+ operation_to_string(operation), blob_name, error_code, static_cast<int>(status_code));
 
  return get_exception(error_message, error_code, status_code);
  } catch (std::exception&) {

cpp/arcticdb/storage/azure/azure_storage.cpp

@@ -64,27 +64,24 @@ std::string get_error_code(const Azure::Core::RequestFailedException& e) {
 void raise_azure_exception(const Azure::Core::RequestFailedException& e) {
  auto error_code = get_error_code(e);
  auto status_code = e.StatusCode;
- std::string error_message;
 
  if(status_code == Azure::Core::Http::HttpStatusCode::NotFound && error_code == AzureErrorCode_to_string(AzureErrorCode::BlobNotFound)) {
  throw KeyNotFoundException(fmt::format("Key Not Found Error: AzureError#{} {}: {}",
  static_cast<int>(status_code), error_code, e.ReasonPhrase));
  }
 
  if(status_code == Azure::Core::Http::HttpStatusCode::Unauthorized || status_code == Azure::Core::Http::HttpStatusCode::Forbidden) {
- raise<ErrorCode::E_PERMISSION>(fmt::format("Permission error: AzureError#{} {}: {}",
-  static_cast<int>(status_code), error_code, e.ReasonPhrase));
+ raise<ErrorCode::E_PERMISSION>("Permission error: AzureError#{} {}: {}",
+ static_cast<int>(status_code), error_code, e.ReasonPhrase);
  }
 
  if(static_cast<int>(status_code) >= 500) {
- error_message = fmt::format("Unexpected Server Error: AzureError#{} {}: {} {}",
+ raise<ErrorCode::E_UNEXPECTED_AZURE_ERROR>("Unexpected Server Error: AzureError#{} {}: {} {}",
  static_cast<int>(status_code), error_code, e.ReasonPhrase, e.what());
  } else {
- error_message = fmt::format("Unexpected Error: AzureError#{} {}: {} {}",
+ raise<ErrorCode::E_UNEXPECTED_AZURE_ERROR>("Unexpected Error: AzureError#{} {}: {} {}",
  static_cast<int>(status_code), error_code, e.ReasonPhrase, e.what());
  }
-
- raise<ErrorCode::E_UNEXPECTED_AZURE_ERROR>(error_message);
 }
 
 bool is_expected_error_type(const std::string& error_code, Azure::Core::Http::HttpStatusCode status_code) {

cpp/arcticdb/storage/library_manager.cpp

@@ -68,7 +68,7 @@ bool is_storage_config_ok(const arcticdb::proto::storage::VariantStorage& storag
  if (is_ok) {
  return true;
  } else if (throw_on_failure) {
- internal::raise<ErrorCode::E_STORED_CONFIG_ERROR>(error_message);
+ internal::raise<ErrorCode::E_STORED_CONFIG_ERROR>("{}", error_message);
  } else {
  return false;
  }

cpp/arcticdb/storage/lmdb/lmdb_storage.cpp

@@ -45,7 +45,7 @@ void raise_lmdb_exception(const ::lmdb::error& e) {
  throw LMDBMapFullException(fmt::format("Map Full Error: LMDBError#{}: {}", error_code, e.what()));
  }
 
- raise<ErrorCode::E_UNEXPECTED_LMDB_ERROR>(fmt::format("Unexpected LMDB Error: LMDBError#{}: {}", error_code, e.what()));
+ raise<ErrorCode::E_UNEXPECTED_LMDB_ERROR>("Unexpected LMDB Error: LMDBError#{}: {}", error_code, e.what());
 }
 
 ::lmdb::env& LmdbStorage::env() {
@@ -300,8 +300,8 @@ void remove_db_files(const fs::path& lib_path) {
  }
  } catch (const std::filesystem::filesystem_error& e) {
  raise<ErrorCode::E_UNEXPECTED_LMDB_ERROR>(
- fmt::format("Unexpected LMDB Error: Failed to remove LMDB file at path: {} error: {}",
-  file_path.string(), e.what()));
+ "Unexpected LMDB Error: Failed to remove LMDB file at path: {} error: {}",
+ file_path.string(), e.what());
  }
  }
 
@@ -314,8 +314,8 @@ void remove_db_files(const fs::path& lib_path) {
  fs::remove_all(lib_path);
  } catch (const fs::filesystem_error& e) {
  raise<ErrorCode::E_UNEXPECTED_LMDB_ERROR>(
- fmt::format("Unexpected LMDB Error: Failed to remove directory: {} error: {}",
-  lib_path.string(), e.what()));
+ "Unexpected LMDB Error: Failed to remove directory: {} error: {}",
+ lib_path.string(), e.what());
  }
  }
  }

cpp/arcticdb/storage/mongo/mongo_mock_client.cpp

@@ -66,8 +66,8 @@ std::optional<MongoFailure> has_failure_trigger(
  try {
  auto start = position + failure_string_for_operation.size();
  auto error_code = MongoError(stoi(key_id.substr(start)));
- auto error_message = fmt::format("Simulated Error, message: operation {}, error code {}",
- operation_to_string(operation), static_cast<int>(error_code));
+ auto error_message = fmt::format("Simulated Error, message: operation {}, key {}, error code {}",
+ operation_to_string(operation), key_id, static_cast<int>(error_code));
 
  return get_failure(error_message, operation, error_code);
  } catch (std::exception&) {

cpp/arcticdb/storage/mongo/mongo_storage.cpp

@@ -44,11 +44,11 @@ void raise_mongo_exception(const mongocxx::operation_exception& e) {
  }
 
  if (error_code == static_cast<int>(MongoError::UnAuthorized) || error_code == static_cast<int>(MongoError::AuthenticationFailed)) {
- raise<ErrorCode::E_PERMISSION>(fmt::format("Permission error: MongoError#{}: {}", error_code, e.what()));
+ raise<ErrorCode::E_PERMISSION>("Permission error: MongoError#{}: {}", error_code, e.what());
  }
 
- raise<ErrorCode::E_UNEXPECTED_MONGO_ERROR>(fmt::format("Unexpected Mongo Error: MongoError#{}: {} {} {}",
-  error_code, e.code().category().name(), e.code().message(), e.what()));
+ raise<ErrorCode::E_UNEXPECTED_MONGO_ERROR>("Unexpected Mongo Error: MongoError#{}: {} {} {}",
+ error_code, e.code().category().name(), e.code().message(), e.what());
 }
 
 bool is_expected_error_type(int error_code) {

cpp/arcticdb/storage/rocksdb/rocksdb_storage.cpp

@@ -69,14 +69,14 @@ RocksDBStorage::RocksDBStorage(const LibraryPath &library_path, OpenMode mode, c
  db_options.create_if_missing = true;
  db_options.create_missing_column_families = true;
  } else {
- raise<ErrorCode::E_UNEXPECTED_ROCKSDB_ERROR>(DEFAULT_ROCKSDB_NOT_OK_ERROR + s.ToString());
+ raise<ErrorCode::E_UNEXPECTED_ROCKSDB_ERROR>("{}", DEFAULT_ROCKSDB_NOT_OK_ERROR + s.ToString());
  }
 
  std::vector<::rocksdb::ColumnFamilyHandle*> handles;
  // Note: the "default" handle will be returned as well. It is necessary to delete this, but not
  // rocksdb's internal handle to the default column family as returned by DefaultColumnFamily().
  s = ::rocksdb::DB::Open(db_options, db_name_, column_families, &handles, &db_);
- storage::check<ErrorCode::E_UNEXPECTED_ROCKSDB_ERROR>(s.ok(), DEFAULT_ROCKSDB_NOT_OK_ERROR + s.ToString());
+ storage::check<ErrorCode::E_UNEXPECTED_ROCKSDB_ERROR>(s.ok(), "{}", DEFAULT_ROCKSDB_NOT_OK_ERROR + s.ToString());
  util::check(handles.size() == column_families.size(), "Open returned wrong number of handles.");
  for (std::size_t i = 0; i < handles.size(); i++) {
  handles_by_key_type_.emplace(column_families[i].name, handles[i]);
@@ -87,11 +87,11 @@ RocksDBStorage::RocksDBStorage(const LibraryPath &library_path, OpenMode mode, c
 RocksDBStorage::~RocksDBStorage() {
  for (const auto& [key_type_name, handle]: handles_by_key_type_) {
  auto s = db_->DestroyColumnFamilyHandle(handle);
- storage::check<ErrorCode::E_UNEXPECTED_ROCKSDB_ERROR>(s.ok(), DEFAULT_ROCKSDB_NOT_OK_ERROR + s.ToString());
+ storage::check<ErrorCode::E_UNEXPECTED_ROCKSDB_ERROR>(s.ok(), "{}", DEFAULT_ROCKSDB_NOT_OK_ERROR + s.ToString());
  }
  handles_by_key_type_.clear();
  auto s = db_->Close();
- storage::check<ErrorCode::E_UNEXPECTED_ROCKSDB_ERROR>(s.ok(), DEFAULT_ROCKSDB_NOT_OK_ERROR + s.ToString());
+ storage::check<ErrorCode::E_UNEXPECTED_ROCKSDB_ERROR>(s.ok(), "{}", DEFAULT_ROCKSDB_NOT_OK_ERROR + s.ToString());
  delete db_;
 }
 
@@ -130,7 +130,7 @@ void RocksDBStorage::do_read(Composite<VariantKey>&& ks, const ReadVisitor& visi
  std::string k_str = to_serialized_key(k);
  std::string value;
  auto s = db_->Get(::rocksdb::ReadOptions(), handle, ::rocksdb::Slice(k_str), &value);
- storage::check<ErrorCode::E_UNEXPECTED_ROCKSDB_ERROR>(s.IsNotFound() || s.ok(), DEFAULT_ROCKSDB_NOT_OK_ERROR + s.ToString());
+ storage::check<ErrorCode::E_UNEXPECTED_ROCKSDB_ERROR>(s.IsNotFound() || s.ok(), "{}", DEFAULT_ROCKSDB_NOT_OK_ERROR + s.ToString());
  if(s.IsNotFound()) {
  log::storage().log(
  opts.dont_warn_about_missing_key ? spdlog::level::debug : spdlog::level::warn,
@@ -157,7 +157,7 @@ bool RocksDBStorage::do_key_exists(const VariantKey& key) {
  return false;
  }
  auto s = db_->Get(::rocksdb::ReadOptions(), handle, ::rocksdb::Slice(k_str), &value);
- storage::check<ErrorCode::E_UNEXPECTED_ROCKSDB_ERROR>(s.IsNotFound() || s.ok(), DEFAULT_ROCKSDB_NOT_OK_ERROR + s.ToString());
+ storage::check<ErrorCode::E_UNEXPECTED_ROCKSDB_ERROR>(s.IsNotFound() || s.ok(), "{}", DEFAULT_ROCKSDB_NOT_OK_ERROR + s.ToString());
  return !s.IsNotFound();
 }
 
@@ -201,7 +201,7 @@ void RocksDBStorage::do_iterate_type(KeyType key_type, const IterateTypeVisitor&
  }
  }
  auto s = it->status();
- storage::check<ErrorCode::E_UNEXPECTED_ROCKSDB_ERROR>(s.IsNotFound() || s.ok(), DEFAULT_ROCKSDB_NOT_OK_ERROR + s.ToString());
+ storage::check<ErrorCode::E_UNEXPECTED_ROCKSDB_ERROR>(s.IsNotFound() || s.ok(), "{}",DEFAULT_ROCKSDB_NOT_OK_ERROR + s.ToString());
 }
 
 std::vector<VariantKey> RocksDBStorage::do_remove_internal(Composite<VariantKey>&& ks, RemoveOpts opts) {
@@ -216,7 +216,7 @@ std::vector<VariantKey> RocksDBStorage::do_remove_internal(Composite<VariantKey>
  if (do_key_exists(k)) {
  auto k_str = to_serialized_key(k);
  auto s = db_->Delete(::rocksdb::WriteOptions(), handle, ::rocksdb::Slice(k_str));
- storage::check<ErrorCode::E_UNEXPECTED_ROCKSDB_ERROR>(s.ok(), DEFAULT_ROCKSDB_NOT_OK_ERROR + s.ToString());
+ storage::check<ErrorCode::E_UNEXPECTED_ROCKSDB_ERROR>(s.ok(), "{}", DEFAULT_ROCKSDB_NOT_OK_ERROR + s.ToString());
  ARCTICDB_DEBUG(log::storage(), "Deleted segment for key {}", variant_key_view(k));
  } else if (!opts.ignores_missing_key_) {
  log::storage().warn("Failed to delete segment for key {}", variant_key_view(k));
@@ -246,7 +246,7 @@ void RocksDBStorage::do_write_internal(Composite<KeySegmentPair>&& kvs) {
  throw DuplicateKeyException(kv.variant_key());
  }
  auto s = db_->Put(::rocksdb::WriteOptions(), handle, ::rocksdb::Slice(k_str), ::rocksdb::Slice(seg_data));
- storage::check<ErrorCode::E_UNEXPECTED_ROCKSDB_ERROR>(s.ok(), DEFAULT_ROCKSDB_NOT_OK_ERROR + s.ToString());
+ storage::check<ErrorCode::E_UNEXPECTED_ROCKSDB_ERROR>(s.ok(), "{}", DEFAULT_ROCKSDB_NOT_OK_ERROR + s.ToString());
  }
  });
 }

cpp/arcticdb/storage/s3/detail-inl.hpp

@@ -45,7 +45,6 @@ namespace s3 {
  using Range = folly::Range<It>;
 
  inline void raise_s3_exception(const Aws::S3::S3Error& err){
- std::string error_message;
  auto type = err.GetErrorType();
  // s3_client.HeadObject returns RESOURCE_NOT_FOUND if a key is not found.
  if(type == Aws::S3::S3Errors::NO_SUCH_KEY || type == Aws::S3::S3Errors::RESOURCE_NOT_FOUND) {
@@ -56,22 +55,22 @@ namespace s3 {
  }
 
  if(type == Aws::S3::S3Errors::ACCESS_DENIED || type == Aws::S3::S3Errors::INVALID_ACCESS_KEY_ID || type == Aws::S3::S3Errors::SIGNATURE_DOES_NOT_MATCH) {
- raise<ErrorCode::E_PERMISSION>(fmt::format("Permission error: S3Error#{} {}: {}",
-  int(err.GetErrorType()),
-  err.GetExceptionName().c_str(),
-  err.GetMessage().c_str()));
+ raise<ErrorCode::E_PERMISSION>("Permission error: S3Error#{} {}: {}",
+ int(err.GetErrorType()),
+ err.GetExceptionName().c_str(),
+ err.GetMessage().c_str());
  }
 
  if(err.ShouldRetry()) {
- raise<ErrorCode::E_S3_RETRYABLE>(fmt::format("Retry-able error: S3Error#{} {}: {}",
-  int(err.GetErrorType()),
-  err.GetExceptionName().c_str(),
-  err.GetMessage().c_str()));
+ raise<ErrorCode::E_S3_RETRYABLE>("Retry-able error: S3Error#{} {}: {}",
+ int(err.GetErrorType()),
+ err.GetExceptionName().c_str(),
+ err.GetMessage().c_str());
  }
 
  // We create a more detailed error explanation in case of NETWORK_CONNECTION errors to remedy #880.
  if (type == Aws::S3::S3Errors::NETWORK_CONNECTION) {
- error_message = fmt::format("Unexpected network error: S3Error#{}: {} {} "
+ raise<ErrorCode::E_UNEXPECTED_S3_ERROR>("Unexpected network error: S3Error#{}: {} {} "
  "This could be due to a connectivity issue or too many open Arctic instances. "
  "Having more than one open Arctic instance is not advised, you should reuse them. "
  "If you absolutely need many open Arctic instances, consider increasing `ulimit -n`.",
@@ -80,14 +79,11 @@ namespace s3 {
  err.GetMessage().c_str());
  }
  else {
- error_message = fmt::format("Unexpected error: S3Error#{} {}: {}",
- int(err.GetErrorType()),
- err.GetExceptionName().c_str(),
- err.GetMessage().c_str());
+ raise<ErrorCode::E_UNEXPECTED_S3_ERROR>("Unexpected error: S3Error#{} {}: {}",
+  int(err.GetErrorType()),
+  err.GetExceptionName().c_str(),
+  err.GetMessage().c_str());
  }
-
- log::storage().error(error_message);
- raise<ErrorCode::E_UNEXPECTED_S3_ERROR>(error_message);
  }
 
  inline bool is_expected_error_type(Aws::S3::S3Errors err) {
@@ -259,9 +255,8 @@ namespace s3 {
  failed_deletes_message<<", ";
  }
  }
- auto error_message = fmt::format("Failed to delete some of the objects: {}.",
+ raise<ErrorCode::E_UNEXPECTED_S3_ERROR>("Failed to delete some of the objects: {}.",
  failed_deletes_message.str());
- raise<ErrorCode::E_UNEXPECTED_S3_ERROR>(error_message);
  }
  }
 

cpp/arcticdb/storage/s3/s3_mock_client.cpp

@@ -41,7 +41,8 @@ std::optional<Aws::S3::S3Error> has_failure_trigger(const std::string& s3_object
  auto failure_code_string = s3_object_name.substr(start, s3_object_name.find_last_of('_') - start);
  auto failure_code = Aws::S3::S3Errors(std::stoi(failure_code_string));
  bool retryable = std::stoi(s3_object_name.substr(s3_object_name.find_last_of('_') + 1));
- return Aws::S3::S3Error(Aws::Client::AWSError<Aws::S3::S3Errors>(failure_code, "Simulated error", "Simulated error message", retryable));
+ return Aws::S3::S3Error(Aws::Client::AWSError<Aws::S3::S3Errors>(failure_code, "Simulated error",
+ fmt::format("Simulated error message for object {}", s3_object_name),retryable));
  } catch (std::exception&) {
  return std::nullopt;
  }

cpp/arcticdb/storage/test/test_storage_exceptions.cpp

@@ -407,7 +407,7 @@ TEST(S3MockStorageTest, TestUnexpectedS3ErrorException ) {
  S3MockStorageFactory factory;
  auto storage = factory.create();
 
- std::string failureSymbol = s3::MockS3Client::get_failure_trigger("sym1", StorageOperation::READ, Aws::S3::S3Errors::NETWORK_CONNECTION, false);
+ std::string failureSymbol = s3::MockS3Client::get_failure_trigger("sym{1}", StorageOperation::READ, Aws::S3::S3Errors::NETWORK_CONNECTION, false);
 
  ASSERT_THROW({
  read_in_store(*storage, failureSymbol);
@@ -460,7 +460,7 @@ TEST(AzureMockStorageTest, TestUnexpectedAzureErrorException ) {
  read_in_store(*storage, failureSymbol);
  }, UnexpectedAzureException);
 
- failureSymbol = azure::MockAzureClient::get_failure_trigger("sym1", StorageOperation::READ,
+ failureSymbol = azure::MockAzureClient::get_failure_trigger("sym{1}", StorageOperation::READ,
  "",
  Azure::Core::Http::HttpStatusCode::InternalServerError);
 
@@ -552,7 +552,7 @@ TEST(MongoMockStorageTest, test_list) {
  }
  ASSERT_EQ(list_in_store(*store), symbols);
 
- write_in_store(*store, mongo::MockMongoClient::get_failure_trigger("symbol_99", StorageOperation::LIST, mongo::MongoError::HostNotFound));
+ write_in_store(*store, mongo::MockMongoClient::get_failure_trigger("symbol_{99}", StorageOperation::LIST, mongo::MongoError::HostNotFound));
 
  ASSERT_THROW(list_in_store(*store), UnexpectedMongoException);
 }

cpp/arcticdb/version/version_store_api.cpp

@@ -480,7 +480,7 @@ void PythonVersionStore::snapshot(
  error_msg += fmt::format("{}:{} ", kv.first, kv.second);
  }
  }
- missing_data::raise<ErrorCode::E_NO_SUCH_VERSION>(error_msg);
+ missing_data::raise<ErrorCode::E_NO_SUCH_VERSION>("{}", error_msg);
  }
  }
  index_keys = utils::values(*sym_index_map);