From c0148c0a38d356a3331761244a2cb8234c1310c4 Mon Sep 17 00:00:00 2001 From: Oliver Date: Tue, 7 Jun 2022 08:58:00 +1000 Subject: [PATCH] Ensure an error gets logged when a delivery method fails (#3144) * Ensure an error gets logged when a delivery method fails - Refactor existing code to log a custom error to the database - Limit error notifications to UI * Adjust unit test * Clear existing notifications before run * Add some debug to work out what is going on * Accommodate extra notification --- InvenTree/InvenTree/exceptions.py | 29 +++++++++++++++++----------- InvenTree/InvenTree/models.py | 1 + InvenTree/order/models.py | 20 +++---------------- InvenTree/part/test_part.py | 6 ++++-- InvenTree/plugin/base/label/label.py | 16 ++------------- InvenTree/plugin/views.py | 18 ++--------------- 6 files changed, 30 insertions(+), 60 deletions(-) diff --git a/InvenTree/InvenTree/exceptions.py b/InvenTree/InvenTree/exceptions.py index 2b64fd8b64..ce79449c53 100644 --- a/InvenTree/InvenTree/exceptions.py +++ b/InvenTree/InvenTree/exceptions.py @@ -9,7 +9,6 @@ import traceback from django.conf import settings from django.core.exceptions import ValidationError as DjangoValidationError from django.utils.translation import gettext_lazy as _ -from django.views.debug import ExceptionReporter import rest_framework.views as drfviews from error_report.models import Error @@ -18,6 +17,23 @@ from rest_framework.exceptions import ValidationError as DRFValidationError from rest_framework.response import Response +def log_error(path): + """Log an error to the database. + + - Uses python exception handling to extract error details + + Arguments: + path: The 'path' (most likely a URL) associated with this error (optional) + """ + kind, info, data = sys.exc_info() + Error.objects.create( + kind=kind.__name__, + info=info, + data='\n'.join(traceback.format_exception(kind, info, data)), + path=path, + ) + + def exception_handler(exc, context): """Custom exception handler for DRF framework. @@ -55,16 +71,7 @@ def exception_handler(exc, context): response = Response(response_data, status=500) - # Log the exception to the database, too - kind, info, data = sys.exc_info() - - Error.objects.create( - kind=kind.__name__, - info=info, - data='\n'.join(traceback.format_exception(kind, info, data)), - path=context['request'].path, - html=ExceptionReporter(context['request'], kind, info, data).get_traceback_html(), - ) + log_error(context['request'].path) if response is not None: # Convert errors returned under the label '__all__' to 'non_field_errors' diff --git a/InvenTree/InvenTree/models.py b/InvenTree/InvenTree/models.py index 90d8a0314b..eaa8ef3b0d 100644 --- a/InvenTree/InvenTree/models.py +++ b/InvenTree/InvenTree/models.py @@ -475,6 +475,7 @@ def after_error_logged(sender, instance: Error, created: bool, **kwargs): 'inventree.error_log', context=context, targets=users, + delivery_methods=set([common.notifications.UIMessageNotification]), ) except Exception as exc: diff --git a/InvenTree/order/models.py b/InvenTree/order/models.py index be96a8fa2b..bdced458c0 100644 --- a/InvenTree/order/models.py +++ b/InvenTree/order/models.py @@ -3,7 +3,6 @@ import logging import os import sys -import traceback from datetime import datetime from decimal import Decimal @@ -21,7 +20,6 @@ from django.utils.translation import gettext_lazy as _ from djmoney.contrib.exchange.exceptions import MissingRate from djmoney.contrib.exchange.models import convert_money from djmoney.money import Money -from error_report.models import Error from markdownx.models import MarkdownxField from mptt.models import TreeForeignKey @@ -29,6 +27,7 @@ import InvenTree.helpers import InvenTree.ready from common.settings import currency_code_default from company.models import Company, SupplierPart +from InvenTree.exceptions import log_error from InvenTree.fields import InvenTreeModelMoneyField, RoundingDecimalField from InvenTree.helpers import (decimal2string, getSetting, increment, notify_responsible) @@ -186,13 +185,7 @@ class Order(MetadataMixin, ReferenceIndexingMixin): # Record the error, try to press on kind, info, data = sys.exc_info() - Error.objects.create( - kind=kind.__name__, - info=info, - data='\n'.join(traceback.format_exception(kind, info, data)), - path='order.get_total_price', - ) - + log_error('order.get_total_price') logger.error(f"Missing exchange rate for '{target_currency}'") # Return None to indicate the calculated price is invalid @@ -208,15 +201,8 @@ class Order(MetadataMixin, ReferenceIndexingMixin): total += line.quantity * convert_money(line.price, target_currency) except MissingRate: # Record the error, try to press on - kind, info, data = sys.exc_info() - - Error.objects.create( - kind=kind.__name__, - info=info, - data='\n'.join(traceback.format_exception(kind, info, data)), - path='order.get_total_price', - ) + log_error('order.get_total_price') logger.error(f"Missing exchange rate for '{target_currency}'") # Return None to indicate the calculated price is invalid diff --git a/InvenTree/part/test_part.py b/InvenTree/part/test_part.py index d11cc5d142..ac03baf0a5 100644 --- a/InvenTree/part/test_part.py +++ b/InvenTree/part/test_part.py @@ -616,6 +616,8 @@ class BaseNotificationIntegrationTest(InvenTreeTestCase): # reload notification methods storage.collect(run_class) + NotificationEntry.objects.all().delete() + # There should be no notification runs self.assertEqual(NotificationEntry.objects.all().count(), 0) @@ -630,8 +632,8 @@ class BaseNotificationIntegrationTest(InvenTreeTestCase): self.part.set_starred(self.user, True) self.part.save() - # There should be 1 notification - self.assertEqual(NotificationEntry.objects.all().count(), 1) + # There should be 1 (or 2) notifications - in some cases an error is generated, which creates a subsequent notification + self.assertIn(NotificationEntry.objects.all().count(), [1, 2]) class PartNotificationTest(BaseNotificationIntegrationTest): diff --git a/InvenTree/plugin/base/label/label.py b/InvenTree/plugin/base/label/label.py index 6880a8b3e6..5baf0ec872 100644 --- a/InvenTree/plugin/base/label/label.py +++ b/InvenTree/plugin/base/label/label.py @@ -1,17 +1,14 @@ """Functions to print a label to a mixin printer.""" import logging -import sys -import traceback from django.conf import settings from django.utils.translation import gettext_lazy as _ -from django.views.debug import ExceptionReporter import pdf2image -from error_report.models import Error import common.notifications +from InvenTree.exceptions import log_error from plugin.registry import registry logger = logging.getLogger('inventree') @@ -63,16 +60,7 @@ def print_label(plugin_slug: str, pdf_data, filename=None, label_instance=None, } # Log an error message to the database - kind, info, data = sys.exc_info() - - Error.objects.create( - kind=kind.__name__, - info=info, - data='\n'.join(traceback.format_exception(kind, info, data)), - path='print_label', - html=ExceptionReporter(None, kind, info, data).get_traceback_html(), - ) - + log_error('plugin.print_label') logger.error(f"Label printing failed: Sending notification to user '{user}'") # pragma: no cover # Throw an error against the plugin instance diff --git a/InvenTree/plugin/views.py b/InvenTree/plugin/views.py index aa43b9bd09..2073f25422 100644 --- a/InvenTree/plugin/views.py +++ b/InvenTree/plugin/views.py @@ -1,14 +1,10 @@ """Views for plugin app.""" import logging -import sys -import traceback from django.conf import settings -from django.views.debug import ExceptionReporter - -from error_report.models import Error +from InvenTree.exceptions import log_error from plugin.registry import registry logger = logging.getLogger('inventree') @@ -29,18 +25,8 @@ class InvenTreePluginViewMixin: try: panels += plug.render_panels(self, self.request, ctx) except Exception: - # Prevent any plugin error from crashing the page render - kind, info, data = sys.exc_info() - # Log the error to the database - Error.objects.create( - kind=kind.__name__, - info=info, - data='\n'.join(traceback.format_exception(kind, info, data)), - path=self.request.path, - html=ExceptionReporter(self.request, kind, info, data).get_traceback_html(), - ) - + log_error(self.request.path) logger.error(f"Plugin '{plug.slug}' could not render custom panels at '{self.request.path}'") return panels