Browse Source

chore(derived_code_mappings): Group various Api errors (#44674)

It seems that unhandled errors tend not to group together because the originating error starts with an HTTPError that can be any URL.

Logging them in a unique line will group them together rather than based on the initial unique URL.
Armen Zambrano G 2 years ago
parent
commit
ba7fb2805d

+ 8 - 1
src/sentry/tasks/derive_code_mappings.py

@@ -90,7 +90,14 @@ def derive_code_mappings(
             logger.warning("The org has uninstalled the Sentry App.", extra=extra)
             return
 
-        raise error  # Let's report the issue
+        # Logging the exception and returning is better than re-raising the error
+        # Otherwise, API errors would not group them since the HTTPError in the stack
+        # has unique URLs, thus, separating the errors
+        logger.exception(
+            "Unhandled ApiError occurred. Nothing is broken. Investigate. Multiple issues grouped.",
+            extra=extra,
+        )
+        return
     except UnableToAcquireLock as error:
         extra["error"] = error
         logger.warning("derive_code_mappings.getting_lock_failed", extra=extra)

+ 4 - 3
tests/sentry/tasks/test_derive_code_mappings.py

@@ -56,13 +56,14 @@ class TestTaskBehavior(BaseDeriveCodeMappings):
         ):
             assert derive_code_mappings(self.project.id, self.event_data) is None
 
-    def test_raises_other_api_errors(self):
+    @patch("sentry.tasks.derive_code_mappings.logger")
+    def test_raises_other_api_errors(self, mock_logger):
         with patch(
             "sentry.integrations.github.client.GitHubClientMixin.get_trees_for_org",
             side_effect=ApiError("foo"),
         ):
-            with pytest.raises(ApiError):
-                derive_code_mappings(self.project.id, self.event_data)
+            derive_code_mappings(self.project.id, self.event_data)
+            assert mock_logger.exception.call_count == 1
 
     def test_unable_to_get_lock(self):
         with patch(