From 6971460b9adc0507786369b90e93b742d28e632c Mon Sep 17 00:00:00 2001 From: Thomas NOEL Date: Thu, 7 Jun 2018 12:16:20 +0200 Subject: [PATCH] misc: only log missing statuses as functional errors (#24327) --- tests/test_form_pages.py | 27 ++++++++++++- wcs/admin/logged_errors.py | 21 +++++++++-- wcs/logged_errors.py | 77 +++++++++++++++++++++++++++++++------- wcs/publisher.py | 2 +- wcs/workflows.py | 12 +++--- 5 files changed, 113 insertions(+), 26 deletions(-) diff --git a/tests/test_form_pages.py b/tests/test_form_pages.py index cda29638..d8c7282d 100644 --- a/tests/test_form_pages.py +++ b/tests/test_form_pages.py @@ -1694,7 +1694,9 @@ def test_form_multi_page_post_edit(pub): assert 'barXYZ' in resp.body # unchanged value is still there assert formdef.data_class().get(data_id).status == 'wf-%s' % st2.id - # jump to a nonexistent status == do not jump + # jump to a nonexistent status == do not jump, but add a LoggedError + LoggedError.wipe() + assert LoggedError.count() == 0 editable.status = 'deleted_status_id' workflow.store() # go back to st1 @@ -1710,6 +1712,27 @@ def test_form_multi_page_post_edit(pub): resp = resp.forms[0].submit('submit') resp = resp.follow() assert formdef.data_class().get(data_id).status == 'wf-%s' % st1.id # stay on st1 + assert LoggedError.count() == 1 + logged_error = LoggedError.select()[0] + assert logged_error.formdata_id == str(formdata.id) + assert logged_error.formdef_id == formdef.id + assert logged_error.workflow_id == workflow.id + assert logged_error.status_id == st1.id + assert logged_error.status_item_id == editable.id + assert logged_error.occurences_count == 1 + + # do it again: increment logged_error.occurences_count + page = login(get_app(pub), username='foo', password='foo').get('/test/%s/' % data_id) + resp = page.forms[0].submit('button_editable') + resp = resp.follow() + resp.forms[0]['f1'] = 'foo3' + resp = resp.forms[0].submit('submit') + resp = resp.forms[0].submit('submit') + resp = resp.follow() + assert formdef.data_class().get(data_id).status == 'wf-%s' % st1.id # stay on st1 + assert LoggedError.count() == 1 + logged_error = LoggedError.select()[0] + assert logged_error.occurences_count == 2 def test_form_count_dispatching(pub): user = create_user(pub) @@ -4359,7 +4382,7 @@ def test_logged_errors(pub): assert LoggedError.count() == 1 error = LoggedError.get_on_index( - '34-12-zerodivisionerror-integer-division-or-modulo-by-zero', 'tech_id') + '34-12-None-None-zerodivisionerror-integer-division-or-modulo-by-zero', 'tech_id') assert error.occurences_count == 2 assert len(LoggedError.get_ids_with_indexed_value('formdef_id', '34')) == 1 diff --git a/wcs/admin/logged_errors.py b/wcs/admin/logged_errors.py index c2373925..4ddf1cdb 100644 --- a/wcs/admin/logged_errors.py +++ b/wcs/admin/logged_errors.py @@ -56,6 +56,19 @@ class LoggedErrorDirectory(Directory): if workflow: r += htmltext('
  • %s %s
    • ') % ( _('Workflow: '), workflow.id, workflow.name) + status = self.error.get_status() + if status: + r += htmltext('') if formdef: formdata = self.error.get_formdata() @@ -68,6 +81,10 @@ class LoggedErrorDirectory(Directory): r += htmltext('') r += htmltext('') + + if not self.error.traceback: + return r.getvalue() + parts = (N_('Exception'), N_('Stack trace (most recent call first)'), N_('Form'), N_('Cookies'), N_('Environment')) current_part = None @@ -105,10 +122,6 @@ class LoggedErrorDirectory(Directory): r += htmltext('
  • %s
    • ' % _('Acked')) r += htmltext('
  • %s
    • ') % _('Delete') r += htmltext('') - if get_publisher().logger.error_email: - r += htmltext('

    ') - r += _('This error has also been sent by email to %s.') % get_publisher().logger.error_email - r += htmltext('

    ') return r.getvalue() def ack(self): diff --git a/wcs/logged_errors.py b/wcs/logged_errors.py index 39e568fb..4030d2b6 100644 --- a/wcs/logged_errors.py +++ b/wcs/logged_errors.py @@ -31,6 +31,8 @@ class LoggedError(XmlStorableObject): formdata_id = None formdef_id = None workflow_id = None + status_id = None + status_item_id = None traceback = None occurences_count = 0 first_occurence_timestamp = None @@ -41,32 +43,41 @@ class LoggedError(XmlStorableObject): XML_NODES = [ ('summary', 'str'), ('traceback', 'str'), ('formdata_id', 'str'), ('formdef_id', 'str'), ('workflow_id', 'str'), + ('status_id', 'str'), ('status_item_id', 'str'), ('occurences_count', 'int'), ('first_occurence_timestamp', 'datetime'), ('latest_occurence_timestamp', 'datetime'), ('acked', 'bool')] @classmethod - def record(cls, error_summary, plain_error_msg, publisher): + def record(cls, error_summary, plain_error_msg=None, formdata=None, + formdef=None, workflow=None, status=None, status_item=None): error = cls() error.summary = error_summary error.traceback = plain_error_msg - try: - context = publisher.substitutions.get_context_variables() - except: - return - formdef_urlname = context.get('form_slug') - if not formdef_urlname: + if formdata: + error.formdata_id = str(formdata.id) + error.formdef_id = formdata.formdef.id + error.workflow_id = formdata.formdef.workflow.id + elif formdef: + error.formdef_id = formdef.id + error.workflow_id = formdef.workflow.id + + if not error.formdef_id: # cannot attach error to formdef, don't record in journal, it will # still be sent by email to administrators. return - error.formdata_id = context.get('form_number_raw') - if formdef_urlname: - formdef = FormDef.get_by_urlname(formdef_urlname) - error.formdef_id = formdef.id - error.workflow_id = formdef.workflow_id + if not error.workflow_id: + error.workflow_id = workflow.id + + if status_item: + error.status_item_id = status_item.id + if status_item.parent: + error.status_id = status_item.parent.id + if status: + error.status_id = status.id error.first_occurence_timestamp = datetime.datetime.now() error.id = '%s-%s' % ( @@ -80,9 +91,27 @@ class LoggedError(XmlStorableObject): error.latest_occurence_timestamp = datetime.datetime.now() error.store() + @classmethod + def record_exception(cls, error_summary, plain_error_msg, publisher): + try: + context = publisher.substitutions.get_context_variables() + except Exception as e: + return + formdata_id = context.get('form_number_raw') + formdef_urlname = context.get('form_slug') + if not formdef_urlname: + # cannot attach error to formdef, don't record in journal, it will + # still be sent by email to administrators. + return + formdef = FormDef.get_by_urlname(formdef_urlname) + formdata = formdef.data_class().get(formdata_id, ignore_errors=True) + cls.record(error_summary, plain_error_msg, formdata=formdata, + formdef=formdef, workflow=formdef.workflow) + @property def tech_id(self): - return ('%s-%s-%s' % (self.formdef_id, self.workflow_id, simplify(self.summary)))[:200] + return ('%s-%s-%s-%s-%s' % (self.formdef_id, self.workflow_id, self.status_id, + self.status_item_id, simplify(self.summary)))[:200] def get_formdef(self): return FormDef.get(self.formdef_id, ignore_errors=True) @@ -91,4 +120,26 @@ class LoggedError(XmlStorableObject): return Workflow.get(self.workflow_id, ignore_errors=True) def get_formdata(self): + if not self.formdata_id: + return None return self.get_formdef().data_class().get(self.formdata_id, ignore_errors=True) + + def get_status(self): + if not self.status_id: + return None + workflow = self.get_workflow() + if not workflow: + return None + for status in workflow.possible_status: + if status.id == self.status_id: + return status + return None + + def get_status_item(self): + status = self.get_status() + if not status or not status.items: + return None + for status_item in status.items: + if status_item.id == self.status_item_id: + return status_item + return None diff --git a/wcs/publisher.py b/wcs/publisher.py index 2a1e49c6..a30c221b 100644 --- a/wcs/publisher.py +++ b/wcs/publisher.py @@ -287,7 +287,7 @@ class WcsPublisher(StubWcsPublisher): super(WcsPublisher, self).log_internal_error(error_summary, plain_error_msg, record=record) if record: - LoggedError.record(error_summary, plain_error_msg, publisher=self) + LoggedError.record_exception(error_summary, plain_error_msg, publisher=self) def apply_global_action_timeouts(self): from wcs.workflows import Workflow, WorkflowGlobalActionTimeoutTrigger diff --git a/wcs/workflows.py b/wcs/workflows.py index bc064e5f..0c65506f 100644 --- a/wcs/workflows.py +++ b/wcs/workflows.py @@ -1725,12 +1725,12 @@ class WorkflowStatusItem(XmlSerialisable): return [] targets = [x for x in self.parent.parent.possible_status if x.id == self.status] - if not targets: - get_publisher().get_app_logger().error( - 'reference to invalid status in workflow %r, status %r, item %r' % ( - self.parent.parent.name, - self.parent.name, - self.description)) + if not targets and formdata: # do not log in presentation context: formdata is needed + from wcs.logged_errors import LoggedError + message = _('reference to invalid status %s in status %s, action %s') % ( + self.status, self.parent.name, _(self.description)) + LoggedError.record(message, formdata=formdata, status_item=self) + return targets def get_jump_label(self): -- 2.17.1