Skip to content

Commit

Permalink
Adapt Raise to use format string as message directly if no args are g…
Browse files Browse the repository at this point in the history
…iven (#1606)

Fixes man-group/arcticdb-man#111

`raise<ErrorCode>()` function takes format string as the first argument.
In some places this has been used incorrectly which leads errors like
man-group/arcticdb-man#111.
- Adapt Raise to use format string as message directly if no args are
given
- To test this, pass `sym{}` as symbol name
- Show symbol name as part of error message in mock clients. This will
test whether `sym{}` is handled correctly.
  • Loading branch information
muhammadhamzasajjad authored Jun 21, 2024
1 parent 875bb7b commit 2a92cfe
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 10 deletions.
4 changes: 2 additions & 2 deletions cpp/arcticdb/storage/azure/azure_mock_client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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&) {
Expand Down
3 changes: 2 additions & 1 deletion cpp/arcticdb/storage/s3/s3_mock_client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
6 changes: 3 additions & 3 deletions cpp/arcticdb/storage/test/test_storage_exceptions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);
}
Expand Down
20 changes: 16 additions & 4 deletions cpp/arcticdb/util/preconditions.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,29 @@ struct Raise {

template<typename...Args>
[[noreturn]] void operator()(fmt::format_string<Args...> format, Args&&...args) const {
std::string combo_format = fmt::format(FMT_COMPILE("{} {}"), error_code_data<code>.name_, format);
std::string msg = fmt::format(fmt::runtime(combo_format), std::forward<Args>(args)...);
std::string msg;
if constexpr(sizeof...(args) == 0) {
msg = fmt::format(FMT_COMPILE("{} {}"), error_code_data<code>.name_, format);
}
else {
std::string combo_format = fmt::format(FMT_COMPILE("{} {}"), error_code_data<code>.name_, format);
msg = fmt::format(fmt::runtime(combo_format), std::forward<Args>(args)...);
}
if constexpr(error_category == ErrorCategory::INTERNAL)
log::root().error(msg);
throw_error<code>(msg);
}

template<typename FormatString, typename...Args>
[[noreturn]] void operator()(FormatString format, Args&&...args) const {
std::string combo_format = fmt::format(FMT_COMPILE("{} {}"), error_code_data<code>.name_, format);
std::string msg = fmt::format(fmt::runtime(combo_format), std::forward<Args>(args)...);
std::string msg;
if constexpr(sizeof...(args) == 0) {
msg = fmt::format(FMT_COMPILE("{} {}"), error_code_data<code>.name_, format);
}
else {
std::string combo_format = fmt::format(FMT_COMPILE("{} {}"), error_code_data<code>.name_, format);
msg = fmt::format(fmt::runtime(combo_format), std::forward<Args>(args)...);
}
if constexpr(error_category == ErrorCategory::INTERNAL)
log::root().error(msg);
throw_error<code>(msg);
Expand Down

0 comments on commit 2a92cfe

Please sign in to comment.