From 515763b32420d9df470b3db24cbdad4aee7a9b57 Mon Sep 17 00:00:00 2001 From: Andon Andonov Date: Mon, 29 Nov 2021 10:13:28 +0200 Subject: [PATCH 1/2] vdk-core: Fix error in user code classified as Platform Error Currently, if an exception is raised in user code (the run method in a data job python script), the error is classified as a Platform Error, even though it is caused by something outside of vdk control. This happens because in the DataJobDefaultHookImplPlugin class, the exception catching logic uses errors.find_whom_to_blame_from_exception() method, instead of job_input_error_classifier.whom_to_blame() method. The former just checks the exception class, while the latter actually parses the exception traceback, and tries to properly classify the error. This change moves error classification from errors.find_whom_to_blame_from_exception() to job_input_error_classifier.whom_to_blame(), and improves the robustness of the latter in case there are changes in vdk file paths which could lead to different file paths in user code exceptions. Testing Done: Unit tests Signed-off-by: Andon Andonov --- .../internal/builtin_plugins/run/data_job.py | 5 ++-- .../run/job_input_error_classifier.py | 8 ++++--- .../run/job_input_error_classifier_test.py | 23 +++++++++++-------- 3 files changed, 21 insertions(+), 15 deletions(-) diff --git a/projects/vdk-core/src/vdk/internal/builtin_plugins/run/data_job.py b/projects/vdk-core/src/vdk/internal/builtin_plugins/run/data_job.py index c76a6feb3c..284dc2569c 100644 --- a/projects/vdk-core/src/vdk/internal/builtin_plugins/run/data_job.py +++ b/projects/vdk-core/src/vdk/internal/builtin_plugins/run/data_job.py @@ -22,6 +22,7 @@ from vdk.internal.builtin_plugins.run.job_context import JobContext from vdk.internal.builtin_plugins.run.run_status import ExecutionStatus from vdk.internal.builtin_plugins.run.step import Step +from vdk.internal.builtin_plugins.run.job_input_error_classifier import whom_to_blame from vdk.internal.core import errors from vdk.internal.core.context import CoreContext from vdk.internal.core.statestore import CommonStoreKeys @@ -75,10 +76,10 @@ def run_step(context: JobContext, step: Step) -> StepResult: if step_executed else ExecutionStatus.NOT_RUNNABLE ) - except BaseException as e: + except Exception as e: status = ExecutionStatus.ERROR details = errors.MSG_WHY_FROM_EXCEPTION(e) - blamee = errors.find_whom_to_blame_from_exception(e) + blamee = whom_to_blame(e, __file__) exception = e errors.log_exception( blamee, diff --git a/projects/vdk-core/src/vdk/internal/builtin_plugins/run/job_input_error_classifier.py b/projects/vdk-core/src/vdk/internal/builtin_plugins/run/job_input_error_classifier.py index e98d4d7765..d58628ccd1 100644 --- a/projects/vdk-core/src/vdk/internal/builtin_plugins/run/job_input_error_classifier.py +++ b/projects/vdk-core/src/vdk/internal/builtin_plugins/run/job_input_error_classifier.py @@ -32,6 +32,7 @@ def whom_to_blame(exception, executor_module): def _is_exception_from_vdk_code(exception, executor_module): + exception_in_vdk = False executor_module = os.path.abspath(executor_module) vdk_code_directory = os.path.dirname(executor_module) call_list = traceback.format_tb(exception.__traceback__) @@ -42,12 +43,13 @@ def _is_exception_from_vdk_code(exception, executor_module): for call in call_list: caller_module = call.split('"')[1] # Extract module path from stacktrace call. if vdk_code_directory in caller_module and caller_module != executor_module: - return True + exception_in_vdk = True elif ( caller_module == executor_module ): # User code starts from this module always. - break - return False + return False + + return exception_in_vdk def is_user_error(received_exception: Exception) -> bool: diff --git a/projects/vdk-core/tests/vdk/internal/builtin_plugins/run/job_input_error_classifier_test.py b/projects/vdk-core/tests/vdk/internal/builtin_plugins/run/job_input_error_classifier_test.py index b85a087c18..83c5589d00 100644 --- a/projects/vdk-core/tests/vdk/internal/builtin_plugins/run/job_input_error_classifier_test.py +++ b/projects/vdk-core/tests/vdk/internal/builtin_plugins/run/job_input_error_classifier_test.py @@ -31,6 +31,17 @@ class ErrorClassifierTest(unittest.TestCase): ), ] + GENERIC_USER_ERROR_STACKTRACE = [ + f""""{EXECUTOR_MODULE}", line 71, in run_step + step_executed = step.runner_func(step, context.job_input)""", + f"""File "{EXECUTOR_MODULE_DIR}/file_based_step.py", line 83, in run_python_step + StepFuncFactory.invoke_run_function(func, job_input)""", + f"""File "{EXECUTOR_MODULE_DIR}/file_based_step.py", line 117, in invoke_run_function + func(**actual_arguments)""", + """File "/example_project/my-second-job/20_python_step.py", line 24, in run + raise Exception("Some test exception from user code") Exception: Some test exception from user code""" + ] + PLATFORM_ERROR_STACKTRACE = [ """File "{exec_module}", line 133, in _run_step step_executed = runner_func(file_path)""".format( @@ -87,18 +98,10 @@ def test_unknown_generic_error(self, mock_is_user_error, mock_traceback_format_t errors.ResolvableBy.USER_ERROR, ) - # Generic error thrown by job_input that is not specifically recognised by VDK should be VAC error. - @patch(f"{traceback.format_tb.__module__}.{traceback.format_tb.__name__}") - @patch(f"{is_user_error.__module__}.{is_user_error.__name__}") - def test_job_input_generic_error( - self, mock_is_user_error, mock_traceback_format_tb - ): - exception = Exception("!") - mock_is_user_error.return_value = False - mock_traceback_format_tb.return_value = self.PLATFORM_ERROR_STACKTRACE + mock_traceback_format_tb.return_value = self.GENERIC_USER_ERROR_STACKTRACE self.assertEqual( whom_to_blame(exception, self.EXECUTOR_MODULE), - errors.ResolvableBy.PLATFORM_ERROR, + errors.ResolvableBy.USER_ERROR, ) From b10a5ee85cb34114aaecb7246eb8651ac1bddb64 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 29 Nov 2021 09:22:12 +0000 Subject: [PATCH 2/2] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- .../vdk-core/src/vdk/internal/builtin_plugins/run/data_job.py | 2 +- .../builtin_plugins/run/job_input_error_classifier_test.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/projects/vdk-core/src/vdk/internal/builtin_plugins/run/data_job.py b/projects/vdk-core/src/vdk/internal/builtin_plugins/run/data_job.py index 284dc2569c..28e4ad25e8 100644 --- a/projects/vdk-core/src/vdk/internal/builtin_plugins/run/data_job.py +++ b/projects/vdk-core/src/vdk/internal/builtin_plugins/run/data_job.py @@ -20,9 +20,9 @@ from vdk.internal.builtin_plugins.run.file_based_step import TYPE_PYTHON from vdk.internal.builtin_plugins.run.file_based_step import TYPE_SQL from vdk.internal.builtin_plugins.run.job_context import JobContext +from vdk.internal.builtin_plugins.run.job_input_error_classifier import whom_to_blame from vdk.internal.builtin_plugins.run.run_status import ExecutionStatus from vdk.internal.builtin_plugins.run.step import Step -from vdk.internal.builtin_plugins.run.job_input_error_classifier import whom_to_blame from vdk.internal.core import errors from vdk.internal.core.context import CoreContext from vdk.internal.core.statestore import CommonStoreKeys diff --git a/projects/vdk-core/tests/vdk/internal/builtin_plugins/run/job_input_error_classifier_test.py b/projects/vdk-core/tests/vdk/internal/builtin_plugins/run/job_input_error_classifier_test.py index 83c5589d00..1c7102f70c 100644 --- a/projects/vdk-core/tests/vdk/internal/builtin_plugins/run/job_input_error_classifier_test.py +++ b/projects/vdk-core/tests/vdk/internal/builtin_plugins/run/job_input_error_classifier_test.py @@ -39,7 +39,7 @@ class ErrorClassifierTest(unittest.TestCase): f"""File "{EXECUTOR_MODULE_DIR}/file_based_step.py", line 117, in invoke_run_function func(**actual_arguments)""", """File "/example_project/my-second-job/20_python_step.py", line 24, in run - raise Exception("Some test exception from user code") Exception: Some test exception from user code""" + raise Exception("Some test exception from user code") Exception: Some test exception from user code""", ] PLATFORM_ERROR_STACKTRACE = [