ashb commented on code in PR #67505:
URL: https://github.com/apache/airflow/pull/67505#discussion_r3374980413


##########
task-sdk/tests/task_sdk/execution_time/test_sentry.py:
##########
@@ -268,3 +268,75 @@ def test_minimum_config(self, mock_sentry_sdk, 
sentry_minimum):
         sentry_minimum.prepare_to_enrich_errors(executor_integration="")
         assert mock_sentry_sdk.integrations.logging.ignore_logger.mock_calls 
== [mock.call("airflow.task")]
         assert mock_sentry_sdk.init.mock_calls == [mock.call(integrations=[])]
+
+    @pytest.mark.parametrize(
+        ("failing_step", "expected_event"),
+        [
+            ("prepare_to_enrich_errors", "sentry_prepare_failed"),
+            ("add_tagging", "sentry_enrichment_failed"),
+            ("add_breadcrumbs", "sentry_enrichment_failed"),
+        ],
+    )
+    def test_enrich_errors_isolates_instrumentation_from_task_run(
+        self,
+        mock_sentry_sdk,
+        sentry,
+        dag_run,
+        task_instance,
+        failing_step,
+        expected_event,
+    ):
+        """An instrumentation failure must not prevent the wrapped task from 
running.
+
+        ``add_tagging`` / ``add_breadcrumbs`` / ``prepare_to_enrich_errors`` 
exceptions used to
+        propagate as task failures (the wrapped ``run()`` never executed). 
``prepare`` and the
+        ``add_tagging``/``add_breadcrumbs`` enrichment block are now guarded 
so instrumentation
+        is best-effort.
+        """
+        ran = {"value": False}
+
+        def fake_run(ti, context, log):
+            ran["value"] = True
+            return "result"
+
+        log = mock.MagicMock()
+        sentry_sdk_module = sys.modules["sentry_sdk"]
+        sentry_sdk_module.new_scope = mock.MagicMock()
+        sentry_sdk_module.new_scope.return_value.__enter__ = mock.MagicMock()
+        sentry_sdk_module.new_scope.return_value.__exit__ = 
mock.MagicMock(return_value=False)

Review Comment:
   1. This leaks fixtures outside of this test.
   2. This already exist as `mock_sentry_sdk` fixture and is used in the test 
right above this one.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to