diff --git a/pyproject.toml b/pyproject.toml index 74f72e9e97e23d..4f98ad1274d9d7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -198,7 +198,6 @@ module = [ "sentry.incidents.endpoints.organization_incident_index", "sentry.incidents.subscription_processor", "sentry.incidents.tasks", - "sentry.integrations.api.bases.integration", "sentry.integrations.aws_lambda.integration", "sentry.integrations.bitbucket.client", "sentry.integrations.bitbucket.installed", diff --git a/src/sentry/api/base.py b/src/sentry/api/base.py index f71c6b2e4f9465..fddcd7dd555142 100644 --- a/src/sentry/api/base.py +++ b/src/sentry/api/base.py @@ -298,7 +298,7 @@ def permission_denied(self, request, message=None, code=None): super().permission_denied(request, message, code) - def handle_exception( # type: ignore[override] + def handle_exception_with_details( self, request: Request, exc: Exception, @@ -321,7 +321,7 @@ def handle_exception( # type: ignore[override] # Django REST Framework's built-in exception handler. If `settings.EXCEPTION_HANDLER` # exists and returns a response, that's used. Otherwise, `exc` is just re-raised # and caught below. - response = super().handle_exception(exc) + response = self.handle_exception(exc) except Exception as err: import sys import traceback @@ -456,7 +456,7 @@ def dispatch(self, request: Request, *args, **kwargs) -> Response: response = handler(request, *args, **kwargs) except Exception as exc: - response = self.handle_exception(request, exc) + response = self.handle_exception_with_details(request, exc) if origin: self.add_cors_headers(request, response) diff --git a/src/sentry/api/bases/project.py b/src/sentry/api/bases/project.py index a3d8331e5b7044..693828c767af83 100644 --- a/src/sentry/api/bases/project.py +++ b/src/sentry/api/bases/project.py @@ -207,7 +207,7 @@ def get_filter_params(self, request: Request, project, date_filter_optional=Fals return params - def handle_exception( # type: ignore[override] + def handle_exception_with_details( self, request: Request, exc: Exception, @@ -221,4 +221,4 @@ def handle_exception( # type: ignore[override] ) response["Location"] = exc.new_url return response - return super().handle_exception(request, exc, handler_context, scope) + return super().handle_exception_with_details(request, exc, handler_context, scope) diff --git a/src/sentry/integrations/api/bases/integration.py b/src/sentry/integrations/api/bases/integration.py index 2273d83622fd15..db35ffec89d120 100644 --- a/src/sentry/integrations/api/bases/integration.py +++ b/src/sentry/integrations/api/bases/integration.py @@ -42,14 +42,16 @@ class IntegrationEndpoint(ControlSiloOrganizationEndpoint): Baseclass for integration endpoints in control silo that need integration exception handling """ - def handle_exception( + def handle_exception_with_details( self, request: Request, exc: Exception, *args: Any, **kwds: Any, ) -> Response: - return _handle_exception(exc) or super().handle_exception(request, exc, *args, **kwds) + return _handle_exception(exc) or super().handle_exception_with_details( + request, exc, *args, **kwds + ) class RegionIntegrationEndpoint(OrganizationEndpoint): @@ -57,11 +59,13 @@ class RegionIntegrationEndpoint(OrganizationEndpoint): Baseclass for integration endpoints in region silo that need integration exception handling """ - def handle_exception( + def handle_exception_with_details( self, request: Request, exc: Exception, *args: Any, **kwds: Any, ) -> Response: - return _handle_exception(exc) or super().handle_exception(request, exc, *args, **kwds) + return _handle_exception(exc) or super().handle_exception_with_details( + request, exc, *args, **kwds + ) diff --git a/src/sentry/integrations/api/endpoints/integration_proxy.py b/src/sentry/integrations/api/endpoints/integration_proxy.py index 5db2f5848659fe..f0e86cf77b02d4 100644 --- a/src/sentry/integrations/api/endpoints/integration_proxy.py +++ b/src/sentry/integrations/api/endpoints/integration_proxy.py @@ -221,7 +221,7 @@ def http_method_not_allowed(self, request): ) return response - def handle_exception( # type: ignore[override] + def handle_exception_with_details( self, request: DRFRequest, exc: Exception, @@ -240,4 +240,4 @@ def handle_exception( # type: ignore[override] logger.info("hybrid_cloud.integration_proxy.host_timeout_error", extra=self.log_extra) return self.respond(status=exc.code) - return super().handle_exception(request, exc, handler_context, scope) + return super().handle_exception_with_details(request, exc, handler_context, scope) diff --git a/src/sentry/integrations/jira/webhooks/base.py b/src/sentry/integrations/jira/webhooks/base.py index 7676c2aa86ae1d..2df7f73b877169 100644 --- a/src/sentry/integrations/jira/webhooks/base.py +++ b/src/sentry/integrations/jira/webhooks/base.py @@ -37,7 +37,7 @@ class JiraWebhookBase(Endpoint, abc.ABC): def dispatch(self, request: Request, *args, **kwargs) -> Response: return super().dispatch(request, *args, **kwargs) - def handle_exception( + def handle_exception_with_details( self, request: Request, exc: Exception, @@ -108,7 +108,7 @@ def handle_exception( # This will log the error locally, capture the exception and send it to Sentry, and create a # generic 500/Internal Error response - return super().handle_exception(request, exc, handler_context, scope) + return super().handle_exception_with_details(request, exc, handler_context, scope) def get_token(self, request: Request) -> str: try: diff --git a/src/sentry/integrations/jira/webhooks/issue_updated.py b/src/sentry/integrations/jira/webhooks/issue_updated.py index 01b3202da3a789..c78025859d79bd 100644 --- a/src/sentry/integrations/jira/webhooks/issue_updated.py +++ b/src/sentry/integrations/jira/webhooks/issue_updated.py @@ -33,7 +33,7 @@ class JiraIssueUpdatedWebhook(JiraWebhookBase): Webhook hit by Jira whenever an issue is updated in Jira's database. """ - def handle_exception( + def handle_exception_with_details( self, request: Request, exc: Exception, @@ -45,7 +45,7 @@ def handle_exception( if response_option: return self.respond(response_option) - return super().handle_exception(request, exc, handler_context, scope) + return super().handle_exception_with_details(request, exc, handler_context, scope) def post(self, request: Request, *args, **kwargs) -> Response: token = self.get_token(request) diff --git a/tests/sentry/api/test_base.py b/tests/sentry/api/test_base.py index 5423734119b40f..2189019e8300a3 100644 --- a/tests/sentry/api/test_base.py +++ b/tests/sentry/api/test_base.py @@ -68,7 +68,7 @@ def __init__( ): # The error which will be thrown when a GET request is made self.error = error - # The argumets which will be passed on to `Endpoint.handle_exception` via `super` + # The argumets which will be passed on to `Endpoint.handle_exception_with_details` via `super` self.handler_context_arg = handler_context_arg self.scope_arg = scope_arg @@ -77,8 +77,10 @@ def __init__( def get(self, request): raise self.error - def handle_exception(self, request, exc, handler_context=None, scope=None): - return super().handle_exception(request, exc, self.handler_context_arg, self.scope_arg) + def handle_exception_with_details(self, request, exc, handler_context=None, scope=None): + return super().handle_exception_with_details( + request, exc, self.handler_context_arg, self.scope_arg + ) class DummyPaginationEndpoint(Endpoint): @@ -339,7 +341,7 @@ def test_handle_exception_when_super_returns_response( mock_endpoint = DummyErroringEndpoint.as_view(error=Exception("nope")) response = mock_endpoint(self.make_request(method="GET")) - # The endpoint should pass along the response generated by `APIView.handle_exception` + # The endpoint should pass along the response generated by `APIView.handle_exception_with_details` assert response == mock_super_handle_exception.return_value @mock.patch("rest_framework.views.APIView.handle_exception", new=reraise) diff --git a/tests/sentry/integrations/api/bases/test_integration.py b/tests/sentry/integrations/api/bases/test_integration.py index ddb38dc7a1a77e..362cc430fa90dd 100644 --- a/tests/sentry/integrations/api/bases/test_integration.py +++ b/tests/sentry/integrations/api/bases/test_integration.py @@ -12,7 +12,7 @@ @patch("sys.stderr.write") class IntegrationEndpointTest(TestCase): - # Since both `IntegrationEndpoint.handle_exception` and `Endpoint.handle_exception` potentially + # Since both `IntegrationEndpoint.handle_exception_with_details` and `Endpoint.handle_exception_with_details` potentially # run, and they both call their own module's copy of `capture_exception`, in order to prove that # neither one is not called, we assert on the underlying method from the SDK @patch("sentry_sdk.Scope.capture_exception") @@ -23,7 +23,7 @@ def test_handle_rest_framework_exception( exc.status_code = 400 # not possible to set in init request = Request(HttpRequest()) - resp = IntegrationEndpoint().handle_exception(request, exc) + resp = IntegrationEndpoint().handle_exception_with_details(request, exc) # `APIException`s are handled by Django REST Framework's built-in exception handler, which # doesn't log errors or report them to Sentry @@ -42,7 +42,7 @@ def test_handle_exception_503( except ApiError as exc: request = Request(HttpRequest()) - resp = IntegrationEndpoint().handle_exception(request, exc) + resp = IntegrationEndpoint().handle_exception_with_details(request, exc) mock_capture_exception.assert_called_with(exc) (((s,), _),) = mock_stderror_write.call_args_list @@ -63,7 +63,7 @@ def test_handle_exception_stdlib( except ValueError as exc: request = Request(HttpRequest()) - resp = IntegrationEndpoint().handle_exception(request, exc) + resp = IntegrationEndpoint().handle_exception_with_details(request, exc) assert mock_capture_exception.call_args.args[0] == exc (((s,), _),) = mock_stderror_write.call_args_list diff --git a/tests/sentry/integrations/jira/test_webhooks.py b/tests/sentry/integrations/jira/test_webhooks.py index 8eef96ebafd829..2dff97d6a4d2c8 100644 --- a/tests/sentry/integrations/jira/test_webhooks.py +++ b/tests/sentry/integrations/jira/test_webhooks.py @@ -236,7 +236,7 @@ def test_atlassian_pen_testing_bot( # This kind of error shouldn't be sent to Sentry assert mock_capture_exception.call_count == 0 - @patch("sentry.api.base.Endpoint.handle_exception", return_value=Response()) + @patch("sentry.api.base.Endpoint.handle_exception_with_details", return_value=Response()) def test_APIError_host_and_path_added_as_tags(self, mock_super_handle_exception: MagicMock): handler_error = ApiError("", url="http://maiseycharlie.jira.com/rest/api/3/dogs/tricks") mock_endpoint = MockErroringJiraEndpoint.as_view(error=handler_error) @@ -244,7 +244,7 @@ def test_APIError_host_and_path_added_as_tags(self, mock_super_handle_exception: request = self.make_request(method="GET") mock_endpoint(request) - # signature is super().handle_exception(request, error, handler_context, scope) + # signature is super().handle_exception_with_details(request, error, handler_context, scope) assert ( mock_super_handle_exception.call_args.args[3]._tags["jira.host"] == "maiseycharlie.jira.com" @@ -254,7 +254,7 @@ def test_APIError_host_and_path_added_as_tags(self, mock_super_handle_exception: == "/rest/api/3/dogs/tricks" ) - @patch("sentry.api.base.Endpoint.handle_exception", return_value=Response()) + @patch("sentry.api.base.Endpoint.handle_exception_with_details", return_value=Response()) def test_handles_xml_as_error_message(self, mock_super_handle_exception: MagicMock): """Moves the XML to `handler_context` and replaces it with a human-friendly message""" xml_string = '500PSQLException: too many connections' @@ -267,12 +267,12 @@ def test_handles_xml_as_error_message(self, mock_super_handle_exception: MagicMo request = self.make_request(method="GET") mock_endpoint(request) - # signature is super().handle_exception(request, error, handler_context, scope) + # signature is super().handle_exception_with_details(request, error, handler_context, scope) assert mock_super_handle_exception.call_args.args[1] == handler_error assert str(handler_error) == "Unknown error when requesting /rest/api/3/dogs/tricks" assert mock_super_handle_exception.call_args.args[2]["xml_response"] == xml_string - @patch("sentry.api.base.Endpoint.handle_exception", return_value=Response()) + @patch("sentry.api.base.Endpoint.handle_exception_with_details", return_value=Response()) def test_handles_html_as_error_message(self, mock_super_handle_exception: MagicMock): """Moves the HTML to `handler_context` and replaces it with a human-friendly message""" html_strings = [ @@ -291,12 +291,12 @@ def test_handles_html_as_error_message(self, mock_super_handle_exception: MagicM request = self.make_request(method="GET") mock_endpoint(request) - # signature is super().handle_exception(request, error, handler_context, scope) + # signature is super().handle_exception_with_details(request, error, handler_context, scope) assert mock_super_handle_exception.call_args.args[1] == handler_error assert str(handler_error) == "Unknown error when requesting /rest/api/3/dogs/tricks" assert mock_super_handle_exception.call_args.args[2]["html_response"] == html_string - @patch("sentry.api.base.Endpoint.handle_exception", return_value=Response()) + @patch("sentry.api.base.Endpoint.handle_exception_with_details", return_value=Response()) def test_replacement_error_messages(self, mock_super_handle_exception: MagicMock): replacement_messages_by_code = { 429: "Rate limit hit when requesting /rest/api/3/dogs/tricks", @@ -316,12 +316,12 @@ def test_replacement_error_messages(self, mock_super_handle_exception: MagicMock request = self.make_request(method="GET") mock_endpoint(request) - # signature is super().handle_exception(request, error, handler_context, scope) + # signature is super().handle_exception_with_details(request, error, handler_context, scope) assert mock_super_handle_exception.call_args.args[1] == handler_error assert str(handler_error) == new_message @patch("sentry.integrations.jira.webhooks.base.logger") - @patch("sentry.api.base.Endpoint.handle_exception", return_value=Response()) + @patch("sentry.api.base.Endpoint.handle_exception_with_details", return_value=Response()) def test_unexpected_jira_errors( self, mock_super_handle_exception: MagicMock, mock_logger: MagicMock ):