refactor: cleanups, comments, and rename manager to session

northwesternfintech · Sep 6, 2023 · edf9c08 · edf9c08
1 parent 3a4fbf2
commit edf9c08
Show file tree

Hide file tree

Showing 5 changed files with 119 additions and 70 deletions.
diff --git a/src/web/connections/base.cpp b/src/web/connections/base.cpp
@@ -12,8 +12,10 @@ Connection::Connection(const std::string& url, CURL* curl_handle) :
     curl_error_buffer_(CURL_ERROR_SIZE, '\0'),
     url_(utils::normalize_url(url))
 {
+    // Populate backtrace
     log_bt(web, "Initialize conn object and curl handle for {}", url_);
 
+    // Check that curl handle is valid
     if (!curl_handle_) [[unlikely]] {
         throw std::runtime_error(
             "Could not create cURL handle, or null handle provided."
@@ -89,6 +91,7 @@ Connection::debug_log_callback_(
     }
 
     if (logging::get_libcurl_logger()->should_log<quill::LogLevel::TraceL3>())
+        [[unlikely]]
         log_t3(libcurl, "Hexdump\n{}", utils::hexdump(raw_data, size));
 
     return 0;

diff --git a/src/web/connections/ws.cpp b/src/web/connections/ws.cpp
@@ -26,32 +26,41 @@ WebSocketConnection::write_callback_(
 
     // Get our frame
     const auto* frame = curl_ws_meta(conn->curl_handle());
-    assert(frame);
 
-    // Log information
-    log_t1(
+    if (!frame) [[unlikely]] {
+        log_c(
+            web,
+            "In WebSocket write callback for {} but got NULL frame, aborting!",
+            conn->url()
+        );
+        abort();
+    }
+
+    // Populate backtrace
+    log_bt(
         web,
-        "WebSocket data callback ran for {} ({} bytes, {} left)",
+        "WS write callback for url {} with buf of len {} ({}x{}), {} bytes left",
         conn->url(),
         size,
+        elem_size,
+        length,
         frame->bytesleft
     );
 
-    if (logging::get_web_logger()->should_log<quill::LogLevel::TraceL3>())
-        log_t3(web, "Data hexdump\n{}", utils::hexdump(buf, length));
-
-    log_bt(
+    // Log information
+    log_t1(
         web,
-        "WS write callback for url {} with buf of len {} ({}x{}), {} bytes left",
+        "WebSocket data callback ran for {} ({} bytes, {} left)",
         conn->url(),
         size,
-        elem_size,
-        length,
         frame->bytesleft
     );
 
+    if (logging::get_web_logger()->should_log<quill::LogLevel::TraceL3>()) [[unlikely]]
+        log_t3(web, "Data hexdump\n{}", utils::hexdump(buf, length));
+
     // Make sure this connection isn't closed
-    if (!conn->open()) {
+    if (!conn->open()) [[unlikely]] {
         log_w(web, "Write callback for {} called after close().", conn->url());
 
         return size; // piped bytes to /dev/null
@@ -61,7 +70,7 @@ WebSocketConnection::write_callback_(
     // NOLINTNEXTLINE(*-pointer-arithmetic)
     conn->write_buf_.insert(conn->write_buf_.end(), buf, buf + size);
 
-    if (frame->bytesleft == 0) { // got all data in frame
+    if (frame->bytesleft == 0) [[likely]] { // got all data in frame; most are short
         // log some data
         log_bt(web, "Entering user data callback for {}", conn->url());
         log_d(
@@ -94,7 +103,8 @@ WebSocketConnection::start_()
     assert(ready());
     assert(!open());
 
-    log_d(web, "Starting web socket connection to {}", url());
+    // Populate backtrace
+    log_bt(web, "Setting up WS connection to {}", url());
 
     // Clear error buffer
     clear_error_buffer_();
@@ -107,18 +117,16 @@ WebSocketConnection::start_()
     curl_easy_setopt(curl_handle(), CURLOPT_WRITEDATA, this);
 
     // Mark the websocket as open
-    log_bt(web, "Set up WS connection to {}", url());
     open() = true;
+    log_d(web, "Set up web socket connection to {}", url());
 }
 
 size_t
 WebSocketConnection::close(WebSocketCloseStatus status, std::vector<uint8_t> data)
 {
     assert(ready());
 
-    log_i(web, "Closing WebSocket connection to {} with code {}", url(), status);
-    log_t2(web, "Data: {}", data);
-
+    // Backtrace log
     log_bt(
         web,
         "Send WS close message to {} with status {} and data {}",
@@ -127,6 +135,17 @@ WebSocketConnection::close(WebSocketCloseStatus status, std::vector<uint8_t> dat
         data
     );
 
+    // Log about arguments
+    log_i(web, "Closing WebSocket connection to {} with code {}", url(), status);
+    log_d(web, "{} bytes of data provided", data.size());
+
+    if ( //
+        !data.empty()
+        && logging::get_web_logger()->should_log<quill::LogLevel::TraceL2>()
+    ) [[unlikely]] {
+        log_t2(web, "Data hexdump\n{}", utils::hexdump(data));
+    }
+
     // Make sure we're not already closed
     if (!open()) {
         log_w(
@@ -167,11 +186,7 @@ WebSocketConnection::send(std::vector<uint8_t> data, unsigned flags)
 {
     assert(ready());
 
-    log_t1(web, "Sending {} bytes to {} with flags {:#b}", data.size(), url(), flags);
-
-    if (logging::get_web_logger()->should_log<quill::LogLevel::TraceL3>())
-        log_t3(web, "Hexdump\n{}", utils::hexdump(data.data(), data.size()));
-
+    // Populate backtrace
     log_bt(
         web,
         "Websocket send to {} with flags {:#b} and {} bytes: {}",
@@ -181,6 +196,12 @@ WebSocketConnection::send(std::vector<uint8_t> data, unsigned flags)
         data
     );
 
+    // Log about arguments
+    log_t1(web, "Sending {} bytes to {} with flags {:#b}", data.size(), url(), flags);
+
+    if (logging::get_web_logger()->should_log<quill::LogLevel::TraceL3>()) [[unlikely]]
+        log_t3(web, "Data hexdump\n{}", utils::hexdump(data.data(), data.size()));
+
     // Clear error buffer
     clear_error_buffer_();
 

diff --git a/src/web/connections/ws.hpp b/src/web/connections/ws.hpp
@@ -108,7 +108,7 @@ class WebSocketConnection : public Connection {
     /**
      * Create a new websocket connection.
      *
-     * Should only be called by the RequestManager.
+     * Should only be called by the Session.
      */
     WebSocketConnection(const std::string& url, callback on_data) :
         Connection(url), on_data_(std::move(on_data))