-
Notifications
You must be signed in to change notification settings - Fork 0
17 ayonapiget does not check for failed httplib connections #35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
97f77cf
1299ab5
303a4a2
74e5261
203e426
be9d172
db95398
ab5a26c
48f4d69
6bf6bc6
46b551a
506f69e
60ab691
a7eb204
ed6515f
0bcf40c
c298526
301c26f
193c6ef
db6521a
c249415
4a0f60d
f25eb33
3f2730f
dd5c22a
f219106
6285410
a9c1ea1
47d2162
7eca99b
12b8fce
0a14d99
9d047fc
0132dde
e16655d
871ed1b
92c0eef
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,3 +20,4 @@ test_logs/ | |
AyonCppApi_CiCd/ | ||
AyonCppApi_cicd/ | ||
__pycache__ | ||
.env* |
Large diffs are not rendered by default.
+0 −48 | src/lib/ynput/core/utils/GetAppDataFoulder.h | |
+0 −22 | src/lib/ynput/core/utils/GetAyonLocal.hpp | |
+2 −2 | src/lib/ynput/lib/logging/AyonLogger.hpp |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,57 +32,202 @@ | |
#include "backward.hpp" | ||
#include "perfPrinter.h" | ||
|
||
// TODO implement the better Crash hanlder | ||
#ifdef _WIN32 | ||
#include <windows.h> | ||
#include <wincrypt.h> | ||
#endif | ||
|
||
// TODO implement the better Crash handler | ||
backward::StackTrace st; | ||
|
||
AyonApi::AyonApi(const std::string &logFilePos, | ||
|
||
// ------------------------------------------------ | ||
// helper functions for getting the ca cert path | ||
// ------------------------------------------------ | ||
std::string parseOutput(std::string& output) { | ||
// Parse the output to extract the directory path | ||
std::string::size_type start = output.find('"'); | ||
std::string::size_type end = output.find('"', start + 1); | ||
if (start != std::string::npos && end != std::string::npos) { | ||
return output.substr(start + 1, end - start - 1); | ||
} else { | ||
throw std::runtime_error("Failed to parse OpenSSL directory from command output."); | ||
} | ||
} | ||
|
||
std::string getOpenSSLDirByCLI() { | ||
std::array<char, 128> buffer; | ||
std::string result; | ||
auto pipeDeleter = [](FILE* pipe) { | ||
#ifdef _WIN32 | ||
_pclose(pipe); | ||
#else | ||
pclose(pipe); | ||
#endif | ||
}; | ||
std::unique_ptr<FILE, decltype(pipeDeleter)> pipe( | ||
#ifdef _WIN32 | ||
_popen("openssl version -d", "r"), | ||
#else | ||
popen("openssl version -d", "r"), | ||
#endif | ||
pipeDeleter | ||
); | ||
if (!pipe) { | ||
throw std::runtime_error("popen() failed!"); | ||
} | ||
while (fgets(buffer.data(), static_cast<int>(buffer.size()), pipe.get()) != nullptr) { | ||
result += buffer.data(); | ||
} | ||
|
||
return parseOutput(result); | ||
} | ||
|
||
|
||
std::string getOpenSSLDir() { | ||
#if OPENSSL_VERSION_NUMBER >= 0x10100000L // OpenSSL 1.1.0+ | ||
const char* sslVersion = OpenSSL_version(OPENSSL_DIR); | ||
std::string sslVersionStr(sslVersion); | ||
return parseOutput(sslVersionStr); | ||
#else // OpenSSL 1.0.x | ||
return parseOutput(SSLeay_version(SSLEAY_DIR)); | ||
#endif | ||
} | ||
// ------------------------------------------------ | ||
|
||
|
||
AyonApi::AyonApi(const std::optional<std::string> &logFilePos, | ||
const std::string &authKey, | ||
const std::string &serverUrl, | ||
const std::string &ayonProjectName, | ||
const std::string &siteId, | ||
std::optional<int> concurrency): | ||
m_num_threads(concurrency.value_or(std::max(int(std::thread::hardware_concurrency() / 2), 1))), | ||
m_authKey(authKey), | ||
m_serverUrl(serverUrl), | ||
m_ayonProjectName(ayonProjectName), | ||
m_siteId(siteId) { | ||
std::optional<int> concurrency) | ||
: m_num_threads(concurrency.value_or(std::max(int(std::thread::hardware_concurrency() / 2), 1))), | ||
m_authKey(authKey), | ||
m_serverUrl(serverUrl), | ||
m_ayonProjectName(ayonProjectName), | ||
m_siteId(siteId) { | ||
PerfTimer("AyonApi::AyonApi"); | ||
|
||
// ----------- Init m_Logger | ||
std::filesystem::path logFileName = "logFile.json"; | ||
std::filesystem::path basePath = logFilePos; | ||
std::filesystem::path logFilePath = std::filesystem::absolute(basePath) / logFileName; | ||
std::filesystem::path logPath; | ||
if (logFilePos.has_value()) { | ||
std::filesystem::path inPath(logFilePos.value()); | ||
|
||
if (inPath.is_relative()) { | ||
logPath = std::filesystem::weakly_canonical(inPath); | ||
} | ||
if (!inPath.has_parent_path()) { | ||
// if the input path is just an filename we will just throw it into tmp | ||
logPath = std::filesystem::temp_directory_path() / inPath; | ||
} | ||
// we allways want the data to be a json, so we just enforce it. | ||
logPath.replace_extension(".json"); | ||
|
||
if (std::filesystem::exists(logFilePath)) { | ||
logFilePath = std::filesystem::canonical(logFilePath); | ||
} | ||
else { | ||
std::filesystem::create_directories(logFilePath.parent_path()); | ||
} | ||
|
||
m_Log = std::make_shared<AyonLogger>(AyonLogger::getInstance(logFilePath.string())); | ||
if (std::filesystem::exists(logPath)) { | ||
logPath = std::filesystem::canonical(logPath); | ||
} | ||
else { | ||
std::filesystem::create_directories(logPath.parent_path()); | ||
} | ||
} | ||
m_Log = std::make_shared<AyonLogger>(AyonLogger::getInstance(logPath.string())); | ||
m_Log->LogLevlWarn(); | ||
|
||
m_Log->info(m_Log->key("AyonApi"), "Init AyonServer httplib::Client"); | ||
m_AyonServer = std::make_unique<httplib::Client>(m_serverUrl); | ||
m_AyonServer->set_bearer_token_auth(m_authKey); | ||
|
||
if (isSSL()) { | ||
m_headers = { | ||
{"X-Api-Key", m_authKey}, | ||
}; | ||
|
||
try { | ||
std::string opensslDirCLI = getOpenSSLDirByCLI(); | ||
|
||
#ifdef _WIN32 | ||
std::string certFileCLI = opensslDirCLI + "\\cert.pem"; | ||
#else | ||
std::string certFileCLI = opensslDirCLI + "/cert.pem"; | ||
#endif | ||
|
||
if (std::filesystem::exists(certFileCLI)) { | ||
m_Log->info("Using cert based on CLI var."); | ||
m_AyonServer->set_ca_cert_path(certFileCLI.c_str()); | ||
} else { | ||
std::string opensslDir = getOpenSSLDir(); | ||
#ifdef _WIN32 | ||
std::string certFile = opensslDir + "\\cert.pem"; | ||
#else | ||
std::string certFile = opensslDir + "/cert.pem"; | ||
#endif | ||
|
||
if (std::filesystem::exists(certFile)) { | ||
m_Log->info("Using cert based on SSLEAY_DIR."); | ||
m_AyonServer->set_ca_cert_path(certFile.c_str()); | ||
} else { | ||
const char* envCertFile = getenv("SSL_CERT_FILE"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe the environment variable should be the first option making use of an early return or just avoiding the other code. I would argue that if you define a cert file explicitly, it should be the one to use if possible. PS;: not saying the fallback idea is bad or anything still a great idea. |
||
if (envCertFile) { | ||
m_Log->info("Using cert based on env variable (SSL_CERT_PATH)."); | ||
m_AyonServer->set_ca_cert_path(envCertFile); | ||
} else { | ||
m_Log->info("Failed to determine the OpenSSL directory. Falling back to the default certificate file path."); | ||
std::string certPath = ( | ||
std::filesystem::path(__FILE__).parent_path().parent_path().parent_path() / "certs" / "cacert.pem" | ||
).string(); | ||
m_AyonServer->set_ca_cert_path(certPath); | ||
} | ||
} | ||
} | ||
} catch (const std::exception &e) { | ||
m_Log->error("Failed to get OpenSSL directory: {}", e.what()); | ||
m_AyonServer->set_ca_cert_path(nullptr); | ||
} | ||
|
||
m_AyonServer->enable_server_certificate_verification(true); | ||
} else { | ||
m_AyonServer->set_bearer_token_auth(m_authKey); | ||
m_headers = {}; | ||
} | ||
|
||
auto res = m_AyonServer->Get("/api/info", m_headers); | ||
if (!res) { | ||
m_Log->error("Failed to connect to the Ayon server."); | ||
} else if (res->status != 200) { | ||
m_Log->warn("Connected to the Ayon server : {}", res->status); | ||
} else { | ||
m_Log->info("Connected to the Ayon server : {}", res->status); | ||
} | ||
|
||
m_Log->info(m_Log->key("AyonApi"), "Constructor Getting Site Roots"); | ||
getSiteRoots(); | ||
}; | ||
} | ||
|
||
AyonApi::~AyonApi() { | ||
m_Log->info(m_Log->key("AyonApi"), "AyonApi::~AyonApi()"); | ||
}; | ||
|
||
std::unordered_map<std::string, std::string>* | ||
AyonApi::getSiteRoots() { | ||
m_Log->info(m_Log->key("AyonApi"), "AyonApi::getSiteRoots()"); | ||
if (m_siteRoots.size() < 1) { | ||
httplib::Headers headers = {{"X-ayon-site-id", m_siteId}}; | ||
nlohmann::json response | ||
= GET(std::make_shared<std::string>("/api/projects/" + m_ayonProjectName + "/siteRoots"), | ||
std::make_shared<httplib::Headers>(headers), 200); | ||
m_siteRoots = response; | ||
if (m_siteRoots.size() < 1) { | ||
std::string platform; | ||
#ifdef _WIN32 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might be best to make this a macro and let the preprocessing place it in the right places. This would allow the compiler to collapse all the string stuff into a singe object. Also if you make it a macro you can just use it anywhere. |
||
platform = "windows"; | ||
#elif __linux__ | ||
platform = "linux"; | ||
#endif | ||
nlohmann::json response = GET(std::make_shared<std::string>("/api/projects/" + m_ayonProjectName + "/siteRoots?platform=" + platform), | ||
std::make_shared<httplib::Headers>(m_headers), 200); | ||
|
||
if (response.empty()) { | ||
m_Log->error("AyonApi::getSiteRoots response is empty"); | ||
return &m_siteRoots; | ||
} else { | ||
m_siteRoots = response; | ||
} | ||
|
||
} | ||
if (m_Log->isKeyActive(m_Log->key("AyonApi"))) { | ||
m_Log->info(m_Log->key("AyonApi"), "found site Roots: "); | ||
|
@@ -116,7 +261,7 @@ AyonApi::rootReplace(const std::string &rootLessPath) { | |
return rootedPath; | ||
} | ||
catch (std::out_of_range &e) { | ||
m_Log->warn("AyonApi::rootedPath error acured {}, list off available root replace str: "); | ||
m_Log->warn("AyonApi::rootedPath error acured {}, list off available root replace str: ", e.what()); | ||
for (auto &g: m_siteRoots) { | ||
m_Log->warn("Key: {}, replacement: {}", g.first, g.second); | ||
} | ||
|
@@ -141,6 +286,12 @@ AyonApi::GET(const std::shared_ptr<std::string> endPoint, | |
while (retryes <= m_maxCallRetrys) { | ||
try { | ||
response = m_AyonServer->Get(*endPoint, *headers); | ||
|
||
if (response == nullptr) { | ||
m_Log->warn("AyonApi::GET response is null: {}", httplib::to_string(response.error())); | ||
return nlohmann::json(); | ||
} | ||
|
||
responeStatus = response->status; | ||
retryes++; | ||
|
||
|
@@ -162,7 +313,7 @@ AyonApi::GET(const std::shared_ptr<std::string> endPoint, | |
} | ||
} // TODO error reason not printed | ||
catch (const httplib::Error &e) { | ||
m_Log->warn("Request Failed because: {}"); | ||
m_Log->warn("Request Failed because: {}", httplib::to_string(e)); | ||
break; | ||
} | ||
m_Log->warn("The connection failed Rety now."); | ||
|
@@ -279,9 +430,9 @@ AyonApi::batchResolvePath(std::vector<std::string> &uriPaths) { | |
{ | ||
PerfTimer("AyonApi::batchResolvePath::sanatizeVector"); | ||
std::set<std::string> s; | ||
unsigned size = uriPaths.size(); | ||
size_t size = uriPaths.size(); | ||
|
||
for (unsigned i = 0; i < size; ++i) s.insert(uriPaths[i]); | ||
for (size_t i = 0; i < size; ++i) s.insert(uriPaths[i]); | ||
uriPaths.assign(s.begin(), s.end()); | ||
m_Log->info("Make sure that the vector has no duplicates. vecSize before: {} after: {}", size, | ||
uriPaths.size()); | ||
|
@@ -587,3 +738,8 @@ AyonApi::logPointer() { | |
m_Log->info(m_Log->key("AyonApi"), "AyonApi::logPointer()"); | ||
return m_Log; | ||
}; | ||
|
||
bool | ||
AyonApi::isSSL() const { | ||
return m_serverUrl.rfind("https://", 0) == 0; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doing this as a compare of a substring might be faster. |
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe it might be best to have everything in the try in a separate function. Would make expanding the business logic a bit simpler.
You can just make it an inlined function if you are afraid of performance or any behavior changes.