Skip to content

Commit

Permalink
vdk-core: replace report_and_rethrow
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 <[email protected]>
  • Loading branch information
Dilyan Marinov committed Jan 26, 2024
1 parent 59d2a85 commit 4a7544b
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 16 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
14 changes: 9 additions & 5 deletions projects/vdk-core/src/vdk/internal/core/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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]
Expand Down Expand Up @@ -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:
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 4a7544b

Please sign in to comment.