Skip to content

Commit

Permalink
vdk-core: replace report_and_rethrow (#3056)
Browse files Browse the repository at this point in the history
## 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 <mdilyan@vmware.com>
  • Loading branch information
DeltaMichael committed Jan 29, 2024
1 parent a9c1c17 commit 64864b1
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand All @@ -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():
Expand All @@ -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())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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":
Expand Down Expand Up @@ -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(
Expand Down
3 changes: 2 additions & 1 deletion projects/vdk-core/src/vdk/internal/core/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,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:
Expand Down
12 changes: 12 additions & 0 deletions projects/vdk-core/tests/vdk/internal/core/test_errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down

0 comments on commit 64864b1

Please sign in to comment.