diff --git a/google/cloud/bigtable/data/exceptions.py b/google/cloud/bigtable/data/exceptions.py index 9b6b4fe3f..7344874df 100644 --- a/google/cloud/bigtable/data/exceptions.py +++ b/google/cloud/bigtable/data/exceptions.py @@ -74,7 +74,44 @@ def __init__(self, message, excs): if len(excs) == 0: raise ValueError("exceptions must be a non-empty sequence") self.exceptions = tuple(excs) - super().__init__(message) + # simulate an exception group in Python < 3.11 by adding exception info + # to the message + first_line = "--+---------------- 1 ----------------" + last_line = "+------------------------------------" + message_parts = [message + "\n" + first_line] + # print error info for each exception in the group + for idx, e in enumerate(excs[:15]): + # apply index header + if idx != 0: + message_parts.append( + f"+---------------- {str(idx+1).rjust(2)} ----------------" + ) + cause = e.__cause__ + # if this exception was had a cause, print the cause first + # used to display root causes of FailedMutationEntryError and FailedQueryShardError + # format matches the error output of Python 3.11+ + if cause is not None: + message_parts.extend( + f"| {type(cause).__name__}: {cause}".splitlines() + ) + message_parts.append("| ") + message_parts.append( + "| The above exception was the direct cause of the following exception:" + ) + message_parts.append("| ") + # attach error message for this sub-exception + # if the subexception is also a _BigtableExceptionGroup, + # error messages will be nested + message_parts.extend(f"| {type(e).__name__}: {e}".splitlines()) + # truncate the message if there are more than 15 exceptions + if len(excs) > 15: + message_parts.append("+---------------- ... ---------------") + message_parts.append(f"| and {len(excs) - 15} more") + if last_line not in message_parts[-1]: + # in the case of nested _BigtableExceptionGroups, the last line + # does not need to be added, since one was added by the final sub-exception + message_parts.append(last_line) + super().__init__("\n ".join(message_parts)) def __new__(cls, message, excs): if is_311_plus: @@ -83,11 +120,19 @@ def __new__(cls, message, excs): return super().__new__(cls) def __str__(self): + if is_311_plus: + # don't return built-in sub-exception message + return self.args[0] + return super().__str__() + + def __repr__(self): """ - String representation doesn't display sub-exceptions. Subexceptions are - described in message + repr representation should strip out sub-exception details """ - return self.args[0] + if is_311_plus: + return super().__repr__() + message = self.args[0].split("\n")[0] + return f"{self.__class__.__name__}({message!r}, {self.exceptions!r})" class MutationsExceptionGroup(_BigtableExceptionGroup): @@ -200,14 +245,12 @@ def __init__( idempotent_msg = ( "idempotent" if failed_mutation_entry.is_idempotent() else "non-idempotent" ) - index_msg = f" at index {failed_idx} " if failed_idx is not None else " " - message = ( - f"Failed {idempotent_msg} mutation entry{index_msg}with cause: {cause!r}" - ) + index_msg = f" at index {failed_idx}" if failed_idx is not None else "" + message = f"Failed {idempotent_msg} mutation entry{index_msg}" super().__init__(message) + self.__cause__ = cause self.index = failed_idx self.entry = failed_mutation_entry - self.__cause__ = cause class RetryExceptionGroup(_BigtableExceptionGroup): @@ -217,10 +260,8 @@ class RetryExceptionGroup(_BigtableExceptionGroup): def _format_message(excs: list[Exception]): if len(excs) == 0: return "No exceptions" - if len(excs) == 1: - return f"1 failed attempt: {type(excs[0]).__name__}" - else: - return f"{len(excs)} failed attempts. Latest: {type(excs[-1]).__name__}" + plural = "s" if len(excs) > 1 else "" + return f"{len(excs)} failed attempt{plural}" def __init__(self, excs: list[Exception]): super().__init__(self._format_message(excs), excs) @@ -268,8 +309,8 @@ def __init__( failed_query: "ReadRowsQuery" | dict[str, Any], cause: Exception, ): - message = f"Failed query at index {failed_index} with cause: {cause!r}" + message = f"Failed query at index {failed_index}" super().__init__(message) + self.__cause__ = cause self.index = failed_index self.query = failed_query - self.__cause__ = cause diff --git a/tests/unit/data/test_exceptions.py b/tests/unit/data/test_exceptions.py index 9d1145e36..2defffc86 100644 --- a/tests/unit/data/test_exceptions.py +++ b/tests/unit/data/test_exceptions.py @@ -25,57 +25,68 @@ import mock # type: ignore -class TestBigtableExceptionGroup: +class TracebackTests311: """ - Subclass for MutationsExceptionGroup, RetryExceptionGroup, and ShardedReadRowsExceptionGroup + Provides a set of tests that should be run on python 3.11 and above, + to verify that the exception traceback looks as expected """ - def _get_class(self): - from google.cloud.bigtable.data.exceptions import _BigtableExceptionGroup - - return _BigtableExceptionGroup - - def _make_one(self, message="test_message", excs=None): - if excs is None: - excs = [RuntimeError("mock")] - - return self._get_class()(message, excs=excs) - - def test_raise(self): + @pytest.mark.skipif( + sys.version_info < (3, 11), reason="requires python3.11 or higher" + ) + def test_311_traceback(self): """ - Create exception in raise statement, which calls __new__ and __init__ + Exception customizations should not break rich exception group traceback in python 3.11 """ - test_msg = "test message" - test_excs = [Exception(test_msg)] - with pytest.raises(self._get_class()) as e: - raise self._get_class()(test_msg, test_excs) - assert str(e.value) == test_msg - assert list(e.value.exceptions) == test_excs + import traceback - def test_raise_empty_list(self): - """ - Empty exception lists are not supported - """ - with pytest.raises(ValueError) as e: - raise self._make_one(excs=[]) - assert "non-empty sequence" in str(e.value) + sub_exc1 = RuntimeError("first sub exception") + sub_exc2 = ZeroDivisionError("second sub exception") + sub_group = self._make_one(excs=[sub_exc2]) + exc_group = self._make_one(excs=[sub_exc1, sub_group]) + + expected_traceback = ( + f" | google.cloud.bigtable.data.exceptions.{type(exc_group).__name__}: {str(exc_group)}", + " +-+---------------- 1 ----------------", + " | RuntimeError: first sub exception", + " +---------------- 2 ----------------", + f" | google.cloud.bigtable.data.exceptions.{type(sub_group).__name__}: {str(sub_group)}", + " +-+---------------- 1 ----------------", + " | ZeroDivisionError: second sub exception", + " +------------------------------------", + ) + exception_caught = False + try: + raise exc_group + except self._get_class(): + exception_caught = True + tb = traceback.format_exc() + tb_relevant_lines = tuple(tb.splitlines()[3:]) + assert expected_traceback == tb_relevant_lines + assert exception_caught @pytest.mark.skipif( sys.version_info < (3, 11), reason="requires python3.11 or higher" ) - def test_311_traceback(self): + def test_311_traceback_with_cause(self): """ - Exception customizations should not break rich exception group traceback in python 3.11 + traceback should display nicely with sub-exceptions with __cause__ set """ import traceback sub_exc1 = RuntimeError("first sub exception") + cause_exc = ImportError("cause exception") + sub_exc1.__cause__ = cause_exc sub_exc2 = ZeroDivisionError("second sub exception") exc_group = self._make_one(excs=[sub_exc1, sub_exc2]) expected_traceback = ( f" | google.cloud.bigtable.data.exceptions.{type(exc_group).__name__}: {str(exc_group)}", " +-+---------------- 1 ----------------", + " | ImportError: cause exception", + " | ", + " | The above exception was the direct cause of the following exception:", + " | ", " | RuntimeError: first sub exception", " +---------------- 2 ----------------", " | ZeroDivisionError: second sub exception", @@ -105,6 +116,126 @@ def test_311_exception_group(self): assert runtime_error.exceptions[0] == exceptions[0] assert others.exceptions[0] == exceptions[1] + +class TracebackTests310: + """ + Provides a set of tests that should be run on python 3.10 and under, + to verify that the exception traceback looks as expected + """ + + @pytest.mark.skipif( + sys.version_info >= (3, 11), reason="requires python3.10 or lower" + ) + def test_310_traceback(self): + """ + Exception customizations should not break rich exception group traceback in python 3.10 + """ + import traceback + + sub_exc1 = RuntimeError("first sub exception") + sub_exc2 = ZeroDivisionError("second sub exception") + sub_group = self._make_one(excs=[sub_exc2]) + exc_group = self._make_one(excs=[sub_exc1, sub_group]) + found_message = str(exc_group).splitlines()[0] + found_sub_message = str(sub_group).splitlines()[0] + + expected_traceback = ( + f"google.cloud.bigtable.data.exceptions.{type(exc_group).__name__}: {found_message}", + "--+---------------- 1 ----------------", + " | RuntimeError: first sub exception", + " +---------------- 2 ----------------", + f" | {type(sub_group).__name__}: {found_sub_message}", + " --+---------------- 1 ----------------", + " | ZeroDivisionError: second sub exception", + " +------------------------------------", + ) + exception_caught = False + try: + raise exc_group + except self._get_class(): + exception_caught = True + tb = traceback.format_exc() + tb_relevant_lines = tuple(tb.splitlines()[3:]) + assert expected_traceback == tb_relevant_lines + assert exception_caught + + @pytest.mark.skipif( + sys.version_info >= (3, 11), reason="requires python3.10 or lower" + ) + def test_310_traceback_with_cause(self): + """ + traceback should display nicely with sub-exceptions with __cause__ set + """ + import traceback + + sub_exc1 = RuntimeError("first sub exception") + cause_exc = ImportError("cause exception") + sub_exc1.__cause__ = cause_exc + sub_exc2 = ZeroDivisionError("second sub exception") + exc_group = self._make_one(excs=[sub_exc1, sub_exc2]) + found_message = str(exc_group).splitlines()[0] + + expected_traceback = ( + f"google.cloud.bigtable.data.exceptions.{type(exc_group).__name__}: {found_message}", + "--+---------------- 1 ----------------", + " | ImportError: cause exception", + " | ", + " | The above exception was the direct cause of the following exception:", + " | ", + " | RuntimeError: first sub exception", + " +---------------- 2 ----------------", + " | ZeroDivisionError: second sub exception", + " +------------------------------------", + ) + exception_caught = False + try: + raise exc_group + except self._get_class(): + exception_caught = True + tb = traceback.format_exc() + tb_relevant_lines = tuple(tb.splitlines()[3:]) + assert expected_traceback == tb_relevant_lines + assert exception_caught + + +class TestBigtableExceptionGroup(TracebackTests311, TracebackTests310): + """ + Subclass for MutationsExceptionGroup, RetryExceptionGroup, and ShardedReadRowsExceptionGroup + """ + + def _get_class(self): + from google.cloud.bigtable.data.exceptions import _BigtableExceptionGroup + + return _BigtableExceptionGroup + + def _make_one(self, message="test_message", excs=None): + if excs is None: + excs = [RuntimeError("mock")] + + return self._get_class()(message, excs=excs) + + def test_raise(self): + """ + Create exception in raise statement, which calls __new__ and __init__ + """ + test_msg = "test message" + test_excs = [Exception(test_msg)] + with pytest.raises(self._get_class()) as e: + raise self._get_class()(test_msg, test_excs) + found_message = str(e.value).splitlines()[ + 0 + ] # added to prase out subexceptions in <3.11 + assert found_message == test_msg + assert list(e.value.exceptions) == test_excs + + def test_raise_empty_list(self): + """ + Empty exception lists are not supported + """ + with pytest.raises(ValueError) as e: + raise self._make_one(excs=[]) + assert "non-empty sequence" in str(e.value) + def test_exception_handling(self): """ All versions should inherit from exception @@ -151,7 +282,10 @@ def test_raise(self, exception_list, total_entries, expected_message): """ with pytest.raises(self._get_class()) as e: raise self._get_class()(exception_list, total_entries) - assert str(e.value) == expected_message + found_message = str(e.value).splitlines()[ + 0 + ] # added to prase out subexceptions in <3.11 + assert found_message == expected_message assert list(e.value.exceptions) == exception_list def test_raise_custom_message(self): @@ -162,7 +296,10 @@ def test_raise_custom_message(self): exception_list = [Exception()] with pytest.raises(self._get_class()) as e: raise self._get_class()(exception_list, 5, message=custom_message) - assert str(e.value) == custom_message + found_message = str(e.value).splitlines()[ + 0 + ] # added to prase out subexceptions in <3.11 + assert found_message == custom_message assert list(e.value.exceptions) == exception_list @pytest.mark.parametrize( @@ -222,7 +359,10 @@ def test_from_truncated_lists( raise self._get_class().from_truncated_lists( first_list, second_list, total_excs, entry_count ) - assert str(e.value) == expected_message + found_message = str(e.value).splitlines()[ + 0 + ] # added to prase out subexceptions in <3.11 + assert found_message == expected_message assert list(e.value.exceptions) == first_list + second_list @@ -241,11 +381,11 @@ def _make_one(self, excs=None): @pytest.mark.parametrize( "exception_list,expected_message", [ - ([Exception()], "1 failed attempt: Exception"), - ([Exception(), RuntimeError()], "2 failed attempts. Latest: RuntimeError"), + ([Exception()], "1 failed attempt"), + ([Exception(), RuntimeError()], "2 failed attempts"), ( [Exception(), ValueError("test")], - "2 failed attempts. Latest: ValueError", + "2 failed attempts", ), ( [ @@ -253,7 +393,7 @@ def _make_one(self, excs=None): [Exception(), ValueError("test")] ) ], - "1 failed attempt: RetryExceptionGroup", + "1 failed attempt", ), ], ) @@ -263,7 +403,10 @@ def test_raise(self, exception_list, expected_message): """ with pytest.raises(self._get_class()) as e: raise self._get_class()(exception_list) - assert str(e.value) == expected_message + found_message = str(e.value).splitlines()[ + 0 + ] # added to prase out subexceptions in <3.11 + assert found_message == expected_message assert list(e.value.exceptions) == exception_list @@ -299,7 +442,10 @@ def test_raise(self, exception_list, succeeded, total_entries, expected_message) """ with pytest.raises(self._get_class()) as e: raise self._get_class()(exception_list, succeeded, total_entries) - assert str(e.value) == expected_message + found_message = str(e.value).splitlines()[ + 0 + ] # added to prase out subexceptions in <3.11 + assert found_message == expected_message assert list(e.value.exceptions) == exception_list assert e.value.successful_rows == succeeded @@ -323,10 +469,7 @@ def test_raise(self): test_exc = ValueError("test") with pytest.raises(self._get_class()) as e: raise self._get_class()(test_idx, test_entry, test_exc) - assert ( - str(e.value) - == "Failed idempotent mutation entry at index 2 with cause: ValueError('test')" - ) + assert str(e.value) == "Failed idempotent mutation entry at index 2" assert e.value.index == test_idx assert e.value.entry == test_entry assert e.value.__cause__ == test_exc @@ -343,10 +486,7 @@ def test_raise_idempotent(self): test_exc = ValueError("test") with pytest.raises(self._get_class()) as e: raise self._get_class()(test_idx, test_entry, test_exc) - assert ( - str(e.value) - == "Failed non-idempotent mutation entry at index 2 with cause: ValueError('test')" - ) + assert str(e.value) == "Failed non-idempotent mutation entry at index 2" assert e.value.index == test_idx assert e.value.entry == test_entry assert e.value.__cause__ == test_exc @@ -361,10 +501,7 @@ def test_no_index(self): test_exc = ValueError("test") with pytest.raises(self._get_class()) as e: raise self._get_class()(test_idx, test_entry, test_exc) - assert ( - str(e.value) - == "Failed idempotent mutation entry with cause: ValueError('test')" - ) + assert str(e.value) == "Failed idempotent mutation entry" assert e.value.index == test_idx assert e.value.entry == test_entry assert e.value.__cause__ == test_exc @@ -391,7 +528,7 @@ def test_raise(self): test_exc = ValueError("test") with pytest.raises(self._get_class()) as e: raise self._get_class()(test_idx, test_query, test_exc) - assert str(e.value) == "Failed query at index 2 with cause: ValueError('test')" + assert str(e.value) == "Failed query at index 2" assert e.value.index == test_idx assert e.value.query == test_query assert e.value.__cause__ == test_exc