From 4a7544b9791d5c5235496f0ca197ef340a3e287c Mon Sep 17 00:00:00 2001 From: Dilyan Marinov Date: Fri, 26 Jan 2024 11:18:03 +0200 Subject: [PATCH] vdk-core: replace report_and_rethrow Why? Calling report_and_rethrow causes confusing stack traces, because of adding an extra frame from the extra function call What? Replace report_and_rethrow with just calling report and raising the exception after How was this tested? CI What kind of change is this? Feature/non-breaking Signed-off-by: Dilyan Marinov --- .../connection/managed_connection_base.py | 6 ++---- .../builtin_plugins/connection/managed_cursor.py | 9 ++++++--- .../vdk/internal/builtin_plugins/run/cli_run.py | 6 ++++-- .../builtin_plugins/run/file_based_step.py | 6 ++++-- projects/vdk-core/src/vdk/internal/core/errors.py | 14 +++++++++----- .../tests/vdk/internal/core/test_errors.py | 12 ++++++++++++ 6 files changed, 37 insertions(+), 16 deletions(-) diff --git a/projects/vdk-core/src/vdk/internal/builtin_plugins/connection/managed_connection_base.py b/projects/vdk-core/src/vdk/internal/builtin_plugins/connection/managed_connection_base.py index 7b9201d174..cc5a30d18a 100644 --- a/projects/vdk-core/src/vdk/internal/builtin_plugins/connection/managed_connection_base.py +++ b/projects/vdk-core/src/vdk/internal/builtin_plugins/connection/managed_connection_base.py @@ -156,10 +156,8 @@ def execute_query(self, query: str) -> List[List[Any]]: ] ) ) - errors.report_and_rethrow( - blamee, - e, - ) + errors.report(blamee, e) + raise e return cast( List[List[Any]], res ) # we return None in case of DML. This is not PEP249 compliant, but is more convenient diff --git a/projects/vdk-core/src/vdk/internal/builtin_plugins/connection/managed_cursor.py b/projects/vdk-core/src/vdk/internal/builtin_plugins/connection/managed_cursor.py index c9abb2193c..205f515921 100644 --- a/projects/vdk-core/src/vdk/internal/builtin_plugins/connection/managed_cursor.py +++ b/projects/vdk-core/src/vdk/internal/builtin_plugins/connection/managed_cursor.py @@ -119,10 +119,11 @@ def execute( ] ) ) - errors.report_and_rethrow( + errors.report( blamee, e, ) + raise e def _decorate_operation(self, managed_operation: ManagedOperation, operation: str): if self.__connection_hook_spec.db_connection_decorate_operation.get_hookimpls(): @@ -143,7 +144,8 @@ def _decorate_operation(self, managed_operation: ManagedOperation, operation: st ] ) ) - errors.report_and_rethrow(errors.ResolvableBy.PLATFORM_ERROR, e) + errors.report(errors.ResolvableBy.PLATFORM_ERROR, e) + raise e def _validate_operation(self, operation: str, parameters: Optional[Container]): if self.__connection_hook_spec.db_connection_validate_operation.get_hookimpls(): @@ -161,10 +163,11 @@ def _validate_operation(self, operation: str, parameters: Optional[Container]): ] ) ) - errors.report_and_rethrow( + errors.report( errors.ResolvableBy.USER_ERROR, exception=e, ) + raise e def _execute_operation(self, managed_operation: ManagedOperation): self._log.info("Executing query:\n%s" % managed_operation.get_operation()) diff --git a/projects/vdk-core/src/vdk/internal/builtin_plugins/run/cli_run.py b/projects/vdk-core/src/vdk/internal/builtin_plugins/run/cli_run.py index f2b9bb1c64..cf03d6a05e 100644 --- a/projects/vdk-core/src/vdk/internal/builtin_plugins/run/cli_run.py +++ b/projects/vdk-core/src/vdk/internal/builtin_plugins/run/cli_run.py @@ -49,7 +49,8 @@ def __validate_and_parse_args(arguments: str) -> Optional[Dict]: ] ) ) - errors.report_and_rethrow(errors.ResolvableBy.USER_ERROR, e) + errors.report(errors.ResolvableBy.USER_ERROR, e) + raise e @staticmethod def __split_into_chunks(exec_steps: List, chunks: int) -> List: @@ -193,12 +194,13 @@ def create_and_run_data_job( ] ) ) - errors.report_and_rethrow( + errors.report( job_input_error_classifier.whom_to_blame( e, __file__, data_job_directory ), e, ) + raise e if execution_result.is_failed() and execution_result.get_exception_to_raise(): raise execution_result.get_exception_to_raise() diff --git a/projects/vdk-core/src/vdk/internal/builtin_plugins/run/file_based_step.py b/projects/vdk-core/src/vdk/internal/builtin_plugins/run/file_based_step.py index 3b55c44038..b09ba83d1c 100644 --- a/projects/vdk-core/src/vdk/internal/builtin_plugins/run/file_based_step.py +++ b/projects/vdk-core/src/vdk/internal/builtin_plugins/run/file_based_step.py @@ -97,7 +97,8 @@ def run(job_input) ] ) ) - errors.report_and_rethrow(errors.ResolvableBy.USER_ERROR, e) + errors.report(errors.ResolvableBy.USER_ERROR, e) + raise e for _, func in inspect.getmembers(python_module, inspect.isfunction): if func.__name__ == "run": @@ -153,7 +154,8 @@ def invoke_run_function(func: Callable, job_input: IJobInput, step_name: str): ) to_be_fixed_by = whom_to_blame(e, __file__, None) - errors.report_and_rethrow(to_be_fixed_by, e) + errors.report(to_be_fixed_by, e) + raise e else: errors.report_and_throw( errors.UserCodeError( diff --git a/projects/vdk-core/src/vdk/internal/core/errors.py b/projects/vdk-core/src/vdk/internal/core/errors.py index ded6a054e6..df3d26aef6 100644 --- a/projects/vdk-core/src/vdk/internal/core/errors.py +++ b/projects/vdk-core/src/vdk/internal/core/errors.py @@ -87,17 +87,19 @@ def pretty_vdk_error_formatter(ex: BaseVdkError): if br == 0: br = max_len wrapped_lines.append(l[: br + 1]) - l = l[br + 1 :] + l = l[br + 1:] wrapped_lines.append(l) else: wrapped_lines.append(line) # build th header header = box_char + header.center(max_len + 6) + box_char # build the lines with the box char - lines = [box_char + " " + s.ljust(max_len + 4) + box_char for s in wrapped_lines] + lines = [box_char + " " + + s.ljust(max_len + 4) + box_char for s in wrapped_lines] # build the box sides side = (max_len + 8) * box_char - ex._pretty_message = "\n".join(["\n" + side, header, side, "\n".join(lines), side]) + ex._pretty_message = "\n".join( + ["\n" + side, header, side, "\n".join(lines), side]) class BaseVdkError(Exception): @@ -116,7 +118,8 @@ def __init__( # Check if error message or dict was passed # for compatibility with vdk plugins self._lines = [] - header = f"{self.__class__.__name__}: An error of resolvable type {vdk_resolvable_actual} occurred" + header = f"{self.__class__.__name__}: An error of resolvable type { + vdk_resolvable_actual} occurred" self._lines.append(header) if error_message_lines and isinstance(error_message_lines[0], ErrorMessage): message = error_message_lines[0] @@ -259,7 +262,8 @@ def log_and_rethrow( # wrap message = [what_happened, why_it_happened, consequences, countermeasures] log.error("\n".join(message)) - report_and_rethrow(to_be_fixed_by, exception) + report(to_be_fixed_by, exception) + raise exception class ErrorMessage: diff --git a/projects/vdk-core/tests/vdk/internal/core/test_errors.py b/projects/vdk-core/tests/vdk/internal/core/test_errors.py index 5ed943aa96..e29c82e379 100644 --- a/projects/vdk-core/tests/vdk/internal/core/test_errors.py +++ b/projects/vdk-core/tests/vdk/internal/core/test_errors.py @@ -168,6 +168,18 @@ def test_report_and_rethrow(self): is 1 ) + def test_report(self): + ex = IndexError("foo") + errors.report( + errors.ResolvableBy.USER_ERROR, + exception=ex, + ) + assert errors.ResolvableByActual.USER in errors.resolvable_context().resolvables + assert ( + len(errors.resolvable_context().resolvables[errors.ResolvableByActual.USER]) + is 1 + ) + def test_report_and_throw(self): with pytest.raises(errors.PlatformServiceError): errors.report_and_throw(PlatformServiceError("My super awesome message"))