vdk-core: BaseVdkError exception propagation flaw fix (#917)

There was an issue, where a particular exceptions were not propagating
to the respective except(catch) clauses. The process was suddenly
blindly exiting with code 1.

Did troubleshoot the flaw to vdk-core's error framework, where
BaseVdkError was extending ClickException without any purpose
known/documented.
That was causing all the BaseVdkError-based DomainErrors like
VdkConfigurationError, UserCodeError, PlatformServiceError, and so on,
to stop propagating to handlers. Did fix the errors framework design
flaw.

Testing Done: added the vdk_exception exit code test coverage,
that relies on proper exception propagation and handling; ci/cd

Signed-off-by: ikoleva <ikoleva@vmware.com>

projects/vdk-core/src/vdk/internal/builtin_plugins/run/execution_results.py

@@ -7,7 +7,6 @@
 from typing import List
 from typing import Optional
 
-from click import ClickException
 from vdk.internal.builtin_plugins.run.run_status import ExecutionStatus
 from vdk.internal.core import errors
 from vdk.internal.core.errors import ErrorMessage
@@ -78,11 +77,7 @@ def get_exception_to_raise(self):
         Returns main exception to be used as a failure reason for the data job.
         """
         if self.exception:
-            return (
-                Exception(self.exception.message)
-                if isinstance(self.exception, ClickException)
-                else self.exception
-            )
+            return self.exception
         step_exceptions = map(lambda x: x.exception, self.steps_list)
         step_exception = next(filter(lambda e: e is not None, step_exceptions), None)
         if step_exception:

projects/vdk-core/src/vdk/internal/builtin_plugins/run/standalone_data_job.py

@@ -8,7 +8,6 @@
 from pathlib import Path
 from typing import cast
 
-from click import ClickException
 from vdk.api.data_job import IStandaloneDataJob
 from vdk.api.job_input import IJobInput
 from vdk.api.plugin.core_hook_spec import CoreHookSpecs
@@ -150,9 +149,7 @@ def __exit__(self, exc_type, exc_value, exc_traceback):
             # if at least one hook implementation returned handled, means we do not need to log the exception
             if not (True in handled):
                 log.exception("Exiting with exception.")
-                exit_code = (
-                    exc_value.exit_code if isinstance(exc_value, ClickException) else 1
-                )
+                exit_code = 2
             else:
                 exit_code = 0
         else:

projects/vdk-core/src/vdk/internal/cli_entry.py

@@ -7,7 +7,6 @@
 
 import click
 import click_log
-from click import ClickException
 from click_plugins import with_plugins
 from pkg_resources import iter_entry_points
 from vdk.api.plugin.core_hook_spec import CoreHookSpecs
@@ -143,7 +142,7 @@ def vdk_main(
             # if at least one hook implementation returned handled, means we do not need to log the exception
             if not (True in handled):
                 log.exception("Exiting with exception.")
-                exit_code = e.exit_code if isinstance(e, ClickException) else 1
+                exit_code = 1
             else:
                 exit_code = 0
             return exit_code

projects/vdk-core/src/vdk/internal/core/errors.py

@@ -21,8 +21,6 @@
 from typing import Any
 from typing import cast
 
-from click import ClickException
-
 
 class ResolvableBy(str, Enum):
     """
@@ -51,7 +49,7 @@ class ResolvableBy(str, Enum):
 log = logging.getLogger(__name__)
 
 
-class BaseVdkError(ClickException):
+class BaseVdkError(Exception):
     """For all errors risen from our "code" (vdk)
 
     There are two child branches in exception hierarchy:

projects/vdk-core/tests/functional/run/test_run_errors.py

@@ -55,6 +55,21 @@ def initialize_job(self, context: JobContext):
     )
 
 
+def test_run_exception_handled(tmp_termination_msg_file: pathlib.Path):
+    errors.clear_intermediate_errors()
+
+    class ExceptionHandler:
+        @staticmethod
+        @hookimpl
+        def vdk_exception(self, exception: Exception) -> bool:
+            return True
+
+    runner = CliEntryBasedTestRunner(ExceptionHandler())
+
+    result: Result = runner.invoke(["run", util.job_path("simple-job")])
+    cli_assert_equal(0, result)
+
+
 def test_run_job_plugin_fails(tmp_termination_msg_file):
     errors.clear_intermediate_errors()
 

projects/vdk-core/tests/vdk/internal/builtin_plugins/ingestion/test_ingester_router.py

@@ -142,7 +142,6 @@ def test_router_raise_error_chained_ingest_plugins_not_registered():
         router.send_tabular_data_for_ingestion(rows=["b"], column_names=["a"])
 
     error_msg = exc_info.value
-    assert (
-        "Could not create new processor plugin of type pre-ingest-test"
-        in error_msg.message
+    assert "Could not create new processor plugin of type pre-ingest-test" in str(
+        error_msg
     )