Bug 2049067 - Don't fail if a previous code review task failed but already created Harbormaster URI

bot/code_review_bot/analysis.py

@@ -1,7 +1,7 @@
 from functools import cached_property
 
 import structlog
-from libmozdata.phabricator import BuildState, UnitResult, UnitResultState
+from libmozdata.phabricator import BuildState, ConduitError, UnitResult, UnitResultState
 
 from code_review_bot.sources.phabricator import PhabricatorBuild, PhabricatorBuildState
 
@@ -146,12 +146,21 @@ def publish_analysis_phabricator(payload, phabricator_api):
                 "Missing base revision on PhabricatorBuild, adding a warning to Unit Tests section on Phabricator"
             )
 
-        phabricator_api.create_harbormaster_uri(
-            build.target_phid,
-            "treeherder",
-            "CI (Treeherder) Jobs",
-            extras["treeherder_url"],
-        )
+        try:
+            phabricator_api.create_harbormaster_uri(
+                build.target_phid,
+                "treeherder",
+                "CI (Treeherder) Jobs",
+                extras["treeherder_url"],
+            )
+        except ConduitError as e:
+            if e.error_info and "Duplicate entry" in e.error_info:
+                logger.warning(
+                    "Harbormaster URI artifact already exists, skipping creation (retry?)",
+                    build=str(build),
+                )
+            else:
+                raise
 
     elif mode == "work":
         phabricator_api.update_build_target(build.target_phid, BuildState.Work)

bot/code_review_bot/workflow.py

@@ -7,7 +7,7 @@
 from itertools import groupby
 
 import structlog
-from libmozdata.phabricator import BuildState, PhabricatorAPI
+from libmozdata.phabricator import BuildState, ConduitError, PhabricatorAPI
 from taskcluster.utils import stringDate
 
 from code_review_bot import Level, stats
@@ -760,6 +760,16 @@ def publish_link(self, revision: Revision, slug: str, name: str, url: str):
             )
             return
 
-        self.phabricator.create_harbormaster_uri(
-            revision.build_target_phid, slug, name, url
-        )
+        try:
+            self.phabricator.create_harbormaster_uri(
+                revision.build_target_phid, slug, name, url
+            )
+        except ConduitError as e:
+            if e.error_info and "Duplicate entry" in e.error_info:
+                logger.warning(
+                    "Harbormaster URI artifact already exists, skipping creation (retry?)",
+                    slug=slug,
+                    url=url,
+                )
+            else:
+                raise

bot/tests/test_phabricator_analysis.py

@@ -4,8 +4,15 @@
 
 import json
 import tempfile
+from unittest import mock
+
+import pytest
+from libmozdata.phabricator import ConduitError
 
 from code_review_bot import mercurial
+from code_review_bot.analysis import (
+    publish_analysis_phabricator,
+)
 from code_review_bot.config import RepositoryConf
 from code_review_bot.revisions import PhabricatorRevision
 from code_review_bot.sources.phabricator import PhabricatorActions
@@ -220,3 +227,46 @@ def mock_hgrun(cmd):
     # Reset settings for following tests
     mock_config.mercurial_cache = None
     mock_config.ssh_key = None
+
+
+def test_publish_analysis_phabricator_duplicate_harbormaster_uri():
+    """
+    When create_harbormaster_uri raises a ConduitError due to a duplicate key
+    (e.g. on task retry after worker shutdown), the error should be swallowed
+    and a warning logged instead of crashing the publication task.
+    """
+    build = mock.MagicMock()
+    build.target_phid = "PHID-HMBT-test"
+    build.missing_base_revision = False
+
+    phabricator_api = mock.MagicMock()
+    phabricator_api.create_harbormaster_uri.side_effect = ConduitError(
+        "Duplicate entry",
+        error_code="ERR-CONDUIT-CORE",
+        error_info="Duplicate entry 'uri-VEVIsJOfD0wc' for key 'harbormaster_buildartifact.key_artifact'",
+    )
+
+    payload = ("success", build, {"treeherder_url": "https://treeherder.mozilla.org/"})
+    publish_analysis_phabricator(payload, phabricator_api)
+
+    phabricator_api.create_harbormaster_uri.assert_called_once()
+
+
+def test_publish_analysis_phabricator_reraises_other_conduit_errors():
+    """
+    Non-duplicate ConduitErrors must still propagate.
+    """
+    build = mock.MagicMock()
+    build.target_phid = "PHID-HMBT-test"
+    build.missing_base_revision = False
+
+    phabricator_api = mock.MagicMock()
+    phabricator_api.create_harbormaster_uri.side_effect = ConduitError(
+        "Some other error",
+        error_code="ERR-CONDUIT-CORE",
+        error_info="Some unrelated error",
+    )
+
+    payload = ("success", build, {"treeherder_url": "https://treeherder.mozilla.org/"})
+    with pytest.raises(ConduitError):
+        publish_analysis_phabricator(payload, phabricator_api)

bot/tests/test_workflow.py

@@ -8,8 +8,10 @@
 
 import pytest
 import responses
+from libmozdata.phabricator import ConduitError
 
 from code_review_bot.config import Settings
+from code_review_bot.revisions import PhabricatorRevision
 from code_review_bot.tasks.clang_format import ClangFormatIssue, ClangFormatTask
 from code_review_bot.tasks.clang_tidy import ClangTidyTask
 from code_review_bot.tasks.clang_tidy_external import ExternalTidyTask
@@ -168,3 +170,48 @@ def test_before_after(mock_taskcluster_config, mock_workflow, mock_task, mock_re
     ]
     assert issues[0].new_issue is True
     assert issues[1].new_issue is False
+
+
+def test_publish_link_duplicate_harbormaster_uri(mock_workflow):
+    """
+    When publish_link raises a ConduitError due to a duplicate Harbormaster URI
+    artifact (e.g. on task retry after worker shutdown), the error should be
+    swallowed and a warning logged.
+    """
+    mock_workflow.update_build = True
+    mock_workflow.phabricator = mock.MagicMock()
+    mock_workflow.phabricator.create_harbormaster_uri.side_effect = ConduitError(
+        "Duplicate entry",
+        error_code="ERR-CONDUIT-CORE",
+        error_info="Duplicate entry 'uri-VEVIsJOfD0wc' for key 'harbormaster_buildartifact.key_artifact'",
+    )
+
+    revision = mock.MagicMock(spec=PhabricatorRevision)
+    revision.build_target_phid = "PHID-HMBT-test"
+
+    mock_workflow.publish_link(
+        revision, "treeherder", "CI Jobs", "https://treeherder.mozilla.org/"
+    )
+
+    mock_workflow.phabricator.create_harbormaster_uri.assert_called_once()
+
+
+def test_publish_link_reraises_other_conduit_errors(mock_workflow):
+    """
+    Non-duplicate ConduitErrors must still propagate from publish_link.
+    """
+    mock_workflow.update_build = True
+    mock_workflow.phabricator = mock.MagicMock()
+    mock_workflow.phabricator.create_harbormaster_uri.side_effect = ConduitError(
+        "Some other error",
+        error_code="ERR-CONDUIT-CORE",
+        error_info="Some unrelated error",
+    )
+
+    revision = mock.MagicMock(spec=PhabricatorRevision)
+    revision.build_target_phid = "PHID-HMBT-test"
+
+    with pytest.raises(ConduitError):
+        mock_workflow.publish_link(
+            revision, "treeherder", "CI Jobs", "https://treeherder.mozilla.org/"
+        )