[GitHub] [nifi-minifi-cpp] fgerlits commented on a diff in pull request #1314: MINIFICPP-1808 Improve RawSocketProtocol authorization error logging

[GitBox <gi...@apache.org>]

fgerlits commented on code in PR #1314:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1314#discussion_r859528263


##########
libminifi/src/sitetosite/RawSocketProtocol.cpp:
##########
@@ -310,31 +310,32 @@ bool RawSiteToSiteClient::handShake() {
     }
   }
 
-  std::string error;
+  auto logPortStateError = [this](const std::string& error) {
+    logger_->log_error("Site2Site HandShake Failed because destination port, %s, is %s", port_id_.to_string(), error);
+  };
 
   switch (code) {
     case PROPERTIES_OK:
       logger_->log_debug("Site2Site HandShake Completed");
       peer_state_ = HANDSHAKED;
       return true;
     case PORT_NOT_IN_VALID_STATE:
-      error = "in invalid state";
-      break;
+      logPortStateError("in invalid state");
+      return false;
     case UNKNOWN_PORT:
-      error = "an unknown port";
-      break;
+      logPortStateError("an unknown port");
+      return false;
     case PORTS_DESTINATION_FULL:
-      error = "full";
-      break;
+      logPortStateError("full");
+      return false;
+    case UNAUTHORIZED:
+      logger_->log_error("Site2Site HandShake failed: UNAUTHORIZED");
+      return false;
     // Unknown error
     default:
       logger_->log_error("HandShake Failed because of unknown respond code %d", code);
       return false;

Review Comment:
   these two could use `logPortStateError()`, too: knowing the port number could be useful in these cases, too
   
   also, I would remove the comment in line 334



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org