WIP: update error handling for notifications

Author: palkerecsenyi

invenio_rdm_records/notifications/builders.py

@@ -666,9 +666,11 @@ def build(
         generic_repository: GenericRepository,
         generic_release: GenericRelease,
         draft,
+        error_message: str,
     ):
         notification = super().build(provider, generic_repository, generic_release)
         notification.context["draft"] = EntityResolverRegistry.reference_entity(draft)
+        notification.context["error_message"] = error_message
         return notification
 
     context = [EntityResolve(key="draft")]

invenio_rdm_records/services/vcs/metadata.py

@@ -12,7 +12,7 @@
 import yaml
 from flask import current_app
 from invenio_i18n import _
-from invenio_vcs.errors import CustomVCSMetadataError
+from invenio_vcs.errors import CustomVCSReleaseNoRetryError
 from invenio_vcs.generic_models import GenericContributor
 from marshmallow import Schema, ValidationError
 from mistune import markdown
@@ -145,8 +145,9 @@ def citation_metadata(self):
             # Load metadata from citation file and serialize it
             return self.load_citation_metadata(data)
         except ValidationError as e:
-            # Wrap the error into CustomVCSMetadataError() so it can be handled upstream
-            raise CustomVCSMetadataError(file=citation_file_path, message=e.messages)
+            # Wrap the error into CustomVCSReleaseNoRetryError() so it can be handled upstream.
+            # This also ensures the release isn't retried without user action.
+            raise CustomVCSReleaseNoRetryError(message=e.messages)
 
     @property
     def extra_metadata(self):

invenio_rdm_records/services/vcs/release.py

@@ -14,6 +14,7 @@
 from invenio_i18n import lazy_gettext as _
 from invenio_notifications.services.uow import NotificationOp
 from invenio_records_resources.services.uow import UnitOfWork
+from invenio_vcs.errors import CustomVCSReleaseNoRetryError
 from invenio_vcs.models import ReleaseStatus
 from invenio_vcs.service import VCSRelease
 
@@ -237,6 +238,20 @@ def publish(self):
             # Flag release as FAILED and raise the exception
             self.release_failed()
 
+            # The release publish can fail for a wide range of reasons, each of which have various inconsistent error types.
+            # Some errors have a 'message' attribute or a 'description' attribute, which is not the value that gets used
+            # when the error is stringified.
+            # Some errors might not have any accessible message, so we use the class name as a last resort.
+            error_message = str(ex)
+            if not error_message:
+                if hasattr(ex, "message"):
+                    # Need to stringify the LazyString, otherwise serialisation will fail
+                    error_message = str(ex.message)
+                elif hasattr(ex, "description"):
+                    error_message = str(ex.description)
+                else:
+                    error_message = type(ex).__name__
+
             with UnitOfWork(db.session) as uow:
                 uow.register(
                     NotificationOp(
@@ -245,6 +260,7 @@ def publish(self):
                             generic_repository=self.generic_repo,
                             generic_release=self.generic_release,
                             draft=draft,
+                            error_message=error_message,
                         )
                     )
                 )
@@ -253,7 +269,8 @@ def publish(self):
             # Commit the FAILED state, other changes were already rollbacked by the UOW
             db.session.commit()
 
-            raise ex
+            # Wrap the error to ensure Celery does not attempt to retry it (since user action is needed to resolve the problem)
+            raise CustomVCSReleaseNoRetryError(message=error_message)
 
     def process_release(self):
         """Processes a github release.

invenio_rdm_records/templates/semantic-ui/invenio_notifications/repository-release.failure.jinja

@@ -1,20 +1,24 @@
 {% set repository_full_name = notification.context.repository_full_name %}
 {% set release_tag = notification.context.release_tag %}
 {% set draft = notification.context.draft %}
+{% set error_message = notification.context.error_message %}
 
 {% set account_settings_link = invenio_url_for("invenio_notifications_settings.index") %}
 {% set draft_link = draft["links"]["self_html"] %}
 
 {%- block subject -%}
-    {{ _("There was a problem publishing a release from {repository_name}", repository_name=repository_full_name) }}
+    {{ _("There was a problem publishing a release from %(repository_name)s", repository_name=repository_full_name) }}
 {%- endblock subject -%}
 
 {%- block html_body -%}
 <table style="font-family:'Lato',Helvetica,Arial,sans-serif;border-spacing:15px">
     <tr>
-        <td>{{ _("Release {release_tag} of {repository_name} failed to publish.", release_tag=release_tag, repository_name=repository_full_name )}}
+        <td>{{ _("Release %(release_tag)s of %(repository_name)s failed to publish.", release_tag=release_tag, repository_name=repository_full_name )}}
         </td>
     </tr>
+    <tr>
+        <td>{{ _("Reason:") }} <strong>{{ error_message }}</strong></td>
+    </tr>
     <tr>
         <td>{{ _("For more details and to fix any errors,") }} <a href="{{ draft_link }}" class="button">{{ _("view the unpublished draft record.") }}</a></td>
     </tr>
@@ -28,7 +32,9 @@
 {%- endblock html_body -%}
 
 {%- block plain_body -%}
-{{ _("Release {release_tag} of {repository_name} failed to publish.", release_tag=release_tag, repository_name=repository_full_name) }}
+{{ _("Release %(release_tag)s of %(repository_name)s failed to publish.", release_tag=release_tag, repository_name=repository_full_name) }}
+
+{{ _("Reason:") }} {{error_message}}
 
 {{ _("For more details and to fix any errors,") }} [{{ _("view the unpublished draft record.") }}]({{ draft_link }})
 
@@ -37,7 +43,9 @@
 
 {# Markdown for Slack/Mattermost/chat #}
 {%- block md_body -%}
-{{ _("Release {release_tag} of {repository_name} failed to publish.", release_tag=release_tag, repository_name=repository_full_name )}}
+{{ _("Release %(release_tag)s of %(repository_name)s failed to publish.", release_tag=release_tag, repository_name=repository_full_name )}}
+
+{{ _("Reason:") }} {{error_message}}
 
 {{ _("For more details and to fix any errors,") }} [{{ _("view the unpublished draft record.") }}]({{ draft_link }})
 

invenio_rdm_records/templates/semantic-ui/invenio_notifications/repository-release.success.jinja

@@ -6,13 +6,13 @@
 {% set record_link = record.data["links"]["self_html"] %}
 
 {%- block subject -%}
-    {{ _("A release from {repository_name} was published successfully", repository_name=repository_full_name) }}
+    {{ _("A release from %(repository_name)s was published successfully", repository_name=repository_full_name) }}
 {%- endblock subject -%}
 
 {%- block html_body -%}
 <table style="font-family:'Lato',Helvetica,Arial,sans-serif;border-spacing:15px">
     <tr>
-        <td>{{ _("Release {release_tag} of {repository_name} has been received and published successfully.", release_tag=release_tag, repository_name=repository_full_name) }}
+        <td>{{ _("Release %(release_tag)s of %(repository_name)s has been received and published successfully.", release_tag=release_tag, repository_name=repository_full_name) }}
         </td>
     </tr>
     <tr>
@@ -28,7 +28,7 @@
 {%- endblock html_body -%}
 
 {%- block plain_body -%}
-{{ _("Release {release_tag} of {repository_name} has been received and published successfully.", release_tag=release_tag, repository_name=repository_full_name) }}
+{{ _("Release %(release_tag)s of %(repository_name)s has been received and published successfully.", release_tag=release_tag, repository_name=repository_full_name) }}
 
 {{ _("For more details,") }} [{{ _("view the published record.") }}]({{ record_link }})
 
@@ -37,7 +37,7 @@
 
 {# Markdown for Slack/Mattermost/chat #}
 {%- block md_body -%}
-{{ _("Release {release_tag} of {repository_name} has been received and published successfully.", release_tag=release_tag, repository_name=repository_full_name) }}
+{{ _("Release %(release_tag)s of %(repository_name)s has been received and published successfully.", release_tag=release_tag, repository_name=repository_full_name) }}
 
 {{ _("For more details,") }} [{{ _("view the published record.") }}]({{ record_link }})