fix(sentry-apps): Handle empty webhook_url in external requests (#109529)

Author: sentry[bot]

src/sentry/sentry_apps/components.py

@@ -13,6 +13,7 @@
 from sentry.sentry_apps.models.sentry_app_installation import SentryAppInstallation
 from sentry.sentry_apps.services.app.model import RpcSentryAppComponent, RpcSentryAppInstallation
 from sentry.sentry_apps.services.app.serial import serialize_sentry_app_installation
+from sentry.sentry_apps.utils.errors import SentryAppIntegratorError
 from sentry.utils import json
 
 
@@ -35,6 +36,17 @@ def _prepare_stacktrace_link(self) -> None:
         schema = self.component.app_schema
         uri = schema.get("uri")
 
+        if not self.install.sentry_app.webhook_url:
+            raise SentryAppIntegratorError(
+                message="Sentry app webhook_url is not configured",
+                webhook_context={
+                    "error_type": "MISSING_URL",
+                    "sentry_app_slug": self.install.sentry_app.slug,
+                    "uri": uri,
+                },
+                status_code=500,
+            )
+
         urlparts = list(urlparse(force_str(self.install.sentry_app.webhook_url)))
         urlparts[2] = str(uri)
 

src/sentry/sentry_apps/external_requests/alert_rule_action_requester.py

@@ -19,7 +19,7 @@
 )
 from sentry.sentry_apps.models.sentry_app_installation import SentryAppInstallation
 from sentry.sentry_apps.services.app.model import RpcSentryAppInstallation
-from sentry.sentry_apps.utils.errors import SentryAppErrorType
+from sentry.sentry_apps.utils.errors import SentryAppErrorType, SentryAppIntegratorError
 from sentry.utils import json
 
 DEFAULT_SUCCESS_MESSAGE = "Success!"
@@ -81,6 +81,15 @@ def run(self) -> SentryAppAlertRuleActionResult:
                     webhook_context={"error_type": halt_reason, **extras},
                     status_code=500,
                 )
+            except SentryAppIntegratorError as e:
+                lifecycle.record_halt(halt_reason=e, extra={**extras})
+                return SentryAppAlertRuleActionResult(
+                    success=False,
+                    message=e.message,
+                    error_type=SentryAppErrorType.INTEGRATOR,
+                    webhook_context={"error_type": e.webhook_context["error_type"], **extras},
+                    status_code=e.status_code,
+                )
             except Exception as e:
                 failure_reason = FAILURE_REASON_BASE.format(
                     SentryAppExternalRequestFailureReason.UNEXPECTED_ERROR
@@ -102,6 +111,19 @@ def run(self) -> SentryAppAlertRuleActionResult:
             )
 
     def _build_url(self) -> str:
+        if not self.sentry_app.webhook_url:
+            raise SentryAppIntegratorError(
+                message="Sentry app webhook_url is not configured",
+                webhook_context={
+                    "error_type": FAILURE_REASON_BASE.format(
+                        SentryAppExternalRequestFailureReason.MISSING_URL
+                    ),
+                    "sentry_app_slug": self.sentry_app.slug,
+                    "uri": self.uri,
+                },
+                status_code=500,
+            )
+
         urlparts = list(urlparse(self.sentry_app.webhook_url))
         urlparts[2] = self.uri
         return urlunparse(urlparts)

src/sentry/sentry_apps/external_requests/issue_link_requester.py

@@ -13,6 +13,7 @@
 from sentry.sentry_apps.external_requests.utils import send_and_save_sentry_app_request, validate
 from sentry.sentry_apps.metrics import (
     SentryAppEventType,
+    SentryAppExternalRequestFailureReason,
     SentryAppExternalRequestHaltReason,
     SentryAppInteractionEvent,
     SentryAppInteractionType,
@@ -125,6 +126,9 @@ def run(self) -> dict[str, Any]:
                     webhook_context={"error_type": BAD_RESPONSE_HALT_REASON, **extras},
                     status_code=500,
                 )
+            except SentryAppIntegratorError as e:
+                lifecycle.record_halt(halt_reason=e, extra={**extras})
+                raise
 
             if not self._validate_response(response):
                 extras["response"] = response
@@ -142,6 +146,19 @@ def run(self) -> dict[str, Any]:
             return response
 
     def _build_url(self) -> str:
+        if not self.sentry_app.webhook_url:
+            raise SentryAppIntegratorError(
+                message="Sentry app webhook_url is not configured",
+                webhook_context={
+                    "error_type": FAILURE_REASON_BASE.format(
+                        SentryAppExternalRequestFailureReason.MISSING_URL
+                    ),
+                    "sentry_app_slug": self.sentry_app.slug,
+                    "uri": self.uri,
+                },
+                status_code=500,
+            )
+
         urlparts = urlparse(self.sentry_app.webhook_url)
         return f"{urlparts.scheme}://{urlparts.netloc}{self.uri}"
 

src/sentry/sentry_apps/external_requests/select_requester.py

@@ -90,6 +90,10 @@ def run(self) -> SelectRequesterResult:
                     status_code=500,
                 ) from e
 
+            except SentryAppIntegratorError as e:
+                lifecycle.record_halt(halt_reason=e, extra={**extras})
+                raise
+
             except Exception as e:
                 failure_reason = FAILURE_REASON_BASE.format(
                     SentryAppExternalRequestFailureReason.UNEXPECTED_ERROR
@@ -133,6 +137,19 @@ def run(self) -> SelectRequesterResult:
                 raise
 
     def _build_url(self) -> str:
+        if not self.sentry_app.webhook_url:
+            raise SentryAppIntegratorError(
+                message="Sentry app webhook_url is not configured",
+                webhook_context={
+                    "error_type": FAILURE_REASON_BASE.format(
+                        SentryAppExternalRequestFailureReason.MISSING_URL
+                    ),
+                    "sentry_app_slug": self.sentry_app.slug,
+                    "uri": self.uri,
+                },
+                status_code=500,
+            )
+
         urlparts: list[str] = [url_part for url_part in urlparse(self.sentry_app.webhook_url)]
         urlparts[2] = self.uri
 

tests/sentry/sentry_apps/external_requests/test_alert_rule_action_requester.py

@@ -348,3 +348,38 @@ def test_makes_failed_request_with_sentry_error(
         assert_many_failure_metrics(
             mock_record=mock_record, messages_or_errors=[Exception(), Exception()]
         )
+
+    @responses.activate
+    @patch("sentry.sentry_apps.external_requests.utils.safe_urlopen")
+    @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
+    def test_makes_failed_request_with_missing_url(
+        self, mock_record: MagicMock, mock_urlopen: MagicMock
+    ) -> None:
+        mock_urlopen.side_effect = Exception()
+        self.sentry_app.webhook_url = ""
+        self.sentry_app.save()
+
+        result = SentryAppAlertRuleActionRequester(
+            install=self.install,
+            uri="/sentry/alert-rule",
+            fields=self.fields,
+        ).run()
+
+        assert not result["success"]
+        assert result["message"] == "Sentry app webhook_url is not configured"
+        assert result["error_type"] == SentryAppErrorType.INTEGRATOR
+        assert result["webhook_context"] == {
+            "error_type": FAILURE_REASON_BASE.format(
+                SentryAppExternalRequestFailureReason.MISSING_URL
+            ),
+            "uri": "/sentry/alert-rule",
+            "installation_uuid": self.install.uuid,
+            "sentry_app_slug": self.sentry_app.slug,
+        }
+
+        assert_count_of_metric(
+            mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=1
+        )
+        assert_count_of_metric(
+            mock_record=mock_record, outcome=EventLifecycleOutcome.HALTED, outcome_count=1
+        )

tests/sentry/sentry_apps/external_requests/test_issue_link_requester.py

@@ -10,7 +10,12 @@
     IssueLinkRequester,
     IssueRequestActionType,
 )
-from sentry.sentry_apps.metrics import SentryAppEventType, SentryAppExternalRequestHaltReason
+from sentry.sentry_apps.metrics import (
+    SentryAppEventType,
+    SentryAppExternalRequestFailureReason,
+    SentryAppExternalRequestHaltReason,
+)
+from sentry.sentry_apps.models.sentry_app import SentryApp
 from sentry.sentry_apps.services.app import app_service
 from sentry.sentry_apps.utils.errors import SentryAppIntegratorError
 from sentry.testutils.asserts import (
@@ -20,6 +25,7 @@
     assert_success_metric,
 )
 from sentry.testutils.cases import TestCase
+from sentry.testutils.silo import assume_test_silo_mode_of
 from sentry.users.services.user.serial import serialize_rpc_user
 from sentry.utils import json
 from sentry.utils.sentry_apps import SentryAppWebhookRequestsBuffer
@@ -267,3 +273,38 @@ def test_invalid_json_response(self, mock_record: MagicMock) -> None:
         assert_count_of_metric(
             mock_record=mock_record, outcome=EventLifecycleOutcome.HALTED, outcome_count=1
         )
+
+    @responses.activate
+    @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
+    def test_no_webhook_url_configured_response(self, mock_record: MagicMock) -> None:
+        with assume_test_silo_mode_of(SentryApp):
+            self.sentry_app.webhook_url = ""
+            self.sentry_app.save()
+
+        # Refresh install to ensure webhook_url is propagated correctly
+        self.install = app_service.get_many(filter=dict(installation_ids=[self.orm_install.id]))[0]
+        with pytest.raises(SentryAppIntegratorError) as exception_info:
+            IssueLinkRequester(
+                install=self.install,
+                group=self.group,
+                uri="/link-issue",
+                fields={},
+                user=self.rpc_user,
+                action=IssueRequestActionType("create"),
+            ).run()
+        assert exception_info.value.message == "Sentry app webhook_url is not configured"
+        assert exception_info.value.webhook_context == {
+            "error_type": FAILURE_REASON_BASE.format(
+                SentryAppExternalRequestFailureReason.MISSING_URL
+            ),
+            "uri": "/link-issue",
+            "sentry_app_slug": self.sentry_app.slug,
+        }
+
+        # SLO assertions
+        assert_count_of_metric(
+            mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=1
+        )
+        assert_count_of_metric(
+            mock_record=mock_record, outcome=EventLifecycleOutcome.HALTED, outcome_count=1
+        )

tests/sentry/sentry_apps/external_requests/test_select_requester.py

@@ -5,12 +5,16 @@
 from requests import HTTPError
 
 from sentry.integrations.types import EventLifecycleOutcome
-from sentry.sentry_apps.external_requests.select_requester import SelectRequester
+from sentry.sentry_apps.external_requests.select_requester import (
+    FAILURE_REASON_BASE,
+    SelectRequester,
+)
 from sentry.sentry_apps.metrics import (
     SentryAppEventType,
     SentryAppExternalRequestFailureReason,
     SentryAppExternalRequestHaltReason,
 )
+from sentry.sentry_apps.models.sentry_app import SentryApp
 from sentry.sentry_apps.services.app import app_service
 from sentry.sentry_apps.utils.errors import SentryAppIntegratorError, SentryAppSentryError
 from sentry.testutils.asserts import (
@@ -21,6 +25,7 @@
     assert_success_metric,
 )
 from sentry.testutils.cases import TestCase
+from sentry.testutils.silo import assume_test_silo_mode_of
 from sentry.utils.sentry_apps import SentryAppWebhookRequestsBuffer
 
 
@@ -339,3 +344,34 @@ def test_unexpected_exception(
         assert_count_of_metric(
             mock_record=mock_record, outcome=EventLifecycleOutcome.FAILURE, outcome_count=1
         )
+
+    @responses.activate
+    @patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
+    def test_sentry_app_integrator_error(self, mock_record: MagicMock) -> None:
+        with assume_test_silo_mode_of(SentryApp):
+            self.sentry_app.webhook_url = ""
+            self.sentry_app.save()
+        self.install = app_service.get_many(filter=dict(installation_ids=[self.orm_install.id]))[0]
+
+        uri = "asdhbaljkdnaklskand"
+        with pytest.raises(SentryAppIntegratorError) as exception_info:
+            SelectRequester(
+                install=self.install,
+                project_slug=self.project.slug,
+                uri=uri,
+            ).run()
+        assert exception_info.value.message == "Sentry app webhook_url is not configured"
+        assert exception_info.value.webhook_context == {
+            "error_type": FAILURE_REASON_BASE.format(
+                SentryAppExternalRequestFailureReason.MISSING_URL
+            ),
+            "sentry_app_slug": self.sentry_app.slug,
+            "uri": uri,
+        }
+
+        assert_count_of_metric(
+            mock_record=mock_record, outcome=EventLifecycleOutcome.STARTED, outcome_count=1
+        )
+        assert_count_of_metric(
+            mock_record=mock_record, outcome=EventLifecycleOutcome.HALTED, outcome_count=1
+        )