Do not fail task on notification failure (#2435)

* Do not fail task on notification failure

* Need to throw on the last iteration in order for it to go to poison queue

* lint

src/api-service/__app__/onefuzzlib/notifications/ado.py

@@ -5,6 +5,7 @@
 
 import logging
 from typing import Iterator, List, Optional, Tuple, Union
+from uuid import UUID
 
 from azure.devops.connection import Connection
 from azure.devops.credentials import BasicAuthentication
@@ -28,7 +29,7 @@ from onefuzztypes.models import ADOTemplate, RegressionReport, Report
 from onefuzztypes.primitives import Container
 
 from ..secrets import get_secret_string_value
-from .common import Render, fail_task
+from .common import Render, log_failed_notification
 
 
 class AdoNotificationException(Exception):
@@ -84,7 +85,7 @@ class ADO:
                 value = self.render(self.config.project)
             else:
                 value = self.render(self.config.ado_fields[key])
-            filters[key.lower()] = value
+        filters[key.lower()] = value
 
         valid_fields = get_valid_fields(
             self.client, project=filters.get("system.teamproject")
@@ -251,6 +252,7 @@ def notify_ado(
     filename: str,
     report: Union[Report, RegressionReport],
     fail_task_on_transient_error: bool,
+    notification_id: UUID,
 ) -> None:
     if isinstance(report, RegressionReport):
         logging.info(
@@ -285,4 +287,8 @@ def notify_ado(
                 f"transient ADO notification failure {notification_info}"
             ) from err
         else:
-            fail_task(report, err)
+            log_failed_notification(report, err, notification_id)
+            raise AdoNotificationException(
+                "Sending file changed event for notification %s to poison queue"
+                % notification_id
+            ) from err

src/api-service/__app__/onefuzzlib/notifications/common.py

@@ -5,10 +5,10 @@
 
 import logging
 from typing import Optional
+from uuid import UUID
 
 from jinja2.sandbox import SandboxedEnvironment
-from onefuzztypes.enums import ErrorCode
+from onefuzztypes.models import Report
-from onefuzztypes.models import Error, Report
 from onefuzztypes.primitives import Container
 
 from ..azure.containers import auth_download_url
@@ -18,23 +18,17 @@ from ..tasks.config import get_setup_container
 from ..tasks.main import Task
 
 
-def fail_task(report: Report, error: Exception) -> None:
+def log_failed_notification(
+    report: Report, error: Exception, notification_id: UUID
+) -> None:
     logging.error(
-        "notification failed: job_id:%s task_id:%s err:%s",
+        "notification failed: notification_id:%s job_id:%s task_id:%s err:%s",
+        notification_id,
         report.job_id,
         report.task_id,
         error,
     )
 
-    task = Task.get(report.job_id, report.task_id)
-    if task:
-        task.mark_failed(
-            Error(
-                code=ErrorCode.NOTIFICATION_FAILURE,
-                errors=["notification failed", str(error)],
-            )
-        )
-
 
 class Render:
     def __init__(

src/api-service/__app__/onefuzzlib/notifications/github_issues.py

@@ -5,6 +5,7 @@
 
 import logging
 from typing import List, Optional, Union
+from uuid import UUID
 
 from github3 import login
 from github3.exceptions import GitHubException
@@ -19,7 +20,7 @@ from onefuzztypes.models import (
 from onefuzztypes.primitives import Container
 
 from ..secrets import get_secret_obj
-from .common import Render, fail_task
+from .common import Render, log_failed_notification
 
 
 class GithubIssue:
@@ -113,6 +114,7 @@ def github_issue(
     container: Container,
     filename: str,
     report: Optional[Union[Report, RegressionReport]],
+    notification_id: UUID,
 ) -> None:
     if report is None:
         return
@@ -129,4 +131,8 @@ def github_issue(
         handler = GithubIssue(config, container, filename, report)
         handler.process()
     except (GitHubException, ValueError) as err:
-        fail_task(report, err)
+        log_failed_notification(report, err, notification_id)
+        raise GitHubException(
+            "Sending file change event for notification %s to poison queue"
+            % notification_id
+        ) from err

src/api-service/__app__/onefuzzlib/notifications/main.py

@@ -141,7 +141,13 @@ def new_files(
             done.append(notification.config)
 
             if isinstance(notification.config, TeamsTemplate):
-                notify_teams(notification.config, container, filename, report)
+                notify_teams(
+                    notification.config,
+                    container,
+                    filename,
+                    report,
+                    notification.notification_id,
+                )
 
             if not report:
                 continue
@@ -153,10 +159,17 @@ def new_files(
                     filename,
                     report,
                     fail_task_on_transient_error,
+                    notification.notification_id,
                 )
 
             if isinstance(notification.config, GithubIssueTemplate):
-                github_issue(notification.config, container, filename, report)
+                github_issue(
+                    notification.config,
+                    container,
+                    filename,
+                    report,
+                    notification.notification_id,
+                )
 
     for (task, containers) in get_queue_tasks():
         if container in containers:

src/api-service/__app__/onefuzzlib/notifications/teams.py

@@ -5,6 +5,7 @@
 
 import logging
 from typing import Any, Dict, List, Optional, Union
+from uuid import UUID
 
 import requests
 from onefuzztypes.models import RegressionReport, Report, TeamsTemplate
@@ -34,6 +35,7 @@ def send_teams_webhook(
     title: str,
     facts: List[Dict[str, str]],
     text: Optional[str],
+    notification_id: UUID,
 ) -> None:
     title = markdown_escape(title)
 
@@ -50,7 +52,12 @@ def send_teams_webhook(
     config_url = get_secret_string_value(config.url)
     response = requests.post(config_url, json=message)
     if not response.ok:
-        logging.error("webhook failed %s %s", response.status_code, response.content)
+        logging.error(
+            "webhook failed notification_id:%s %s %s",
+            notification_id,
+            response.status_code,
+            response.content,
+        )
 
 
 def notify_teams(
@@ -58,6 +65,7 @@ def notify_teams(
     container: Container,
     filename: str,
     report: Optional[Union[Report, RegressionReport]],
+    notification_id: UUID,
 ) -> None:
     text = None
     facts: List[Dict[str, str]] = []
@@ -130,4 +138,4 @@ def notify_teams(
             }
         ]
 
-    send_teams_webhook(config, title, facts, text)
+    send_teams_webhook(config, title, facts, text, notification_id)