From 8efe0b2118c21a3e14ffca23bff79d536a0742f6 Mon Sep 17 00:00:00 2001
From: Kirk Rodrigues <2454684+kirkrodrigues@users.noreply.github.com>
Date: Thu, 8 Feb 2024 19:06:19 -0500
Subject: [PATCH] Clean-up clp-s' CLI error behaviour: - Make no
 timestamp/schema matches a success return. - Make error return values
 consistent. - Clean-up log messages on lex/parse error.

---
 components/core/src/clp_s/clp-s.cpp          | 15 +++++++++------
 components/core/src/clp_s/search/kql/kql.cpp | 13 ++++++-------
 components/core/src/clp_s/search/kql/kql.hpp |  2 +-
 3 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/components/core/src/clp_s/clp-s.cpp b/components/core/src/clp_s/clp-s.cpp
index 0e22e30be..63ffc3d7b 100644
--- a/components/core/src/clp_s/clp-s.cpp
+++ b/components/core/src/clp_s/clp-s.cpp
@@ -38,14 +38,14 @@ int main(int argc, char const* argv[]) {
         spdlog::set_pattern("%Y-%m-%dT%H:%M:%S.%e%z [%l] %v");
     } catch (std::exception& e) {
         // NOTE: We can't log an exception if the logger couldn't be constructed
-        return -1;
+        return 1;
     }
 
     CommandLineArguments command_line_arguments("clp-s");
     auto parsing_result = command_line_arguments.parse_arguments(argc, argv);
     switch (parsing_result) {
         case CommandLineArguments::ParsingResult::Failure:
-            return -1;
+            return 1;
         case CommandLineArguments::ParsingResult::InfoCommand:
             return 0;
         case CommandLineArguments::ParsingResult::Success:
@@ -133,6 +133,9 @@ int main(int argc, char const* argv[]) {
 
         auto query_stream = std::istringstream(query);
         auto expr = kql::parse_kql_expression(query_stream);
+        if (nullptr == expr) {
+            return 1;
+        }
 
         if (std::dynamic_pointer_cast<EmptyExpr>(expr)) {
             SPDLOG_ERROR("Query '{}' is logically false", query);
@@ -178,8 +181,8 @@ int main(int argc, char const* argv[]) {
         // the timestamp index
         EvaluateTimestampIndex timestamp_index(timestamp_dict);
         if (clp_s::EvaluatedValue::False == timestamp_index.run(expr)) {
-            SPDLOG_ERROR("No matching timestamp ranges for query '{}'", query);
-            return 1;
+            SPDLOG_INFO("No matching timestamp ranges for query '{}'", query);
+            return 0;
         }
 
         auto schema_tree = clp_s::ReaderUtils::read_schema_tree(archives_dir);
@@ -188,8 +191,8 @@ int main(int argc, char const* argv[]) {
         // Narrow against schemas
         SchemaMatch match_pass(schema_tree, schemas);
         if (expr = match_pass.run(expr); std::dynamic_pointer_cast<EmptyExpr>(expr)) {
-            SPDLOG_ERROR("No matching schemas for query '{}'", query);
-            return 1;
+            SPDLOG_INFO("No matching schemas for query '{}'", query);
+            return 0;
         }
 
         std::unique_ptr<OutputHandler> output_handler;
diff --git a/components/core/src/clp_s/search/kql/kql.cpp b/components/core/src/clp_s/search/kql/kql.cpp
index 52dc4603b..fce008a32 100644
--- a/components/core/src/clp_s/search/kql/kql.cpp
+++ b/components/core/src/clp_s/search/kql/kql.cpp
@@ -3,6 +3,7 @@
 #include <vector>
 
 #include <antlr4-runtime.h>
+#include <spdlog/spdlog.h>
 
 #include "KqlBaseVisitor.h"
 #include "KqlLexer.h"
@@ -221,7 +222,6 @@ class ParseTreeVisitor : public KqlBaseVisitor {
 };
 
 std::shared_ptr<Expression> parse_kql_expression(std::istream& in) {
-    std::shared_ptr<Expression> expr = EmptyExpr::create();
     ErrorListener lexer_error_listener;
     ErrorListener parser_error_listener;
 
@@ -234,15 +234,14 @@ std::shared_ptr<Expression> parse_kql_expression(std::istream& in) {
     KqlParser::StartContext* tree = parser.start();
 
     if (lexer_error_listener.error()) {
-        std::cout << "Lexer Error" << std::endl;
-        return expr;
+        SPDLOG_ERROR("Lexer error");
+        return {};
     } else if (parser_error_listener.error()) {
-        std::cout << "Parser Error" << std::endl;
-        return expr;
+        SPDLOG_ERROR("Parser error");
+        return {};
     }
 
     ParseTreeVisitor visitor;
-    expr = std::any_cast<std::shared_ptr<Expression>>(visitor.visitStart(tree));
-    return expr;
+    return std::any_cast<std::shared_ptr<Expression>>(visitor.visitStart(tree));
 }
 }  // namespace clp_s::search::kql
diff --git a/components/core/src/clp_s/search/kql/kql.hpp b/components/core/src/clp_s/search/kql/kql.hpp
index ce74157fb..51c2202a3 100644
--- a/components/core/src/clp_s/search/kql/kql.hpp
+++ b/components/core/src/clp_s/search/kql/kql.hpp
@@ -9,7 +9,7 @@ namespace clp_s::search::kql {
 /**
  * Generate a search AST from a Kibana expression in an input stream
  * @param in input stream containing a Kibana expression followed by EOF
- * @return a search AST
+ * @return a search AST on success, nullptr otherwise
  */
 std::shared_ptr<Expression> parse_kql_expression(std::istream& in);
 }  // namespace clp_s::search::kql