feat(question_ui): error handling

questionpy_sdk/webserver/attempt.py

@@ -1,7 +1,6 @@
 #  This file is part of the QuestionPy SDK. (https://questionpy.org)
 #  The QuestionPy SDK is free software released under terms of the MIT license. See LICENSE.md.
 #  (c) Technische Universität Berlin, innoCampus <[email protected]>
-
 from typing import Literal, TypedDict
 
 from questionpy_common.api.attempt import AttemptModel, AttemptScoredModel
@@ -10,6 +9,7 @@
     QuestionFormulationUIRenderer,
     QuestionUIRenderer,
 )
+from questionpy_sdk.webserver.question_ui.errors import RenderErrorCollections, log_render_errors
 from questionpy_server.api.models import AttemptStarted
 
 
@@ -27,6 +27,8 @@ class _AttemptRenderContext(TypedDict):
     specific_feedback: str | None
     right_answer: str | None
 
+    render_errors: RenderErrorCollections
+
 
 def get_attempt_render_context(
     attempt: AttemptModel,
@@ -39,6 +41,8 @@ def get_attempt_render_context(
 ) -> _AttemptRenderContext:
     renderer_args = (attempt.ui.placeholders, display_options, seed, last_attempt_data)
 
+    formulation_renderer = QuestionFormulationUIRenderer(attempt.ui.formulation, *renderer_args)
+
     context: _AttemptRenderContext = {
         "attempt_status": (
             "Started"
@@ -50,18 +54,29 @@ def get_attempt_render_context(
         "attempt_state": attempt_state,
         "options": display_options.model_dump(include={"general_feedback", "feedback", "right_answer"}),
         "form_disabled": disabled,
-        "formulation": QuestionFormulationUIRenderer(attempt.ui.formulation, *renderer_args).html,
+        "formulation": formulation_renderer.html,
         "attempt": attempt,
         "general_feedback": None,
         "specific_feedback": None,
         "right_answer": None,
+        "render_errors": {},
     }
 
+    if formulation_renderer.errors:
+        context["render_errors"]["Formulation"] = formulation_renderer.errors
     if display_options.general_feedback and attempt.ui.general_feedback:
-        context["general_feedback"] = QuestionUIRenderer(attempt.ui.general_feedback, *renderer_args).html
+        renderer = QuestionUIRenderer(attempt.ui.general_feedback, *renderer_args)
+        context["general_feedback"] = renderer.html
+        context["render_errors"]["General Feedback"] = renderer.errors
     if display_options.feedback and attempt.ui.specific_feedback:
-        context["specific_feedback"] = QuestionUIRenderer(attempt.ui.specific_feedback, *renderer_args).html
+        renderer = QuestionUIRenderer(attempt.ui.specific_feedback, *renderer_args)
+        context["specific_feedback"] = renderer.html
+        context["render_errors"]["Specific Feedback"] = renderer.errors
     if display_options.right_answer and attempt.ui.right_answer:
-        context["right_answer"] = QuestionUIRenderer(attempt.ui.right_answer, *renderer_args).html
+        renderer = QuestionUIRenderer(attempt.ui.right_answer, *renderer_args)
+        context["right_answer"] = renderer.html
+        context["render_errors"]["Right Answer"] = renderer.errors
+
+    log_render_errors(context["render_errors"])
 
     return context

questionpy_sdk/webserver/question_ui.py → questionpy_sdk/webserver/question_ui/__init__.py

@@ -14,6 +14,12 @@
 from lxml import etree
 from pydantic import BaseModel
 
+from questionpy_sdk.webserver.question_ui.errors import (
+    InvalidAttributeValueError,
+    RenderErrorCollection,
+    XMLSyntaxError,
+)
+
 
 def _assert_element_list(query: Any) -> list[etree._Element]:
     """Checks if the XPath query result is a list of Elements.
@@ -185,7 +191,16 @@ def __init__(
         attempt: dict | None = None,
     ) -> None:
         xml = self._replace_qpy_urls(xml)
-        self._xml = etree.ElementTree(etree.fromstring(xml))
+        self.errors = RenderErrorCollection()
+
+        try:
+            root = etree.fromstring(xml)
+        except etree.XMLSyntaxError as error:
+            parser = etree.XMLParser(recover=True)
+            root = etree.fromstring(xml, parser=parser)
+            self.errors.insert(XMLSyntaxError(error=error))
+
+        self._xml = etree.ElementTree(root)
         self._xpath = etree.XPathDocumentEvaluator(self._xml)
         self._xpath.register_namespace("xhtml", self.XHTML_NAMESPACE)
         self._xpath.register_namespace("qpy", self.QPY_NAMESPACE)
@@ -265,13 +280,21 @@ def _hide_unwanted_feedback(self) -> None:
             feedback_type = element.get(f"{{{self.QPY_NAMESPACE}}}feedback")
 
             # Check conditions to remove the element
-            if (feedback_type == "general" and not self._options.general_feedback) or (
-                feedback_type == "specific" and not self._options.feedback
+            if not (
+                (feedback_type == "general" and self._options.general_feedback)
+                or (feedback_type == "specific" and self._options.feedback)
             ):
                 parent = element.getparent()
                 if parent is not None:
                     parent.remove(element)
 
+            expected = ("general", "specific")
+            if feedback_type not in expected:
+                error = InvalidAttributeValueError(
+                    element=element, attribute="qpy:feedback", value=feedback_type or "", expected=expected
+                )
+                self.errors.insert(error)
+
     def _hide_if_role(self) -> None:
         """Hides elements based on user role.
 

questionpy_sdk/webserver/question_ui/errors.py

@@ -0,0 +1,148 @@
+#  This file is part of the QuestionPy SDK. (https://questionpy.org)
+#  The QuestionPy SDK is free software released under terms of the MIT license. See LICENSE.md.
+#  (c) Technische Universität Berlin, innoCampus <[email protected]>
+import html
+import logging
+from abc import ABC, abstractmethod
+from bisect import insort
+from collections.abc import Iterable, Iterator, Sized
+from dataclasses import dataclass
+from functools import cached_property
+from operator import attrgetter
+from typing import TypeAlias
+
+from lxml import etree
+
+_log = logging.getLogger(__name__)
+
+
+@dataclass(frozen=True)
+class RenderError(ABC):
+    """Represents a generic error which occurred during rendering."""
+
+    @cached_property
+    @abstractmethod
+    def line(self) -> int | None:
+        """Original line number where the error occurred or None if unknown."""
+
+    @cached_property
+    def order(self) -> int:
+        """Can be used to order multiple errors."""
+        return self.line or 0
+
+    @cached_property
+    @abstractmethod
+    def message(self) -> str:
+        pass
+
+    @cached_property
+    def html_message(self) -> str:
+        return html.escape(self.message)
+
+
+@dataclass(frozen=True)
+class RenderElementError(RenderError, ABC):
+    element: etree._Element
+
+    @cached_property
+    def element_representation(self) -> str:
+        # Create the prefix of an element. We do not want to keep 'html' as a prefix.
+        prefix = f"{self.element.prefix}:" if self.element.prefix and self.element.prefix != "html" else ""
+        return prefix + etree.QName(self.element).localname
+
+    @cached_property
+    def line(self) -> int | None:
+        """Original line number as found by the parser or None if unknown."""
+        return self.element.sourceline  # type: ignore[return-value]
+
+
+@dataclass(frozen=True)
+class InvalidAttributeValueError(RenderElementError):
+    """Invalid attribute value."""
+
+    attribute: str
+    value: str
+    expected: Iterable[str] | None = None
+
+    def _message(self, *, as_html: bool) -> str:
+        if as_html:
+            (opening, closing) = ("<code>", "</code>")
+            value = html.escape(self.value)
+        else:
+            (opening, closing) = ("'", "'")
+            value = self.value
+
+        expected = ""
+        if self.expected:
+            expected = f" Expected one of [{opening}" + f"{closing}, {opening}".join(self.expected) + f"{closing}]."
+
+        return (
+            f"Invalid value {opening}{value}{closing} for attribute {opening}{self.attribute}{closing} "
+            f"on element {opening}{self.element_representation}{closing}.{expected}"
+        )
+
+    @cached_property
+    def message(self) -> str:
+        return self._message(as_html=False)
+
+    @cached_property
+    def html_message(self) -> str:
+        return self._message(as_html=True)
+
+
+@dataclass(frozen=True)
+class XMLSyntaxError(RenderError):
+    """Syntax error while parsing the XML."""
+
+    error: etree.XMLSyntaxError
+
+    @cached_property
+    def line(self) -> int | None:
+        return self.error.lineno
+
+    @cached_property
+    def order(self) -> int:
+        # Syntax errors can lead to a multitude of other errors therefore we want them to be the first in order.
+        return -1
+
+    @cached_property
+    def message(self) -> str:
+        return f"Syntax error: {self.error.msg}"
+
+    @cached_property
+    def html_message(self) -> str:
+        return f"Invalid syntax: <samp>{html.escape(self.error.msg)}</samp>"
+
+
+class RenderErrorCollection(Iterable, Sized):
+    """Collects render errors and provides a sorted iterator."""
+
+    _errors: list[RenderError]
+
+    def __init__(self) -> None:
+        self._errors = []
+
+    def insert(self, error: RenderError) -> None:
+        insort(self._errors, error, key=attrgetter("order"))
+
+    def __iter__(self) -> Iterator[RenderError]:
+        return iter(self._errors)
+
+    def __len__(self) -> int:
+        return len(self._errors)
+
+    def __repr__(self) -> str:
+        return f"{self.__class__.__name__}({self._errors})"
+
+
+RenderErrorCollections: TypeAlias = dict[str, RenderErrorCollection]
+"""Section to RenderErrorCollection map."""
+
+
+def log_render_errors(render_errors: RenderErrorCollections) -> None:
+    for section, errors in render_errors.items():
+        errors_string = ""
+        for error in errors:
+            line = f"Line {error.line}: " if error.line else ""
+            errors_string += f"\n\t- {line}{error.message}"
+        _log.warning(f"{len(errors)} error(s) occurred while rendering {section}:{errors_string}")

questionpy_sdk/webserver/static/styles.css

@@ -227,3 +227,71 @@ fieldset {
     padding: 0.1rem 1rem;
     width: 100%;
 }
+
+.container-render-errors {
+    border: 2px solid red;
+    border-radius: 0.4rem;
+    padding: 0 0.2rem;
+    background-color: #ffecec;
+}
+
+.container-render-errors table {
+    width: auto;
+    max-width: 100%;
+    border-collapse: collapse;
+    margin: 0 1.2rem 1rem 1.2rem
+}
+
+/* Add spacing between each section / tbody.*/
+.container-render-errors table tbody + tbody::before {
+    content: "";
+    display: table-row;
+    height: 1.5rem;
+}
+
+.container-render-errors th, .container-render-errors td {
+    border: 1px solid darkgrey;
+}
+
+.container-render-errors table th[scope="rowgroup"] {
+    border: 0;
+    padding-bottom: 0.4rem;
+    font-size: 1.2rem;
+    text-decoration: underline;
+}
+
+.container-render-errors table tr td:first-child,
+.container-render-errors table tr th:first-child:not([scope="rowgroup"]) {
+    border-left: 0;
+    padding-right: 0.5rem;
+}
+
+.container-render-errors table tr td:first-child {
+    vertical-align: top;
+    text-align: right;
+    font-variant-numeric: lining-nums tabular-nums;
+}
+
+.container-render-errors table tr td:last-child,
+.container-render-errors table tr th:last-child:not([scope="rowgroup"]) {
+    border-right: 0;
+    padding-left: 0.5rem;
+
+}
+
+.container-render-errors table tbody th {
+    border-top: 0;
+}
+
+.container-render-errors table tbody td {
+    border-bottom: 0;
+}
+
+.container-render-errors code {
+    background-color: #ddd;
+    border-radius: 0.2rem;
+    padding: 0 0.3rem;
+    /* Whitespace should be visible as it might be the cause of an error. */
+    white-space: break-spaces;
+    overflow-wrap: anywhere;
+}

questionpy_sdk/webserver/templates/attempt.html.jinja2

@@ -11,6 +11,31 @@
 {% endblock %}
 
 {% block content %}
+    {% if render_errors %}
+        <div class="container-render-errors">
+            <h2>Render Errors</h2>
+            <table>
+                {% for section, errors in render_errors.items() %}
+                <tbody>
+                    <tr>
+                        <th colspan="2" scope="rowgroup">{{ section }}</th>
+                    </tr>
+                    <tr>
+                        <th>Line</th>
+                        <th>Error-Message</th>
+                    </tr>
+                    {% for error in errors %}
+                    <tr>
+                        <td>{{ error.line or '' }}</td>
+                        <td>{{ error.html_message|safe }}</td>
+                    </tr>
+                    {% endfor %}
+                </tbody>
+                {% endfor %}
+            </table>
+        </div>
+    {% endif %}
+
     <div class="container-question-info">
         <p class="score">
             {{ ("Score: " ~ attempt.score) if attempt.score is defined else "Not yet scored" }}

tests/questionpy_sdk/webserver/test_data/faulty.xhtml

@@ -0,0 +1,5 @@
+<div xmlns="http://www.w3.org/1999/xhtml" xmlns:qpy="http://questionpy.org/ns/question">
+    <span qpy:feedback="unknown">Unknown feedback type.</span>
+    <span no-attribute-value>Missing attribute value.</span>
+    <span qpy:feedback="unknown">Unknown feedback type.</span>
+</div>