diff --git a/upload/tests/views/test_bundle_analysis.py b/upload/tests/views/test_bundle_analysis.py index 3e65c0b7c0..b227081251 100644 --- a/upload/tests/views/test_bundle_analysis.py +++ b/upload/tests/views/test_bundle_analysis.py @@ -104,6 +104,7 @@ def test_upload_bundle_analysis_success(db, client, mocker, mock_redis): "endpoint": "bundle_analysis", "is_using_shelter": "no", "position": "end", + "result": "success", }, ) @@ -207,6 +208,7 @@ def test_upload_bundle_analysis_success_shelter(db, client, mocker, mock_redis): "endpoint": "bundle_analysis", "is_using_shelter": "no", "position": "end", + "result": "success", }, ) @@ -245,6 +247,7 @@ def test_upload_bundle_analysis_org_token(db, client, mocker, mock_redis): "endpoint": "bundle_analysis", "is_using_shelter": "no", "position": "end", + "result": "success", }, ) @@ -292,6 +295,7 @@ def test_upload_bundle_analysis_existing_commit(db, client, mocker, mock_redis): "endpoint": "bundle_analysis", "is_using_shelter": "no", "position": "end", + "result": "success", }, ) @@ -340,7 +344,8 @@ def test_upload_bundle_analysis_missing_args(db, client, mocker, mock_redis): "action": "bundle_analysis", "endpoint": "bundle_analysis", "is_using_shelter": "no", - "position": "start", + "position": "end", + "result": "bad_request", }, ) @@ -412,6 +417,7 @@ def test_upload_bundle_analysis_github_oidc_auth( "endpoint": "bundle_analysis", "is_using_shelter": "no", "position": "end", + "result": "success", }, ) @@ -472,6 +478,7 @@ def test_upload_bundle_analysis_measurement_datasets_created( "endpoint": "bundle_analysis", "is_using_shelter": "no", "position": "end", + "result": "success", }, ) @@ -533,6 +540,7 @@ def test_upload_bundle_analysis_measurement_timeseries_disabled( "endpoint": "bundle_analysis", "is_using_shelter": "no", "position": "end", + "result": "success", }, ) @@ -574,7 +582,8 @@ def test_upload_bundle_analysis_no_repo(db, client, mocker, mock_redis): "action": "bundle_analysis", "endpoint": "bundle_analysis", "is_using_shelter": "no", - "position": "start", + "position": "end", + "result": "error", }, ) @@ -629,6 +638,7 @@ def test_upload_bundle_analysis_tokenless_success(db, client, mocker, mock_redis "endpoint": "bundle_analysis", "is_using_shelter": "no", "position": "end", + "result": "success", }, ) @@ -810,3 +820,34 @@ def test_upload_bundle_analysis_tokenless_mismatched_branch( assert res.status_code == 401 assert res.json() == {"detail": "Not valid tokenless upload"} assert not upload.called + + +def test_upload_bundle_analysis_view_exception_handling(db, client, mocker, mock_redis): + try: + with patch( + "upload.views.bundle_analysis.BundleAnalysisView._handle_upload", + side_effect=Exception("Test Exception"), + ): + client = APIClient() + repository = RepositoryFactory.create() + client.credentials(HTTP_AUTHORIZATION=f"token {repository.upload_token}") + + mock_inc_counter = mocker.patch("upload.views.bundle_analysis.inc_counter") + + client.post( + reverse("upload-bundle-analysis"), + { + "commit": "6fd5b89357fc8cdf34d6197549ac7c6d7e5977ef", + "slug": f"{repository.author.username}::::{repository.name}", + }, + format="json", + ) + except Exception as e: + # Check that the Test Exception was raised and the inc_counter went up + assert str(e) == "Test Exception" + mock_inc_counter.assert_any_call( + ANY, + labels=mocker.ANY, + ) + labels = mock_inc_counter.call_args[1]["labels"] + assert labels["result"] == "error" diff --git a/upload/views/bundle_analysis.py b/upload/views/bundle_analysis.py index 6bfc369654..c500868c4f 100644 --- a/upload/views/bundle_analysis.py +++ b/upload/views/bundle_analysis.py @@ -1,6 +1,6 @@ import logging import uuid -from typing import Any, Callable +from typing import Any, Callable, Tuple from django.conf import settings from django.http import HttpRequest @@ -48,6 +48,7 @@ "endpoint", "is_using_shelter", "position", + "result", ], ) @@ -85,23 +86,13 @@ class BundleAnalysisView(APIView, ShelterMixin): def get_exception_handler(self) -> Callable: return repo_auth_custom_exception_handler - def post(self, request: HttpRequest) -> Response: - labels = generate_upload_prometheus_metrics_labels( - action="bundle_analysis", - endpoint="bundle_analysis", - request=self.request, - is_shelter_request=self.is_shelter_request(), - position="start", - include_empty_labels=False, - ) - inc_counter( - BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER, - labels=labels, - ) - + def _handle_upload(self, request: HttpRequest) -> Tuple[str, Response]: serializer = UploadSerializer(data=request.data) if not serializer.is_valid(): - return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) + return ( + "bad_request", + Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST), + ) data = serializer.validated_data if isinstance(request.user, Owner): @@ -191,18 +182,6 @@ def post(self, request: HttpRequest) -> Response: task_arguments=task_arguments, ), ) - labels = generate_upload_prometheus_metrics_labels( - action="bundle_analysis", - endpoint="bundle_analysis", - request=self.request, - is_shelter_request=self.is_shelter_request(), - position="end", - include_empty_labels=False, - ) - inc_counter( - BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER, - labels=labels, - ) dispatch_upload_task( task_arguments, @@ -235,4 +214,40 @@ def post(self, request: HttpRequest) -> Response: ), ) - return Response({"url": url}, status=201) + return ("success", Response({"url": url}, status=201)) + + def post(self, request: HttpRequest) -> Response: + labels = generate_upload_prometheus_metrics_labels( + action="bundle_analysis", + endpoint="bundle_analysis", + request=self.request, + is_shelter_request=self.is_shelter_request(), + position="start", + include_empty_labels=False, + ) + labels["result"] = "pending" + inc_counter( + BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER, + labels=labels, + ) + + try: + upload_result, response = self._handle_upload(request) + return response + except Exception as e: + log.error( + "Error handling bundle analysis upload", + extra=dict( + error=e, + ), + exc_info=True, + ) + upload_result = "error" + raise + finally: + labels["position"] = "end" + labels["result"] = upload_result + inc_counter( + BUNDLE_ANALYSIS_UPLOAD_VIEWS_COUNTER, + labels=labels, + )