general: store/display error context stack (#74791)

tests/backoffice_pages/test_all.py

@@ -5123,7 +5123,8 @@ def test_backoffice_logged_errors(pub):
     assert 'ZeroDivisionError' in resp2.text
     resp = resp2.click('Failed to evaluate condition')
     assert 'ZeroDivisionError: integer division or modulo by zero' in resp.text
-    assert 'Python Expression: <code>1//0</code>' in resp.text
+    assert 'Condition: <code>1//0</code>' in resp.text
+    assert 'Condition type: <code>python</code>' in resp.text
     resp = resp.click('Delete').follow()
     assert pub.loggederror_class.count() == 0
 

tests/form_pages/test_all.py

@@ -3408,7 +3408,16 @@ def test_logged_errors(pub):
         )
     )[0]
     assert error.occurences_count == 2
-    assert error.expression == '2//0'
+    assert error.context == {
+        'stack': [
+            {
+                'condition': '2//0',
+                'condition_type': 'python',
+                'source_label': 'Automatic Jump',
+                'source_url': 'http://example.net/backoffice/workflows/12/status/just_submitted/items/_jump/',
+            }
+        ]
+    }
 
     assert pub.loggederror_class.count([Equal('formdef_id', '34')]) == 1
     assert pub.loggederror_class.count([Equal('formdef_id', 'X')]) == 0

tests/form_pages/test_formdata.py

@@ -1590,6 +1590,57 @@ def test_formdata_named_wscall_in_conditions(http_requests, pub):
     assert http_requests.count() == 1
 
 
+def test_formdata_error_with_wscall_in_conditions(http_requests, pub):
+    create_user(pub)
+    NamedWsCall.wipe()
+
+    wscall = NamedWsCall()
+    wscall.name = 'Hello world'
+    wscall.request = {'url': 'http://remote.example.net/404', 'method': 'GET'}
+    wscall.record_on_errors = True
+    wscall.store()
+    assert wscall.slug == 'hello_world'
+
+    FormDef.wipe()
+    formdef = FormDef()
+    formdef.name = 'test'
+    formdef.fields = [
+        fields.PageField(id='0', label='1st page'),
+        fields.PageField(
+            id='1',
+            label='2nd page',
+            condition={'type': 'python', 'value': 'webservice.hello_world["foo"] == "bar"'},
+        ),
+    ]
+    formdef.store()
+    formdef.data_class().wipe()
+
+    pub.loggederror_class.wipe()
+    resp = login(get_app(pub), username='foo', password='foo').get('/test/')
+    assert '>1st page<' in resp.text
+    assert '>2nd page<' in resp.text
+
+    # condition error and wscall error
+    assert pub.loggederror_class.count() == 2
+    wscall_error, condition_error = pub.loggederror_class.select(order_by='id')
+    assert (
+        wscall_error.context
+        == condition_error.context
+        == {
+            'stack': [
+                {
+                    'condition': 'webservice.hello_world["foo"] == "bar"',
+                    'condition_type': 'python',
+                    'source_label': 'Field: 2nd page',
+                    'source_url': 'http://example.net/backoffice/forms/1/fields/1/',
+                }
+            ]
+        }
+    )
+    assert wscall_error.summary == '[WSCALL] 404 Not Found'
+    assert condition_error.summary == 'Failed to evaluate condition'
+
+
 def test_formdata_named_wscall_in_comment(pub):
     create_user(pub)
     NamedWsCall.wipe()

tests/workflow/test_all.py

@@ -1964,6 +1964,7 @@ def test_redirect_to_url(pub):
 
 
 def test_workflow_action_condition(pub):
+    Workflow.wipe()
     workflow = Workflow(name='jump condition migration')
     st1 = workflow.add_status('Status1', 'st1')
     workflow.store()
@@ -2046,8 +2047,16 @@ def test_workflow_action_condition(pub):
     assert logged_error.summary == 'Failed to evaluate condition'
     assert logged_error.exception_class == 'NameError'
     assert logged_error.exception_message == "name 'foobar' is not defined"
-    assert logged_error.expression == 'foobar == barfoo'
-    assert logged_error.expression_type == 'python'
+    assert logged_error.context == {
+        'stack': [
+            {
+                'condition': 'foobar == barfoo',
+                'condition_type': 'python',
+                'source_label': 'Manual Jump',
+                'source_url': 'http://example.net/backoffice/workflows/1/status/st1/items/_x/',
+            }
+        ]
+    }
 
 
 def test_workflow_field_migration(pub):

tests/workflow/test_jump.py

@@ -158,8 +158,16 @@ def test_jump_bad_python_condition(pub):
     assert logged_error.summary == 'Failed to evaluate condition'
     assert logged_error.exception_class == 'NameError'
     assert logged_error.exception_message == "name 'form_var_foobar' is not defined"
-    assert logged_error.expression == 'form_var_foobar == 0'
-    assert logged_error.expression_type == 'python'
+    assert logged_error.context == {
+        'stack': [
+            {
+                'condition': 'form_var_foobar == 0',
+                'condition_type': 'python',
+                'source_label': 'Automatic Jump',
+                'source_url': '',
+            }
+        ]
+    }
 
     pub.loggederror_class.wipe()
     item.condition = {'type': 'python', 'value': '~ invalid ~'}
@@ -169,8 +177,16 @@ def test_jump_bad_python_condition(pub):
     assert logged_error.summary == 'Failed to evaluate condition'
     assert logged_error.exception_class == 'SyntaxError'
     assert logged_error.exception_message == 'invalid syntax (<string>, line 1)'
-    assert logged_error.expression == '~ invalid ~'
-    assert logged_error.expression_type == 'python'
+    assert logged_error.context == {
+        'stack': [
+            {
+                'condition': '~ invalid ~',
+                'source_url': '',
+                'source_label': 'Automatic Jump',
+                'condition_type': 'python',
+            }
+        ]
+    }
 
 
 def test_jump_django_conditions(pub):
@@ -207,8 +223,16 @@ def test_jump_django_conditions(pub):
     assert logged_error.summary == 'Failed to evaluate condition'
     assert logged_error.exception_class == 'TemplateSyntaxError'
     assert logged_error.exception_message == "Could not parse the remainder: '~' from '~'"
-    assert logged_error.expression == '~ invalid ~'
-    assert logged_error.expression_type == 'django'
+    assert logged_error.context == {
+        'stack': [
+            {
+                'condition': '~ invalid ~',
+                'source_url': '',
+                'source_label': 'Automatic Jump',
+                'condition_type': 'django',
+            }
+        ]
+    }
 
 
 def test_timeout(pub):

wcs/admin/logged_errors.py

@@ -28,6 +28,28 @@ from wcs.qommon.form import CheckboxesWidget, DateWidget, Form
 from wcs.sql_criterias import Equal, Less, NotEqual, NotNull, Null, Or
 
 
+class ErrorFrame:
+    def __init__(self, context):
+        self.context = context or {}
+
+    def source(self):
+        if self.context.get('source_url'):
+            return {
+                'url': self.context.get('source_url'),
+                'label': self.context.get('source_label'),
+            }
+        return None
+
+    def get_frame_lines(self):
+        for key, value in self.context.items():
+            key_label = {
+                'condition': _('Condition'),
+                'condition_type': _('Condition type'),
+            }.get(key)
+            if key_label:
+                yield {'label': key_label, 'value': value}
+
+
 class LoggedErrorDirectory(Directory):
     _q_exports = ['', 'delete', 'ack']
     do_not_call_in_templates = True
@@ -63,6 +85,10 @@ class LoggedErrorDirectory(Directory):
             'text': _('Text'),
         }.get(self.error.expression_type, _('Unknown'))
 
+    def get_context_frames(self):
+        for frame_context in reversed(self.error.context.get('stack') or []):
+            yield ErrorFrame(frame_context)
+
     def get_tabs(self):
         r = TemplateIO(html=True)
         parts = (

wcs/conditions.py

@@ -49,21 +49,22 @@ class Condition:
         local_variables = self.get_data()
         return getattr(self, 'evaluate_' + self.type)(local_variables)
 
-    def evaluate(self):
-        try:
-            return self.unsafe_evaluate()
-        except Exception as e:
-            if self.record_errors:
-                summary = _('Failed to evaluate condition')
-                get_publisher().record_error(
-                    summary,
-                    formdata=self.context.get('formdata'),
-                    status_item=self.context.get('status_item'),
-                    expression=self.value,
-                    expression_type=self.type,
-                    exception=e,
-                )
-            raise RuntimeError()
+    def evaluate(self, source_label=None, source_url=None):
+        with get_publisher().error_context(
+            condition=self.value, condition_type=self.type, source_label=source_label, source_url=source_url
+        ):
+            try:
+                return self.unsafe_evaluate()
+            except Exception as e:
+                if self.record_errors:
+                    summary = _('Failed to evaluate condition')
+                    get_publisher().record_error(
+                        summary,
+                        formdata=self.context.get('formdata'),
+                        status_item=self.context.get('status_item'),
+                        exception=e,
+                    )
+                raise RuntimeError()
 
     def evaluate_python(self, local_variables):
         global_variables = get_publisher().get_global_eval_dict()

wcs/fields/base.py

@@ -15,6 +15,7 @@
 # along with this program; if not, see <http://www.gnu.org/licenses/>.
 
 import collections
+import copy
 import datetime
 import html
 import re
@@ -236,6 +237,11 @@ class Field:
         for k, v in kwargs.items():
             setattr(self, k.replace('-', '_'), v)
 
+    def __getstate__(self):
+        odict = copy.copy(self.__dict__)
+        odict.pop('_formdef', None)
+        return odict
+
     @classmethod
     def init(cls):
         pass
@@ -243,6 +249,11 @@ class Field:
     def get_type_label(self):
         return self.description
 
+    def get_admin_url(self):
+        if not getattr(self, '_formdef', None):
+            return ''
+        return self._formdef.get_field_admin_url(field=self)
+
     @property
     def include_in_listing(self):
         return 'listings' in (self.display_locations or [])
@@ -584,16 +595,24 @@ class Field:
         return changed
 
     @staticmethod
-    def evaluate_condition(dict_vars, formdef, condition, record_errors=True):
+    def evaluate_condition(
+        dict_vars, formdef, condition, source_label=None, source_url=None, record_errors=True
+    ):
         from .page import PageCondition
 
-        return PageCondition(
-            condition, {'dict_vars': dict_vars, 'formdef': formdef}, record_errors
-        ).evaluate()
+        return PageCondition(condition, {'dict_vars': dict_vars, 'formdef': formdef}, record_errors).evaluate(
+            source_label=source_label, source_url=source_url
+        )
 
     def is_visible(self, dict, formdef):
         try:
-            return self.evaluate_condition(dict, formdef, self.condition)
+            return self.evaluate_condition(
+                dict,
+                formdef,
+                self.condition,
+                source_label=_('Field: %s') % self.ellipsized_label,
+                source_url=self.get_admin_url(),
+            )
         except RuntimeError:
             return True
 

wcs/fields/block.py

@@ -342,9 +342,8 @@ class BlockField(WidgetField):
 
     def __getstate__(self):
         # do not store _block cache
-        odict = copy.copy(self.__dict__)
-        if '_block' in odict:
-            del odict['_block']
+        odict = super().__getstate__()
+        odict.pop('_block', None)
         return odict
 
     def __setstate__(self, ndict):

wcs/formdef.py

@@ -2010,6 +2010,8 @@ class FormDef(StorableObject):
                 o.fields = pickle.load(fd, **PICKLE_KWARGS)
             except EOFError:
                 pass  # old format
+        for field in o.fields or []:
+            field._formdef = o  # keep formdef reference
         return o
 
     @classmethod

wcs/logged_errors.py

@@ -18,6 +18,7 @@ import re
 
 from django.utils.formats import number_format
 from django.utils.timezone import now
+from quixote import get_publisher
 from quixote.html import htmlescape, htmltext
 
 from wcs.carddef import CardDef
@@ -42,6 +43,7 @@ class LoggedError:
     status_item_id = None
     expression = None
     expression_type = None
+    context = None
     traceback = None
     exception_class = None
     exception_message = None
@@ -93,6 +95,8 @@ class LoggedError:
         if status:
             error.status_id = status.id
 
+        error.context = get_publisher().get_error_context()
+
         error.first_occurence_timestamp = now()
         error.tech_id = error.build_tech_id()
         error.occurences_count += 1
@@ -111,6 +115,7 @@ class LoggedError:
         self.traceback = error.traceback
         self.expression = error.expression
         self.expression_type = error.expression_type
+        self.context = error.context
         # exception should be the same (same tech_id), record just in case
         self.exception_class = error.exception_class
         self.exception_message = error.exception_message

wcs/publisher.py

@@ -605,6 +605,7 @@ class WcsPublisher(QommonPublisher):
     def cleanup(self):
         self._cached_user_fields_formdef = None
         self._update_related_seen = None
+        self._error_context = None
         from . import sql
 
         sql.cleanup_connection()
@@ -673,6 +674,22 @@ class WcsPublisher(QommonPublisher):
         finally:
             self.keep_all_block_rows_mode = False
 
+    # stacked contexts to include in logged errors
+    _error_context = None
+
+    @contextmanager
+    def error_context(self, **kwargs):
+        if not self._error_context:
+            self._error_context = []
+        self._error_context.append(kwargs)
+        try:
+            yield True
+        finally:
+            self._error_context.pop()
+
+    def get_error_context(self):
+        return {'stack': self._error_context} if self._error_context else None
+
     def clean_deleted_users(self, **kwargs):
         for user_id in self.user_class.get_to_delete_ids():
             self.user_class.remove_object(user_id)

wcs/qommon/static/css/dc2/admin.scss

@@ -3143,3 +3143,18 @@ form div.widget[data-widget-name="model_file_mode"] {
 		display: block;
 	}
 }
+
+#panel-general ul.logged-error-frames {
+	padding: 0;
+	margin: 0;
+	li {
+		padding: 0;
+		margin: 0;
+	}
+	.logged-error-frames--context {
+		list-style: none;
+	}
+	> li:nth-child(2n) {
+		background: #eee;
+	}
+}

wcs/sql.py

@@ -1177,6 +1177,7 @@ def do_loggederrors_table():
                                 status_item_id VARCHAR,
                                 expression VARCHAR,
                                 expression_type VARCHAR,
+                                context JSONB,
                                 traceback TEXT,
                                 exception_class VARCHAR,
                                 exception_message VARCHAR,
@@ -1199,6 +1200,8 @@ def do_loggederrors_table():
     # migrations
     if 'kind' not in existing_fields:
         cur.execute('''ALTER TABLE %s ADD COLUMN kind VARCHAR''' % table_name)
+    if 'context' not in existing_fields:
+        cur.execute('''ALTER TABLE %s ADD COLUMN context JSONB''' % table_name)
 
     # delete obsolete fields
     for field in existing_fields - needed_fields:
@@ -3731,6 +3734,7 @@ class LoggedError(SqlMixin, wcs.logged_errors.LoggedError):
         ('status_item_id', 'varchar'),
         ('expression', 'varchar'),
         ('expression_type', 'varchar'),
+        ('context', 'jsonb'),
         ('traceback', 'text'),
         ('exception_class', 'varchar'),
         ('exception_message', 'varchar'),
@@ -5111,7 +5115,7 @@ def get_period_total(
 # latest migration, number + description (description is not used
 # programmaticaly but will make sure git conflicts if two migrations are
 # separately added with the same number)
-SQL_LEVEL = (105, 'change test result json structure')
+SQL_LEVEL = (106, 'add context column to logged_errors table')
 
 
 def migrate_global_views(conn, cur):
@@ -5239,10 +5243,11 @@ def migrate():
         # 50: switch role uuid column to varchar
         do_role_table()
         migrate_legacy_roles()
-    if sql_level < 53:
+    if sql_level < 106:
         # 47: store LoggedErrors in SQL
         # 48: remove acked attribute from LoggedError
         # 53: add kind column to logged_errors table
+        # 106: add context column to logged_errors table
         do_loggederrors_table()
     if sql_level < 94:
         # 3: introduction of _structured for user fields

wcs/templates/wcs/backoffice/logged-error.html

@@ -43,6 +43,17 @@
           {% if error.expression or error.expression_type %}
             <li>{{ view.error_expression_type_label }}{% trans ":" %} <code>{{ error.expression }}</code></li>
           {% endif %}
+          {% if error.context %}
+            <li><ul class="logged-error-frames">
+              {% for frame in view.get_context_frames %}
+                <li>{% if frame.source %}<a href="{{ frame.source.url }}">{{ frame.source.label }}</a>{% endif %}
+                  <ul class="logged-error-frames--context">
+                    {% for frame_context in frame.get_frame_lines %}
+                      <li>{{ frame_context.label }}{% trans ":" %} <code>{{ frame_context.value }}</code></li>
+                    {% endfor %}
+                  </ul></li>
+              {% endfor %}</ul></li>
+          {% endif %}
           {% if error.exception_class or error.exception_message %}
             <li>{% trans "Error message:" %} <code>{{ error.exception_class }}: {{ error.exception_message }}</code></li>
           {% endif %}

wcs/workflows.py

@@ -3068,7 +3068,9 @@ class WorkflowStatusItem(XmlSerialisable):
         return ''
 
     def get_admin_url(self):
-        return self.parent.get_admin_url() + 'items/%s/' % self.id
+        if self.parent:
+            return self.parent.get_admin_url() + 'items/%s/' % self.id
+        return ''
 
     def get_inspect_details(self):
         return getattr(self, 'label', '')
@@ -3133,7 +3135,10 @@ class WorkflowStatusItem(XmlSerialisable):
     def check_condition(self, formdata, record_errors=True):
         context = {'formdata': formdata, 'status_item': self}
         try:
-            return Condition(self.condition, context, record_errors=record_errors).evaluate()
+            return Condition(self.condition, context, record_errors=record_errors).evaluate(
+                source_label=str(self.description),
+                source_url=self.get_admin_url(),
+            )
         except RuntimeError:
             return False