From 0cfda341b47ac1e1a52b0805cbd9e6add931fd70 Mon Sep 17 00:00:00 2001 From: Tom McCarthy Date: Mon, 25 Jan 2021 17:35:14 +0100 Subject: [PATCH 1/5] fix: Add tracebacks for all caught exceptions to SQSBatchProcessingError output --- aws_lambda_powertools/utilities/batch/base.py | 7 ++++--- .../utilities/batch/exceptions.py | 21 +++++++++++++++++++ aws_lambda_powertools/utilities/batch/sqs.py | 11 +++++++--- tests/functional/test_utilities_batch.py | 10 ++++----- 4 files changed, 38 insertions(+), 11 deletions(-) diff --git a/aws_lambda_powertools/utilities/batch/base.py b/aws_lambda_powertools/utilities/batch/base.py index 38fc1ca75fc..2a7a956a19d 100644 --- a/aws_lambda_powertools/utilities/batch/base.py +++ b/aws_lambda_powertools/utilities/batch/base.py @@ -85,7 +85,7 @@ def success_handler(self, record: Any, result: Any): self.success_messages.append(record) return entry - def failure_handler(self, record: Any, exception: Exception): + def failure_handler(self, record: Any, exception: Tuple): """ Failure callback @@ -94,8 +94,9 @@ def failure_handler(self, record: Any, exception: Exception): tuple "fail", exceptions args, original record """ - entry = ("fail", exception.args, record) - logger.debug(f"Record processing exception: {exception}") + exception_string = f"{exception[0]}:{exception[1]}" + entry = ("fail", exception_string, record) + logger.debug(f"Record processing exception: {exception_string}") self.exceptions.append(exception) self.fail_messages.append(record) return entry diff --git a/aws_lambda_powertools/utilities/batch/exceptions.py b/aws_lambda_powertools/utilities/batch/exceptions.py index 3e456eacec4..c9fd9f81b15 100644 --- a/aws_lambda_powertools/utilities/batch/exceptions.py +++ b/aws_lambda_powertools/utilities/batch/exceptions.py @@ -1,7 +1,28 @@ """ Batch processing exceptions """ +import traceback class SQSBatchProcessingError(Exception): """When at least one message within a batch could not be processed""" + + def __init__(self, msg="", child_exceptions=()): + super().__init__(msg) + self.msg = msg + self.child_exceptions = child_exceptions + + def __str__(self): + parent_exception_str = super(SQSBatchProcessingError, self).__str__() + exception_list = [f"{parent_exception_str}\n"] + + for exception in self.child_exceptions: + extype, ex, tb = exception + formatted = "".join(traceback.format_exception(extype, ex, tb)) + exception_list.append(formatted) + + return "\n".join(exception_list) + + def log(self): + for exc in self.child_exceptions: + print(exc) diff --git a/aws_lambda_powertools/utilities/batch/sqs.py b/aws_lambda_powertools/utilities/batch/sqs.py index 7da8ab52288..380543450ec 100644 --- a/aws_lambda_powertools/utilities/batch/sqs.py +++ b/aws_lambda_powertools/utilities/batch/sqs.py @@ -4,6 +4,7 @@ Batch SQS utilities """ import logging +import sys from typing import Callable, Dict, List, Optional, Tuple import boto3 @@ -92,8 +93,8 @@ def _process_record(self, record) -> Tuple: try: result = self.handler(record) return self.success_handler(record, result) - except Exception as exc: - return self.failure_handler(record, exc) + except Exception: + return self.failure_handler(record, sys.exc_info()) def _prepare(self): """ @@ -123,7 +124,11 @@ def _clean(self): logger.debug(f"{len(self.fail_messages)} records failed processing, but exceptions are suppressed") else: logger.debug(f"{len(self.fail_messages)} records failed processing, raising exception") - raise SQSBatchProcessingError(list(self.exceptions)) + raise SQSBatchProcessingError( + msg=f"Not all records processed succesfully. {len(self.exceptions)} individual errors logged " + f"separately below.", + child_exceptions=self.exceptions, + ) return delete_message_response diff --git a/tests/functional/test_utilities_batch.py b/tests/functional/test_utilities_batch.py index 234f90e9f16..a453f0bfe07 100644 --- a/tests/functional/test_utilities_batch.py +++ b/tests/functional/test_utilities_batch.py @@ -85,7 +85,7 @@ def test_partial_sqs_processor_context_with_failure(sqs_event_factory, record_ha with partial_processor(records, record_handler) as ctx: ctx.process() - assert len(error.value.args[0]) == 1 + assert len(error.value.child_exceptions) == 1 stubber.assert_no_pending_responses() @@ -144,7 +144,7 @@ def lambda_handler(event, context): with pytest.raises(SQSBatchProcessingError) as error: lambda_handler(event, {}) - assert len(error.value.args[0]) == 2 + assert len(error.value.child_exceptions) == 2 stubber.assert_no_pending_responses() @@ -171,7 +171,7 @@ def lambda_handler(event, context): with pytest.raises(SQSBatchProcessingError) as error: lambda_handler(event, {}) - assert len(error.value.args[0]) == 1 + assert len(error.value.child_exceptions) == 1 stubber.assert_no_pending_responses() @@ -203,7 +203,7 @@ def lambda_handler(event, context): stubber.assert_no_pending_responses() - assert len(error.value.args[0]) == 1 + assert len(error.value.child_exceptions) == 1 assert capsys.readouterr().out == "Oh no ! It's a failure.\n" @@ -289,4 +289,4 @@ def test_partial_sqs_processor_context_only_failure(sqs_event_factory, record_ha with partial_processor(records, record_handler) as ctx: ctx.process() - assert len(error.value.args[0]) == 2 + assert len(error.value.child_exceptions) == 2 From 9ca591f6e06cf4d104f8dbbc53169894cce38c19 Mon Sep 17 00:00:00 2001 From: Tom McCarthy Date: Mon, 25 Jan 2021 17:43:55 +0100 Subject: [PATCH 2/5] chore: Remove debugging code --- aws_lambda_powertools/utilities/batch/exceptions.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/aws_lambda_powertools/utilities/batch/exceptions.py b/aws_lambda_powertools/utilities/batch/exceptions.py index c9fd9f81b15..70dff287ee3 100644 --- a/aws_lambda_powertools/utilities/batch/exceptions.py +++ b/aws_lambda_powertools/utilities/batch/exceptions.py @@ -15,14 +15,9 @@ def __init__(self, msg="", child_exceptions=()): def __str__(self): parent_exception_str = super(SQSBatchProcessingError, self).__str__() exception_list = [f"{parent_exception_str}\n"] - for exception in self.child_exceptions: extype, ex, tb = exception formatted = "".join(traceback.format_exception(extype, ex, tb)) exception_list.append(formatted) return "\n".join(exception_list) - - def log(self): - for exc in self.child_exceptions: - print(exc) From 12f1cbe4a7c8838298898a6775706fb43a5733c7 Mon Sep 17 00:00:00 2001 From: Tom McCarthy Date: Tue, 26 Jan 2021 16:05:13 +0100 Subject: [PATCH 3/5] chore: fix unit test to account for change in exception passing --- tests/unit/test_utilities_batch.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_utilities_batch.py b/tests/unit/test_utilities_batch.py index 136e6ff2e8c..2e4633eacfc 100644 --- a/tests/unit/test_utilities_batch.py +++ b/tests/unit/test_utilities_batch.py @@ -99,8 +99,12 @@ def test_partial_sqs_process_record_failure(mocker, partial_sqs_processor): result = partial_sqs_processor._process_record(record) handler_mock.assert_called_once_with(record) - failure_handler_mock.assert_called_once_with(record, failure_result) + failure_handler_called_with_args, _ = failure_handler_mock.call_args + failure_handler_mock.assert_called_once() + assert (failure_handler_called_with_args[0]) == record + assert isinstance(failure_handler_called_with_args[1], tuple) + assert failure_handler_called_with_args[1][1] == failure_result assert result == expected_value From 1f0f60560c87b67fdd07110815f6c39c3d61da72 Mon Sep 17 00:00:00 2001 From: Tom McCarthy Date: Tue, 2 Feb 2021 17:52:16 +0100 Subject: [PATCH 4/5] chore: use kwargs in method calls, add comment to SQSBatchProcessingError --- aws_lambda_powertools/utilities/batch/exceptions.py | 2 ++ aws_lambda_powertools/utilities/batch/sqs.py | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/aws_lambda_powertools/utilities/batch/exceptions.py b/aws_lambda_powertools/utilities/batch/exceptions.py index 70dff287ee3..c2ead04a7b1 100644 --- a/aws_lambda_powertools/utilities/batch/exceptions.py +++ b/aws_lambda_powertools/utilities/batch/exceptions.py @@ -12,6 +12,8 @@ def __init__(self, msg="", child_exceptions=()): self.msg = msg self.child_exceptions = child_exceptions + # Overriding this method so we can output all child exception tracebacks when we raise this exception to prevent + # errors being lost. See https://github.com/awslabs/aws-lambda-powertools-python/issues/275 def __str__(self): parent_exception_str = super(SQSBatchProcessingError, self).__str__() exception_list = [f"{parent_exception_str}\n"] diff --git a/aws_lambda_powertools/utilities/batch/sqs.py b/aws_lambda_powertools/utilities/batch/sqs.py index 380543450ec..82c24ece10a 100644 --- a/aws_lambda_powertools/utilities/batch/sqs.py +++ b/aws_lambda_powertools/utilities/batch/sqs.py @@ -91,10 +91,10 @@ def _process_record(self, record) -> Tuple: An object to be processed. """ try: - result = self.handler(record) - return self.success_handler(record, result) + result = self.handler(record=record) + return self.success_handler(record=record, result=result) except Exception: - return self.failure_handler(record, sys.exc_info()) + return self.failure_handler(record=record, exception=sys.exc_info()) def _prepare(self): """ From 383e8af3e43d91117aa4969cbe7062f70d8baff2 Mon Sep 17 00:00:00 2001 From: Tom McCarthy Date: Tue, 2 Feb 2021 17:58:37 +0100 Subject: [PATCH 5/5] chore: Fix tests after changing method calls to use kwargs --- tests/unit/test_utilities_batch.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/unit/test_utilities_batch.py b/tests/unit/test_utilities_batch.py index 2e4633eacfc..c491f0829cb 100644 --- a/tests/unit/test_utilities_batch.py +++ b/tests/unit/test_utilities_batch.py @@ -81,8 +81,8 @@ def test_partial_sqs_process_record_success(mocker, partial_sqs_processor): result = partial_sqs_processor._process_record(record) - handler_mock.assert_called_once_with(record) - success_handler_mock.assert_called_once_with(record, success_result) + handler_mock.assert_called_once_with(record=record) + success_handler_mock.assert_called_once_with(record=record, result=success_result) assert result == expected_value @@ -98,13 +98,13 @@ def test_partial_sqs_process_record_failure(mocker, partial_sqs_processor): result = partial_sqs_processor._process_record(record) - handler_mock.assert_called_once_with(record) + handler_mock.assert_called_once_with(record=record) - failure_handler_called_with_args, _ = failure_handler_mock.call_args + _, failure_handler_called_with_args = failure_handler_mock.call_args failure_handler_mock.assert_called_once() - assert (failure_handler_called_with_args[0]) == record - assert isinstance(failure_handler_called_with_args[1], tuple) - assert failure_handler_called_with_args[1][1] == failure_result + assert (failure_handler_called_with_args["record"]) == record + assert isinstance(failure_handler_called_with_args["exception"], tuple) + assert failure_handler_called_with_args["exception"][1] == failure_result assert result == expected_value