From 98b4362f2dd6553ac81808f40b00eaaf25be97af Mon Sep 17 00:00:00 2001
From: Sergey Yakubov <sergey.yakubov@desy.de>
Date: Tue, 13 Feb 2018 13:51:38 +0100
Subject: [PATCH] more work on Error
---
broker/src/hidra2_broker/database/mongodb.go | 2 +-
common/cpp/CMakeLists.txt | 1 -
common/cpp/include/common/error.h | 26 ++++++++++++++++---
common/cpp/include/system_wrappers/io.h | 2 +-
common/cpp/src/system_io/system_io.cpp | 2 ++
common/cpp/src/system_io/system_io_linux.cpp | 1 +
.../cpp/src/system_io/system_io_windows.cpp | 1 +
.../worker/getnext_broker/getnext_broker.cpp | 11 +++++++-
.../worker/process_folder/process_folder.cpp | 6 ++---
tests/system_io/CMakeLists.txt | 2 +-
.../read_file_content/CMakeLists.txt | 4 +--
.../CMakeLists.txt | 4 +--
.../cleanup_linux.sh | 0
.../cleanup_windows.bat | 0
.../read_folder_content.cpp | 0
.../setup_linux.sh | 0
.../setup_windows.bat | 0
worker/api/cpp/CMakeLists.txt | 2 +-
worker/api/cpp/src/folder_data_broker.cpp | 2 +-
worker/api/cpp/src/http_client.cpp | 2 +-
worker/api/cpp/src/http_error.h | 2 +-
worker/api/cpp/src/server_data_broker.cpp | 1 +
.../api/cpp/unittests/test_folder_broker.cpp | 2 ++
.../api/cpp/unittests/test_server_broker.cpp | 17 +++++++++++-
worker/tools/folder_to_db/src/main.cpp | 2 +-
25 files changed, 71 insertions(+), 21 deletions(-)
rename tests/system_io/{read_files_in_folder => read_folder_content}/CMakeLists.txt (92%)
rename tests/system_io/{read_files_in_folder => read_folder_content}/cleanup_linux.sh (100%)
rename tests/system_io/{read_files_in_folder => read_folder_content}/cleanup_windows.bat (100%)
rename tests/system_io/{read_files_in_folder => read_folder_content}/read_folder_content.cpp (100%)
rename tests/system_io/{read_files_in_folder => read_folder_content}/setup_linux.sh (100%)
rename tests/system_io/{read_files_in_folder => read_folder_content}/setup_windows.bat (100%)
diff --git a/broker/src/hidra2_broker/database/mongodb.go b/broker/src/hidra2_broker/database/mongodb.go
index 4c76cb3f9..c6e54099e 100644
--- a/broker/src/hidra2_broker/database/mongodb.go
+++ b/broker/src/hidra2_broker/database/mongodb.go
@@ -54,7 +54,7 @@ func (db *Mongodb) dataBaseExist(dbname string) (err error) {
}
if !db.databaseInList(dbname) {
- return errors.New("database not found: " + dbname)
+ return errors.New("dataset not found: " + dbname)
}
return nil
diff --git a/common/cpp/CMakeLists.txt b/common/cpp/CMakeLists.txt
index 288ffe42c..7865db18d 100644
--- a/common/cpp/CMakeLists.txt
+++ b/common/cpp/CMakeLists.txt
@@ -2,7 +2,6 @@ add_subdirectory(src/system_io)
add_subdirectory(src/data_structs)
-
if(BUILD_MONGODB_CLIENTLIB)
add_subdirectory(src/database)
endif()
diff --git a/common/cpp/include/common/error.h b/common/cpp/include/common/error.h
index 080ece577..8ef74d4f5 100644
--- a/common/cpp/include/common/error.h
+++ b/common/cpp/include/common/error.h
@@ -6,10 +6,17 @@
namespace hidra2 {
+enum class ErrorType {
+ kHidraError,
+ kEOF,
+ kHttpError
+};
+
class ErrorInterface {
public:
virtual std::string Explain() const noexcept = 0;
- virtual void Set(const std::string& error) noexcept = 0;
+ virtual void Append(const std::string& value) noexcept = 0;
+ virtual ErrorType GetErrorType() noexcept = 0;
virtual ~ErrorInterface() = default; // needed for unique_ptr to delete itself
};
@@ -19,15 +26,23 @@ using Error = std::unique_ptr<ErrorInterface>;
class SimpleError: public ErrorInterface {
private:
std::string error_;
+ ErrorType error_type_ = ErrorType::kHidraError;
public:
explicit SimpleError(const std::string& error): error_{error} {
}
+ SimpleError(const std::string& error, ErrorType error_type ): error_{error}, error_type_{error_type} {
+ }
+
+ void Append(const std::string& value) noexcept override {
+ error_ += ": " + value;
+ }
+
std::string Explain() const noexcept override {
return error_;
}
- void Set(const std::string& error)noexcept override {
- error_ = error;
+ ErrorType GetErrorType()noexcept override {
+ return error_type_;
}
};
@@ -36,5 +51,10 @@ inline Error TextError(const std::string& error) {
return Error{new SimpleError{error}};
}
+inline Error TextErrorWithType(const std::string& error, ErrorType error_type) {
+ return Error{new SimpleError{error, error_type}};
+}
+
+
}
#endif //HIDRA2_ERROR_H
diff --git a/common/cpp/include/system_wrappers/io.h b/common/cpp/include/system_wrappers/io.h
index 9b73e407a..3f955b229 100644
--- a/common/cpp/include/system_wrappers/io.h
+++ b/common/cpp/include/system_wrappers/io.h
@@ -13,7 +13,7 @@
namespace hidra2 {
namespace IOErrors {
-auto const kFileNotFound = "File not found";
+auto const kFileNotFound = "No such file or directory";
auto const kReadError = "Read error";
auto const kPermissionDenied = "Permission denied";
auto const kUnknownError = "Unknown error";
diff --git a/common/cpp/src/system_io/system_io.cpp b/common/cpp/src/system_io/system_io.cpp
index 0339443e2..42a9f8b5c 100644
--- a/common/cpp/src/system_io/system_io.cpp
+++ b/common/cpp/src/system_io/system_io.cpp
@@ -47,6 +47,7 @@ FileData SystemIO::GetDataFromFile(const std::string& fname, uint64_t fsize, Err
int fd = open(fname.c_str(), O_RDONLY);
*err = IOErrorFromErrno();
if (*err != nullptr) {
+ (*err)->Append(fname);
return nullptr;
}
uint8_t* data_array = nullptr;
@@ -61,6 +62,7 @@ FileData SystemIO::GetDataFromFile(const std::string& fname, uint64_t fsize, Err
FileData data{data_array};
if (*err != nullptr) {
close(fd);
+ (*err)->Append(fname);
return nullptr;
}
errno = 0;
diff --git a/common/cpp/src/system_io/system_io_linux.cpp b/common/cpp/src/system_io/system_io_linux.cpp
index abee2bf59..36adc6d3d 100644
--- a/common/cpp/src/system_io/system_io_linux.cpp
+++ b/common/cpp/src/system_io/system_io_linux.cpp
@@ -94,6 +94,7 @@ void SystemIO::CollectFileInformationRecursivly(const std::string& path,
auto dir = opendir((path).c_str());
if (dir == nullptr) {
*err = IOErrorFromErrno();
+ (*err)->Append(path);
return;
}
while (true) {
diff --git a/common/cpp/src/system_io/system_io_windows.cpp b/common/cpp/src/system_io/system_io_windows.cpp
index d4816ef18..1a976f8a4 100644
--- a/common/cpp/src/system_io/system_io_windows.cpp
+++ b/common/cpp/src/system_io/system_io_windows.cpp
@@ -108,6 +108,7 @@ void SystemIO::CollectFileInformationRecursivly(const std::string& path,
HANDLE handle = FindFirstFile((path + "\\*.*").c_str(), &find_data);
if (handle == INVALID_HANDLE_VALUE) {
*err = IOErrorFromGetLastError();
+ (*err)->Append(path);
return;
}
diff --git a/examples/worker/getnext_broker/getnext_broker.cpp b/examples/worker/getnext_broker/getnext_broker.cpp
index 20237acb8..0f578333b 100644
--- a/examples/worker/getnext_broker/getnext_broker.cpp
+++ b/examples/worker/getnext_broker/getnext_broker.cpp
@@ -18,15 +18,24 @@ void WaitThreads(std::vector<std::thread>* threads) {
}
}
+void ProcessError(const Error& err) {
+ if (err == nullptr) return;
+ if (err->GetErrorType() != hidra2::ErrorType::kEOF) {
+ std::cout << err->Explain() << std::endl;
+ exit(EXIT_FAILURE);
+ }
+}
+
std::vector<std::thread> StartThreads(const std::string& server, const std::string& run_name, int nthreads,
std::vector<int>* nfiles) {
auto exec_next = [server, run_name, nfiles](int i) {
hidra2::FileInfo fi;
Error err;
auto broker = hidra2::DataBrokerFactory::CreateServerBroker(server, run_name, &err);
- while (broker->GetNext(&fi, nullptr) == nullptr) {
+ while ((err = broker->GetNext(&fi, nullptr)) == nullptr) {
(*nfiles)[i] ++;
}
+ ProcessError(err);
};
std::vector<std::thread> threads;
diff --git a/examples/worker/process_folder/process_folder.cpp b/examples/worker/process_folder/process_folder.cpp
index da04b67c8..fd789d2fd 100644
--- a/examples/worker/process_folder/process_folder.cpp
+++ b/examples/worker/process_folder/process_folder.cpp
@@ -41,7 +41,7 @@ void ConnectToBrocker(std::unique_ptr<hidra2::DataBroker>* broker, Statistics* s
high_resolution_clock::time_point t1 = high_resolution_clock::now();
Error err = (*broker)->Connect();
if (err != nullptr) {
- std::cout << "Cannot connect to broker" << std::endl;
+ std::cout << err->Explain() << std::endl;
exit(EXIT_FAILURE);
}
high_resolution_clock::time_point t2 = high_resolution_clock::now();
@@ -60,8 +60,8 @@ void ReadAllData(std::unique_ptr<hidra2::DataBroker>* broker, Statistics* statis
nfiles++;
size += file_info.size;
}
- if (err->Explain() != hidra2::WorkerErrorMessage::kNoData) {
- std::cout << "Read error" << std::endl;
+ if (err->GetErrorType() != hidra2::ErrorType::kEOF) {
+ std::cout << err->Explain() << std::endl;
exit(EXIT_FAILURE);
}
diff --git a/tests/system_io/CMakeLists.txt b/tests/system_io/CMakeLists.txt
index 595503fce..ca5d8cd86 100644
--- a/tests/system_io/CMakeLists.txt
+++ b/tests/system_io/CMakeLists.txt
@@ -1,5 +1,5 @@
CMAKE_MINIMUM_REQUIRED(VERSION 3.7) # needed for fixtures
-add_subdirectory(read_files_in_folder)
+add_subdirectory(read_folder_content)
add_subdirectory(read_file_content)
diff --git a/tests/system_io/read_file_content/CMakeLists.txt b/tests/system_io/read_file_content/CMakeLists.txt
index 8109443ef..253fc7d58 100644
--- a/tests/system_io/read_file_content/CMakeLists.txt
+++ b/tests/system_io/read_file_content/CMakeLists.txt
@@ -16,6 +16,6 @@ set_target_properties(${TARGET_NAME} PROPERTIES LINKER_LANGUAGE CXX)
add_test_setup_cleanup(${TARGET_NAME})
add_integration_test(${TARGET_NAME} readfile "test/1 123")
-add_integration_test(${TARGET_NAME} filenotfound "test_notexist notfound")
-add_integration_test(${TARGET_NAME} filenoaccess "file_noaccess Permissiondenied")
+add_integration_test(${TARGET_NAME} filenotfound "test_notexist Nosuchfileordirectory:test_notexist")
+add_integration_test(${TARGET_NAME} filenoaccess "file_noaccess Permissiondenied:file_noaccess")
diff --git a/tests/system_io/read_files_in_folder/CMakeLists.txt b/tests/system_io/read_folder_content/CMakeLists.txt
similarity index 92%
rename from tests/system_io/read_files_in_folder/CMakeLists.txt
rename to tests/system_io/read_folder_content/CMakeLists.txt
index a0065e1f8..abad5b81f 100644
--- a/tests/system_io/read_files_in_folder/CMakeLists.txt
+++ b/tests/system_io/read_folder_content/CMakeLists.txt
@@ -22,6 +22,6 @@ ELSE()
ENDIF(WIN32)
-add_integration_test(${TARGET_NAME} foldernotfound "test_notexist notfound")
-add_integration_test(${TARGET_NAME} foldernoaccess "test_noaccess1 Permissiondenied")
+add_integration_test(${TARGET_NAME} foldernotfound "test_notexist Nosuchfileordirectory:test_notexist")
+add_integration_test(${TARGET_NAME} foldernoaccess "test_noaccess1 Permissiondenied:test_noaccess1")
diff --git a/tests/system_io/read_files_in_folder/cleanup_linux.sh b/tests/system_io/read_folder_content/cleanup_linux.sh
similarity index 100%
rename from tests/system_io/read_files_in_folder/cleanup_linux.sh
rename to tests/system_io/read_folder_content/cleanup_linux.sh
diff --git a/tests/system_io/read_files_in_folder/cleanup_windows.bat b/tests/system_io/read_folder_content/cleanup_windows.bat
similarity index 100%
rename from tests/system_io/read_files_in_folder/cleanup_windows.bat
rename to tests/system_io/read_folder_content/cleanup_windows.bat
diff --git a/tests/system_io/read_files_in_folder/read_folder_content.cpp b/tests/system_io/read_folder_content/read_folder_content.cpp
similarity index 100%
rename from tests/system_io/read_files_in_folder/read_folder_content.cpp
rename to tests/system_io/read_folder_content/read_folder_content.cpp
diff --git a/tests/system_io/read_files_in_folder/setup_linux.sh b/tests/system_io/read_folder_content/setup_linux.sh
similarity index 100%
rename from tests/system_io/read_files_in_folder/setup_linux.sh
rename to tests/system_io/read_folder_content/setup_linux.sh
diff --git a/tests/system_io/read_files_in_folder/setup_windows.bat b/tests/system_io/read_folder_content/setup_windows.bat
similarity index 100%
rename from tests/system_io/read_files_in_folder/setup_windows.bat
rename to tests/system_io/read_folder_content/setup_windows.bat
diff --git a/worker/api/cpp/CMakeLists.txt b/worker/api/cpp/CMakeLists.txt
index 60aeac037..49bcfa605 100644
--- a/worker/api/cpp/CMakeLists.txt
+++ b/worker/api/cpp/CMakeLists.txt
@@ -13,7 +13,7 @@ set(SOURCE_FILES
# Library
################################
add_library(${TARGET_NAME} STATIC ${SOURCE_FILES} $<TARGET_OBJECTS:system_io>
- $<TARGET_OBJECTS:data_structs>)
+ $<TARGET_OBJECTS:data_structs>)
set (CMAKE_PREFIX_PATH "${LIBCURL_DIR}")
find_package (CURL REQUIRED)
diff --git a/worker/api/cpp/src/folder_data_broker.cpp b/worker/api/cpp/src/folder_data_broker.cpp
index 11e44d963..f31253646 100644
--- a/worker/api/cpp/src/folder_data_broker.cpp
+++ b/worker/api/cpp/src/folder_data_broker.cpp
@@ -37,7 +37,7 @@ Error FolderDataBroker::CanGetData(FileInfo* info, FileData* data, int nfile) co
}
if (nfile >= (int) filelist_.size()) {
- return Error{TextError(WorkerErrorMessage::kNoData)};
+ return Error{TextErrorWithType(WorkerErrorMessage::kNoData, ErrorType::kEOF)};
}
return nullptr;
}
diff --git a/worker/api/cpp/src/http_client.cpp b/worker/api/cpp/src/http_client.cpp
index 7b9c98368..acfec717c 100644
--- a/worker/api/cpp/src/http_client.cpp
+++ b/worker/api/cpp/src/http_client.cpp
@@ -15,7 +15,7 @@ Error HttpCodeToWorkerError(const HttpCode& code) {
break;
case HttpCode::NoContent:
message = WorkerErrorMessage::kNoData;
- break;
+ return TextErrorWithType(message, ErrorType::kEOF);
case HttpCode::NotFound:
message = WorkerErrorMessage::kSourceNotFound;
break;
diff --git a/worker/api/cpp/src/http_error.h b/worker/api/cpp/src/http_error.h
index 2c3f5f362..8ac563f57 100644
--- a/worker/api/cpp/src/http_error.h
+++ b/worker/api/cpp/src/http_error.h
@@ -8,7 +8,7 @@ namespace hidra2 {
class HttpError: public SimpleError {
public:
- HttpError(const std::string& error, HttpCode http_code): SimpleError{error}, http_code_{http_code} {
+ HttpError(const std::string& error, HttpCode http_code): SimpleError{error, ErrorType::kHttpError}, http_code_{http_code} {
}
HttpCode GetCode() const {
return http_code_;
diff --git a/worker/api/cpp/src/server_data_broker.cpp b/worker/api/cpp/src/server_data_broker.cpp
index 8cd9c3efe..4712df342 100644
--- a/worker/api/cpp/src/server_data_broker.cpp
+++ b/worker/api/cpp/src/server_data_broker.cpp
@@ -26,6 +26,7 @@ Error ServerDataBroker::GetFileInfoFromServer(FileInfo* info, const std::string&
err = HttpCodeToWorkerError(code);
if (err != nullptr) {
+ err->Append(responce);
return err;
}
diff --git a/worker/api/cpp/unittests/test_folder_broker.cpp b/worker/api/cpp/unittests/test_folder_broker.cpp
index d3a673f43..de0ac6969 100644
--- a/worker/api/cpp/unittests/test_folder_broker.cpp
+++ b/worker/api/cpp/unittests/test_folder_broker.cpp
@@ -195,6 +195,8 @@ TEST_F(FolderDataBrokerTests, GetNextFromEmptyFolderReturnsError) {
FileInfo fi;
auto err = data_broker->GetNext(&fi, nullptr);
+ ASSERT_THAT(err->GetErrorType(), Eq(hidra2::ErrorType::kEOF));
+
ASSERT_THAT(err->Explain(), Eq(hidra2::WorkerErrorMessage::kNoData));
}
diff --git a/worker/api/cpp/unittests/test_server_broker.cpp b/worker/api/cpp/unittests/test_server_broker.cpp
index 4f0b6614f..c4bdc10a5 100644
--- a/worker/api/cpp/unittests/test_server_broker.cpp
+++ b/worker/api/cpp/unittests/test_server_broker.cpp
@@ -24,6 +24,7 @@ using hidra2::SimpleError;
using ::testing::AtLeast;
using ::testing::Eq;
+using ::testing::HasSubstr;
using ::testing::Ne;
using ::testing::Test;
using ::testing::_;
@@ -99,10 +100,24 @@ TEST_F(ServerDataBrokerTests, GetNextReturnsErrorFromHttpClient) {
auto err = data_broker->GetNext(&info, nullptr);
- ASSERT_THAT(err->Explain(), Eq(hidra2::WorkerErrorMessage::kSourceNotFound));
+ ASSERT_THAT(err->Explain(), HasSubstr(hidra2::WorkerErrorMessage::kSourceNotFound));
+ ASSERT_THAT(err->GetErrorType(), hidra2::ErrorType::kHttpError);
ASSERT_THAT(dynamic_cast<HttpError*>(err.get())->GetCode(), Eq(HttpCode::NotFound));
}
+TEST_F(ServerDataBrokerTests, GetNextReturnsEOFFromHttpClient) {
+ EXPECT_CALL(mock_http_client, Get_t(_, _, _)).WillOnce(DoAll(
+ SetArgPointee<1>(HttpCode::NoContent),
+ SetArgPointee<2>(nullptr),
+ Return("")));
+
+ auto err = data_broker->GetNext(&info, nullptr);
+
+ ASSERT_THAT(err->Explain(), HasSubstr(hidra2::WorkerErrorMessage::kNoData));
+ ASSERT_THAT(err->GetErrorType(), hidra2::ErrorType::kEOF);
+}
+
+
FileInfo CreateFI() {
FileInfo fi;
fi.size = 100;
diff --git a/worker/tools/folder_to_db/src/main.cpp b/worker/tools/folder_to_db/src/main.cpp
index 8e79fed17..c0440577a 100644
--- a/worker/tools/folder_to_db/src/main.cpp
+++ b/worker/tools/folder_to_db/src/main.cpp
@@ -84,7 +84,7 @@ int main(int argc, char* argv[]) {
auto err = importer.Convert(import_params.uri, import_params.folder, import_params.db_name,
&statistics);
if (err != nullptr) {
- std::cout << "Error import to database" << std::endl;
+ std::cout << "Error import to database: " << err->Explain() << std::endl;
return 1;
}
--
GitLab