#972: Refactored exception handling for Script Options parser

exasol · Oct 9, 2024 · 0d6d814 · 0d6d814
1 parent b37f762
commit 0d6d814
Show file tree

Hide file tree

Showing 12 changed files with 120 additions and 146 deletions.
diff --git a/exaudfclient/base/javacontainer/javacontainer_impl.cc b/exaudfclient/base/javacontainer/javacontainer_impl.cc
@@ -35,18 +35,21 @@ JavaVMImpl::JavaVMImpl(bool checkOnly, bool noJNI, SwigFactory& swigFactory)
     m_exaJavaPath = "/exaudf/base/javacontainer"; // TODO hardcoded path
 
     JavaScriptOptions::ScriptOptionLinesParserLegacy scriptOptionsParser;
-    JavaScriptOptions::Extractor extractor(scriptOptionsParser, swigFactory,
-                                           [&](const std::string &msg){throwException(msg);});
+    try {
+        JavaScriptOptions::Extractor extractor(scriptOptionsParser, swigFactory);
 
-    DBG_FUNC_CALL(cerr,extractor.extract(m_scriptCode));  // To be called before scripts are imported. Otherwise, the script classname from an imported script could be used
+        DBG_FUNC_CALL(cerr,extractor.extract(m_scriptCode));  // To be called before scripts are imported. Otherwise, the script classname from an imported script could be used
 
-    DBG_FUNC_CALL(cerr,setClasspath());
+        DBG_FUNC_CALL(cerr,setClasspath());
 
-    m_jvmOptions = std::move(extractor.moveJvmOptions());
+        m_jvmOptions = std::move(extractor.moveJvmOptions());
 
-    for (set<string>::iterator it = extractor.getJarPaths().begin(); it != extractor.getJarPaths().end();
-         ++it) {
-        addJarToClasspath(*it);
+        for (set<string>::iterator it = extractor.getJarPaths().begin(); it != extractor.getJarPaths().end();
+             ++it) {
+            addJarToClasspath(*it);
+        }
+    } catch(const std::runtime_error& ex) {
+        throwException(ex);
     }
 
     m_needsCompilation = checkNeedsCompilation();

diff --git a/exaudfclient/base/javacontainer/script_options/extractor.cc b/exaudfclient/base/javacontainer/script_options/extractor.cc
@@ -11,24 +11,22 @@ namespace SWIGVMContainers {
 namespace JavaScriptOptions {
 
 Extractor::Extractor(ScriptOptionsParser & parser,
-                     SwigFactory& swigFactory,
-                     std::function<void(const std::string&)> throwException)
-: m_throwException(throwException)
-, m_parser(parser)
+                     SwigFactory& swigFactory)
+: m_parser(parser)
 , m_swigFactory(swigFactory) {}
 
 void Extractor::extract(std::string & scriptCode) {
     m_parser.prepareScriptCode(scriptCode);
     EXTR_DBG_FUNC_CALL(m_parser.parseForScriptClass( [&](const std::string& value){
             EXTR_DBG_FUNC_CALL(m_converter.convertScriptClassName(value));
-        }, m_throwException));
-    EXTR_DBG_FUNC_CALL(m_parser.extractImportScripts(m_swigFactory, m_throwException));
+        }));
+    EXTR_DBG_FUNC_CALL(m_parser.extractImportScripts(m_swigFactory));
     EXTR_DBG_FUNC_CALL(m_parser.parseForJvmOptions( [&](const std::string& value){
             EXTR_DBG_FUNC_CALL(m_converter.convertJvmOption(value));
-        }, m_throwException));
+        }));
     EXTR_DBG_FUNC_CALL(m_parser.parseForExternalJars( [&](const std::string& value){
             EXTR_DBG_FUNC_CALL(m_converter.convertExternalJar(value));
-        }, m_throwException));
+        }));
     scriptCode = std::move(m_parser.getScriptCode());
 }
 

diff --git a/exaudfclient/base/javacontainer/script_options/extractor.h b/exaudfclient/base/javacontainer/script_options/extractor.h
@@ -21,8 +21,7 @@ class Extractor {
 
     public:
         Extractor(ScriptOptionsParser & parser,
-                  SwigFactory& swigFactory,
-                  std::function<void(const std::string&)> throwException);
+                  SwigFactory& swigFactory);
 
         const std::set<std::string> & getJarPaths() const {
             return m_converter.getJarPaths();
@@ -35,7 +34,6 @@ class Extractor {
         void extract(std::string & scriptCode);
 
     private:
-        std::function<void(const std::string&)> m_throwException;
 
         Converter m_converter;
         ScriptOptionsParser & m_parser;

diff --git a/exaudfclient/base/javacontainer/script_options/parser.h b/exaudfclient/base/javacontainer/script_options/parser.h
@@ -23,30 +23,26 @@ struct ScriptOptionsParser {
     If the option is found, the function removes the option from "scriptCode" and calls "callback" with the option value and position
     within "scriptCode".
     */
-    virtual void parseForScriptClass(std::function<void(const std::string &option)> callback,
-                                     std::function<void(const std::string&)> throwException) = 0;
+    virtual void parseForScriptClass(std::function<void(const std::string &option)> callback) = 0;
     /*
     Searches for JVM options.
     If an option is found, the function removes the option from "scriptCode" and calls "callback" with the option value and position
     within "scriptCode".
     */
-    virtual void parseForJvmOptions(std::function<void(const std::string &option)> callback,
-                                     std::function<void(const std::string&)> throwException) = 0;
+    virtual void parseForJvmOptions(std::function<void(const std::string &option)> callback) = 0;
 
     /*
     Searches for External Jar.
     If an option is found, the function removes the option from "scriptCode" and calls "callback" with the option value and position
     within "scriptCode".
     */
-    virtual void parseForExternalJars(std::function<void(const std::string &option)> callback,
-                                      std::function<void(const std::string&)> throwException) = 0;
+    virtual void parseForExternalJars(std::function<void(const std::string &option)> callback) = 0;
 
     /*
      Searches for the "%import" options and embeds the respective imported script code at the same location as
      the option in the script code.
     */
-    virtual void extractImportScripts(SwigFactory & swigFactory,
-                                      std::function<void(const std::string&)> throwException) = 0;
+    virtual void extractImportScripts(SwigFactory & swigFactory) = 0;
 
     /*
      Returns the (eventually modified) script code.

diff --git a/exaudfclient/base/javacontainer/script_options/parser_legacy.cc b/exaudfclient/base/javacontainer/script_options/parser_legacy.cc
@@ -5,6 +5,9 @@
 #include "base/swig_factory/swig_factory.h"
 
 #include <memory>
+#include <stdexcept>
+
+
 
 namespace SWIGVMContainers {
 
@@ -20,8 +23,7 @@ void ScriptOptionLinesParserLegacy::prepareScriptCode(const std::string & script
     m_scriptCode = scriptCode;
 }
 
-void ScriptOptionLinesParserLegacy::extractImportScripts(SwigFactory & swigFactory,
-                                                         std::function<void(const std::string&)> throwException) {
+void ScriptOptionLinesParserLegacy::extractImportScripts(SwigFactory & swigFactory) {
     std::unique_ptr<SWIGMetadataIf> metaData;
     // Attention: We must hash the parent script before modifying it (adding the
     // package definition). Otherwise we don't recognize if the script imports its self
@@ -101,19 +103,22 @@ void ScriptOptionLinesParserLegacy::extractImportScripts(SwigFactory & swigFacto
     while (true) {
         std::string newScript;
         size_t scriptPos;
-        parseForSingleOption(m_keywords.importKeyword(),
-                             [&](const std::string& value, size_t pos){scriptPos = pos; newScript = value;},
-                             [&](const std::string& msg){throwException("F-UDF-CL-SL-JAVA-1614" + msg);});
+        try {
+            parseForSingleOption(m_keywords.importKeyword(),
+                                 [&](const std::string& value, size_t pos){scriptPos = pos; newScript = value;});
+        } catch (const std::runtime_error & ex) {
+            throw std::runtime_error(std::string("F-UDF-CL-SL-JAVA-1614 ") +  ex.what());
+        }
         if (!newScript.empty()) {
             if (!metaData) {
                 metaData.reset(swigFactory.makeSwigMetadata());
                 if (!metaData)
-                    throwException("F-UDF-CL-SL-JAVA-1615: Failure while importing scripts");
+                    throw std::runtime_error("F-UDF-CL-SL-JAVA-1615: Failure while importing scripts");
             }
             const char *importScriptCode = metaData->moduleContent(newScript.c_str());
             const char *exception = metaData->checkException();
             if (exception)
-                throwException("F-UDF-CL-SL-JAVA-1616: " + std::string(exception));
+                throw std::runtime_error("F-UDF-CL-SL-JAVA-1616: " + std::string(exception));
             if (importedScriptChecksums.addScript(importScriptCode)) {
                 // Script has not been imported yet
                 // If this imported script contains %import statements
@@ -126,66 +131,64 @@ void ScriptOptionLinesParserLegacy::extractImportScripts(SwigFactory & swigFacto
     }
 }
 
-void ScriptOptionLinesParserLegacy::parseForScriptClass(std::function<void(const std::string &option)> callback,
-                                 std::function<void(const std::string&)> throwException) {
+void ScriptOptionLinesParserLegacy::parseForScriptClass(std::function<void(const std::string &option)> callback) {
+    try {
     parseForSingleOption(m_keywords.scriptClassKeyword(),
-                            [&](const std::string& value, size_t pos){callback(value);},
-                            [&](const std::string& msg){throwException("F-UDF-CL-SL-JAVA-1610" + msg);});
+                            [&](const std::string& value, size_t pos){callback(value);});
+    } catch (const std::runtime_error& ex) {
+        throw std::runtime_error(std::string("F-UDF-CL-SL-JAVA-1610") + ex.what());
+    }
 }
 
-void ScriptOptionLinesParserLegacy::parseForJvmOptions(std::function<void(const std::string &option)> callback,
-                                 std::function<void(const std::string&)> throwException) {
-   parseForMultipleOptions(m_keywords.jvmOptionKeyword(),
-                            [&](const std::string& value, size_t pos){callback(value);},
-                            [&](const std::string& msg){throwException("F-UDF-CL-SL-JAVA-1612" + msg);});
+void ScriptOptionLinesParserLegacy::parseForJvmOptions(std::function<void(const std::string &option)> callback) {
+    try {
+       parseForMultipleOptions(m_keywords.jvmOptionKeyword(),
+                                [&](const std::string& value, size_t pos){callback(value);});
+    } catch(const std::runtime_error& ex) {
+        throw std::runtime_error(std::string("F-UDF-CL-SL-JAVA-1612") + ex.what());
+    }
 }
 
-void ScriptOptionLinesParserLegacy::parseForExternalJars(std::function<void(const std::string &option)> callback,
-                                 std::function<void(const std::string&)> throwException) {
-   parseForMultipleOptions(m_keywords.jarKeyword(),
-                            [&](const std::string& value, size_t pos){callback(value);},
-                            [&](const std::string& msg){throwException("F-UDF-CL-SL-JAVA-1613" + msg);});
+void ScriptOptionLinesParserLegacy::parseForExternalJars(std::function<void(const std::string &option)> callback) {
+    try {
+       parseForMultipleOptions(m_keywords.jarKeyword(),
+                                [&](const std::string& value, size_t pos){callback(value);});
+    } catch(const std::runtime_error& ex) {
+        throw std::runtime_error(std::string("F-UDF-CL-SL-JAVA-1613") + ex.what());
+    }
 }
 
 std::string && ScriptOptionLinesParserLegacy::getScriptCode() {
     return std::move(m_scriptCode);
 }
 
 void ScriptOptionLinesParserLegacy::parseForSingleOption(const std::string keyword,
-                            std::function<void(const std::string &option, size_t pos)> callback,
-                            std::function<void(const std::string&)> throwException) {
+                            std::function<void(const std::string &option, size_t pos)> callback) {
     size_t pos;
-    const std::string option =
-      ExecutionGraph::extractOptionLine(
-          m_scriptCode,
-          keyword,
-          m_whitespace,
-          m_lineend,
-          pos,
-          [&](const char* msg){throwException(std::string("F-UDF-CL-SL-JAVA-1606: ") + msg);}
-          );
-    if (option != "") {
-        callback(option, pos);
+    try {
+        const std::string option =
+          ExecutionGraph::extractOptionLine(m_scriptCode, keyword, m_whitespace, m_lineend, pos);
+        if (option != "") {
+            callback(option, pos);
+        }
+    } catch(const std::runtime_error& ex) {
+        throw std::runtime_error(std::string("F-UDF-CL-SL-JAVA-1606") + ex.what());
     }
 }
 
 void ScriptOptionLinesParserLegacy::parseForMultipleOptions(const std::string keyword,
-                            std::function<void(const std::string &option, size_t pos)> callback,
-                            std::function<void(const std::string&)> throwException) {
+                            std::function<void(const std::string &option, size_t pos)> callback) {
     size_t pos;
     while (true) {
-        const std::string option =
-          ExecutionGraph::extractOptionLine(
-              m_scriptCode,
-              keyword,
-              m_whitespace,
-              m_lineend,
-              pos,
-              [&](const char* msg){throwException(std::string("F-UDF-CL-SL-JAVA-1607: ") + msg);}
-              );
-        if (option == "")
-            break;
-        callback(option, pos);
+        try {
+            const std::string option =
+              ExecutionGraph::extractOptionLine(m_scriptCode, keyword, m_whitespace, m_lineend, pos);
+            if (option == "")
+                break;
+            callback(option, pos);
+        } catch(const std::runtime_error& ex) {
+            throw std::runtime_error(std::string("F-UDF-CL-SL-JAVA-1607") + ex.what());
+        }
     }
 }
 

diff --git a/exaudfclient/base/javacontainer/script_options/parser_legacy.h b/exaudfclient/base/javacontainer/script_options/parser_legacy.h
@@ -17,27 +17,21 @@ class ScriptOptionLinesParserLegacy : public ScriptOptionsParser {
 
         void prepareScriptCode(const std::string & scriptCode) override;
 
-        void parseForScriptClass(std::function<void(const std::string &option)> callback,
-                                 std::function<void(const std::string&)> throwException) override;
+        void parseForScriptClass(std::function<void(const std::string &option)> callback) override;
 
-        void parseForJvmOptions(std::function<void(const std::string &option)> callback,
-                                std::function<void(const std::string&)> throwException) override;
+        void parseForJvmOptions(std::function<void(const std::string &option)> callback) override;
 
-        void parseForExternalJars(std::function<void(const std::string &option)> callback,
-                                  std::function<void(const std::string&)> throwException) override;
+        void parseForExternalJars(std::function<void(const std::string &option)> callback) override;
 
-        void extractImportScripts(SwigFactory & swigFactory,
-                                  std::function<void(const std::string&)> throwException) override;
+        void extractImportScripts(SwigFactory & swigFactory) override;
 
         std::string && getScriptCode() override;
 
     private:
         void parseForSingleOption(const std::string key,
-                                        std::function<void(const std::string &option, size_t pos)> callback,
-                                        std::function<void(const std::string&)> throwException);
+                                        std::function<void(const std::string &option, size_t pos)> callback);
         void parseForMultipleOptions(const std::string key,
-                                                std::function<void(const std::string &option, size_t pos)> callback,
-                                                std::function<void(const std::string&)> throwException);
+                                                std::function<void(const std::string &option, size_t pos)> callback);
 
     private:
         const std::string m_whitespace;

diff --git a/exaudfclient/base/script_options_parser/ctpg/script_option_lines_ctpg.cc b/exaudfclient/base/script_options_parser/ctpg/script_option_lines_ctpg.cc
@@ -133,7 +133,7 @@ constexpr parser option_parser(
     )
 );
 
-void parse(std::string&& code, options_type& result, std::function<void(const char*)> throwException) {
+void parse(std::string&& code, options_type& result) {
     std::stringstream error_buffer;
     auto res = option_parser.parse(
         parse_options{}.set_skip_whitespace(false),
@@ -147,13 +147,13 @@ void parse(std::string&& code, options_type& result, std::function<void(const ch
     {
         std::stringstream ss;
         ss << "Error parsing script options: " << error_buffer.str();
-        throwException(ss.str().c_str());
+        throw std::runtime_error(ss.str());
     }
 }
 
 } //namespace ParserInternals
 
-void parseOptions(const std::string& code, options_map_t & result, std::function<void(const char*)> throwException) {
+void parseOptions(const std::string& code, options_map_t & result) {
 
     size_t current_pos = 0;
 
@@ -163,7 +163,7 @@ void parseOptions(const std::string& code, options_map_t & result, std::function
         std::string line = code.substr(current_pos, new_pos);
         if (!line.empty() && !std::all_of(line.begin(),line.end(), [](const char c) {return std::isspace(c);})) {
             options_type parser_result;
-            ParserInternals::parse(std::move(line), parser_result, throwException);
+            ParserInternals::parse(std::move(line), parser_result);
             for (const auto & option: parser_result)
             {
                 ScriptOption entry = {

diff --git a/exaudfclient/base/script_options_parser/ctpg/script_option_lines_ctpg.h b/exaudfclient/base/script_options_parser/ctpg/script_option_lines_ctpg.h
@@ -45,9 +45,9 @@ using options_map_t = std::map<std::string, options_t>;
  * \param code Reference to string where the script code is stored.
  * \param result Result of all found options.
  * \param throwException Function to be called to throw exception.
- *
+ * \throws std::runtime_error if parsing fails
  */
-void parseOptions(const std::string& code, options_map_t & result, std::function<void(const char*)> throwException);
+void parseOptions(const std::string& code, options_map_t & result);
 
 } //namespace CTPG