From bed07163c13b72b4c06274e1f89da58103544962 Mon Sep 17 00:00:00 2001 From: Ildar Iskhakov Date: Thu, 31 Oct 2024 16:24:42 +0800 Subject: [PATCH 1/3] SocialAuthAuthCanceledExceptionMiddleware should only process social auth related exceptions --- engine/apps/social_auth/middlewares.py | 49 ++++++++++++++------------ 1 file changed, 27 insertions(+), 22 deletions(-) diff --git a/engine/apps/social_auth/middlewares.py b/engine/apps/social_auth/middlewares.py index 19180ebc4..09583dcd8 100644 --- a/engine/apps/social_auth/middlewares.py +++ b/engine/apps/social_auth/middlewares.py @@ -16,25 +16,30 @@ class SocialAuthAuthCanceledExceptionMiddleware(SocialAuthExceptionMiddleware): def process_exception(self, request, exception): - backend = getattr(exception, "backend", None) - url_builder = UIURLBuilder(request.user.organization) - url_builder_function = url_builder.chatops - - if backend is not None and isinstance(backend, LoginSlackOAuth2V2): - url_builder_function = url_builder.user_profile - - if exception: - logger.warning(f"SocialAuthAuthCanceledExceptionMiddleware.process_exception: {exception}") - - if isinstance(exception, exceptions.AuthCanceled): - # if user canceled authentication, redirect them to the previous page using the same link - # as we used to redirect after auth/install - return redirect(url_builder_function()) - elif isinstance(exception, exceptions.AuthFailed): - # if authentication was failed, redirect user to the plugin page using the same link - # as we used to redirect after auth/install with error flag - return redirect(url_builder_function(f"?slack_error={SLACK_AUTH_FAILED}")) - elif isinstance(exception, KeyError) and REDIRECT_AFTER_SLACK_INSTALL in exception.args: - return HttpResponse(status=status.HTTP_401_UNAUTHORIZED) - elif isinstance(exception, InstallMultiRegionSlackException): - return redirect(url_builder_function(f"?tab=Slack&slack_error={SLACK_REGION_ERROR}")) + strategy = getattr(request, "social_strategy", None) + if strategy is None or self.raise_exception(request, exception): + return + + if isinstance(exception, exceptions.SocialAuthBaseException): + backend = getattr(exception, "backend", None) + url_builder = UIURLBuilder(request.user.organization) + url_builder_function = url_builder.chatops + + if backend is not None and isinstance(backend, LoginSlackOAuth2V2): + url_builder_function = url_builder.user_profile + + if exception: + logger.warning(f"SocialAuthAuthCanceledExceptionMiddleware.process_exception: {exception}") + + if isinstance(exception, exceptions.AuthCanceled): + # if user canceled authentication, redirect them to the previous page using the same link + # as we used to redirect after auth/install + return redirect(url_builder_function()) + elif isinstance(exception, exceptions.AuthFailed): + # if authentication was failed, redirect user to the plugin page using the same link + # as we used to redirect after auth/install with error flag + return redirect(url_builder_function(f"?slack_error={SLACK_AUTH_FAILED}")) + elif isinstance(exception, KeyError) and REDIRECT_AFTER_SLACK_INSTALL in exception.args: + return HttpResponse(status=status.HTTP_401_UNAUTHORIZED) + elif isinstance(exception, InstallMultiRegionSlackException): + return redirect(url_builder_function(f"?tab=Slack&slack_error={SLACK_REGION_ERROR}")) From 7f759256e10e2a719966765432ff0fd0a2940b6e Mon Sep 17 00:00:00 2001 From: Ildar Iskhakov Date: Mon, 4 Nov 2024 19:48:58 +0800 Subject: [PATCH 2/3] Raise exceptions instead of returning 400 and 500s --- Tiltfile | 2 +- engine/apps/api/views/alert_group.py | 24 +++---- .../apps/api/views/alert_receive_channel.py | 4 +- .../views/alert_receive_channel_template.py | 6 +- engine/apps/api/views/alerts.py | 6 +- engine/apps/api/views/auth.py | 3 +- engine/apps/api/views/escalation_chain.py | 6 +- engine/apps/api/views/labels.py | 2 + engine/apps/api/views/live_setting.py | 6 +- engine/apps/api/views/on_call_shifts.py | 3 +- engine/apps/api/views/organization.py | 3 +- engine/apps/api/views/user.py | 71 ++++++++----------- engine/common/api_helpers/exceptions.py | 10 +++ 13 files changed, 72 insertions(+), 74 deletions(-) diff --git a/Tiltfile b/Tiltfile index 264424161..217978fa2 100644 --- a/Tiltfile +++ b/Tiltfile @@ -44,7 +44,7 @@ def extra_deps(): return grafana_deps -allow_k8s_contexts(["kind-kind"]) +allow_k8s_contexts(["kind-kind", "orbstack"]) # Build the image including frontend folder for pytest docker_build_sub( diff --git a/engine/apps/api/views/alert_group.py b/engine/apps/api/views/alert_group.py index 117fb9ce9..6bfa93aa3 100644 --- a/engine/apps/api/views/alert_group.py +++ b/engine/apps/api/views/alert_group.py @@ -29,7 +29,7 @@ from apps.labels.utils import is_labels_feature_enabled from apps.mobile_app.auth import MobileAppAuthTokenAuthentication from apps.user_management.models import Team, User -from common.api_helpers.exceptions import BadRequest +from common.api_helpers.exceptions import BadRequest, Forbidden from common.api_helpers.filters import ( NO_TEAM_VALUE, DateRangeFilterMixin, @@ -210,12 +210,9 @@ def retrieve(self, request, *args, **kwargs): if obj_team is None: obj_team = Team(public_primary_key=None, name="General", email=None, avatar_url=None) - return Response( - data={"error_code": "wrong_team", "owner_team": TeamSerializer(obj_team).data}, - status=status.HTTP_403_FORBIDDEN, - ) + raise Forbidden(detail={"error_code": "wrong_team", "owner_team": TeamSerializer(obj_team).data}) - return Response(data={"error_code": "wrong_team"}, status=status.HTTP_403_FORBIDDEN) + raise Forbidden(detail={"error_code": "wrong_team"}) class AlertGroupSearchFilter(SearchFilter): @@ -509,12 +506,11 @@ def resolve(self, request, pk): else: # Check resolution note required setting only if resolution_note_text was not provided. if organization.is_resolution_note_required and not alert_group.has_resolution_notes: - return Response( - data={ + raise BadRequest( + detail={ "code": AlertGroupAPIError.RESOLUTION_NOTE_REQUIRED.value, "detail": "Alert group without resolution note cannot be resolved due to organization settings", - }, - status=status.HTTP_400_BAD_REQUEST, + } ) alert_group.resolve_by_user_or_backsync(self.request.user, action_source=ActionSource.WEB) return Response(AlertGroupSerializer(alert_group, context={"request": self.request}).data) @@ -558,11 +554,11 @@ def attach(self, request, pk=None): try: root_alert_group = self.get_queryset().get(public_primary_key=request.data["root_alert_group_pk"]) except AlertGroup.DoesNotExist: - return Response(status=status.HTTP_400_BAD_REQUEST) + raise BadRequest() if root_alert_group.resolved or root_alert_group.root_alert_group is not None: - return Response(status=status.HTTP_400_BAD_REQUEST) + raise BadRequest() if root_alert_group == alert_group: - return Response(status=status.HTTP_400_BAD_REQUEST) + raise BadRequest() alert_group.attach_by_user(self.request.user, root_alert_group, action_source=ActionSource.WEB) return Response(AlertGroupSerializer(alert_group, context={"request": self.request}).data) @@ -832,7 +828,7 @@ def bulk_action(self, request): kwargs = {} if action_name not in AlertGroup.BULK_ACTIONS: - return Response("Unknown action", status=status.HTTP_400_BAD_REQUEST) + raise BadRequest(detail="Unknown action") if action_name == AlertGroup.SILENCE: if delay is None: diff --git a/engine/apps/api/views/alert_receive_channel.py b/engine/apps/api/views/alert_receive_channel.py index e70be1429..2dd7346b1 100644 --- a/engine/apps/api/views/alert_receive_channel.py +++ b/engine/apps/api/views/alert_receive_channel.py @@ -38,7 +38,7 @@ from apps.integrations.legacy_prefix import has_legacy_prefix, remove_legacy_prefix from apps.labels.utils import is_labels_feature_enabled from apps.mobile_app.auth import MobileAppAuthTokenAuthentication -from common.api_helpers.exceptions import BadRequest +from common.api_helpers.exceptions import BadRequest, Conflict from common.api_helpers.filters import NO_TEAM_VALUE, ByTeamModelFieldFilterMixin, TeamModelMultipleChoiceFilter from common.api_helpers.mixins import ( CreateSerializerMixin, @@ -611,7 +611,7 @@ def validate_name(self, request): organization = self.request.auth.organization name_used = AlertReceiveChannel.objects.filter(organization=organization, verbal_name=verbal_name).exists() if name_used: - r = Response(status=status.HTTP_409_CONFLICT) + raise Conflict() else: r = Response(status=status.HTTP_200_OK) diff --git a/engine/apps/api/views/alert_receive_channel_template.py b/engine/apps/api/views/alert_receive_channel_template.py index 579ae3aa7..e17a571df 100644 --- a/engine/apps/api/views/alert_receive_channel_template.py +++ b/engine/apps/api/views/alert_receive_channel_template.py @@ -1,11 +1,11 @@ -from rest_framework import mixins, status, viewsets +from rest_framework import mixins, viewsets from rest_framework.permissions import IsAuthenticated -from rest_framework.response import Response from apps.alerts.models import AlertReceiveChannel from apps.api.permissions import RBACPermission from apps.api.serializers.alert_receive_channel import AlertReceiveChannelTemplatesSerializer from apps.auth_token.auth import PluginAuthentication +from common.api_helpers.exceptions import BadRequest from common.api_helpers.mixins import PublicPrimaryKeyMixin, TeamFilteringMixin from common.insight_log import EntityEvent, write_resource_insight_log from common.jinja_templater.apply_jinja_template import JinjaTemplateError @@ -47,7 +47,7 @@ def update(self, request, *args, **kwargs): try: result = super().update(request, *args, **kwargs) except JinjaTemplateError as e: - return Response(e.fallback_message, status.HTTP_400_BAD_REQUEST) + raise BadRequest(e.fallback_message) instance = self.get_object() new_state = instance.insight_logs_serialized write_resource_insight_log( diff --git a/engine/apps/api/views/alerts.py b/engine/apps/api/views/alerts.py index 2c09739a5..a0ddd96e3 100644 --- a/engine/apps/api/views/alerts.py +++ b/engine/apps/api/views/alerts.py @@ -1,4 +1,4 @@ -from rest_framework import status +from rest_framework.exceptions import NotFound from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response from rest_framework.views import APIView @@ -16,8 +16,8 @@ def get(self, request, id): try: alert = Alert.objects.get(public_primary_key=id) except Alert.DoesNotExist: - return Response(status=status.HTTP_404_NOT_FOUND) + raise NotFound() if alert.group.channel.organization != request.auth.organization: - return Response(status=status.HTTP_404_NOT_FOUND) + raise NotFound() serializer = AlertRawSerializer(alert) return Response(serializer.data) diff --git a/engine/apps/api/views/auth.py b/engine/apps/api/views/auth.py index 63aafc7f8..c17997852 100644 --- a/engine/apps/api/views/auth.py +++ b/engine/apps/api/views/auth.py @@ -21,6 +21,7 @@ from apps.grafana_plugin.ui_url_builder import UIURLBuilder from apps.slack.installation import install_slack_integration from apps.social_auth.backends import SLACK_INSTALLATION_BACKEND, LoginSlackOAuth2V2 +from common.api_helpers.exceptions import InternalServerError logger = logging.getLogger(__name__) @@ -59,7 +60,7 @@ def overridden_login_social_auth(request: Request, backend: str) -> Response: return Response("slack integration installed", 201) except Exception as e: logger.exception("overridden_login_social_auth: Failed to install slack via chatops-proxy: %s", e) - return Response({"error": "something went wrong, try again later"}, 500) + raise InternalServerError("Something went wrong, try again later") else: # Otherwise use social-auth. url_to_redirect_to = do_auth(request.backend, redirect_name=REDIRECT_FIELD_NAME).url diff --git a/engine/apps/api/views/escalation_chain.py b/engine/apps/api/views/escalation_chain.py index e48a96a48..450a5a1e1 100644 --- a/engine/apps/api/views/escalation_chain.py +++ b/engine/apps/api/views/escalation_chain.py @@ -2,7 +2,7 @@ from django_filters import rest_framework as filters from drf_spectacular.utils import PolymorphicProxySerializer, extend_schema, extend_schema_view, inline_serializer from emoji import emojize -from rest_framework import serializers, status, viewsets +from rest_framework import serializers, viewsets from rest_framework.decorators import action from rest_framework.filters import SearchFilter from rest_framework.permissions import IsAuthenticated @@ -18,7 +18,7 @@ from apps.auth_token.auth import PluginAuthentication from apps.mobile_app.auth import MobileAppAuthTokenAuthentication from apps.user_management.models import Team -from common.api_helpers.exceptions import BadRequest +from common.api_helpers.exceptions import BadRequest, Forbidden from common.api_helpers.filters import ( NO_TEAM_VALUE, ByTeamModelFieldFilterMixin, @@ -164,7 +164,7 @@ def copy(self, request, pk): try: team = request.user.available_teams.get(public_primary_key=team_id) if team_id else None except Team.DoesNotExist: - return Response(data={"error_code": "wrong_team"}, status=status.HTTP_403_FORBIDDEN) + raise Forbidden(detail={"error_code": "wrong_team"}) copy = obj.make_copy(name, team) serializer = self.get_serializer(copy) write_resource_insight_log( diff --git a/engine/apps/api/views/labels.py b/engine/apps/api/views/labels.py index d27215f2e..8c0e6efce 100644 --- a/engine/apps/api/views/labels.py +++ b/engine/apps/api/views/labels.py @@ -52,6 +52,8 @@ def get_keys(self, request): """List of labels keys""" organization = self.request.auth.organization keys, response = LabelsAPIClient(organization.grafana_url, organization.api_token).get_keys() + # TODO: Change all error responses here and below in the file to raise exceptions + # LabelsRepoAPIException should be raised as drf APIException, so we can handle them in handle_exception return Response(keys, status=response.status_code) @extend_schema(responses=LabelOptionSerializer) diff --git a/engine/apps/api/views/live_setting.py b/engine/apps/api/views/live_setting.py index 3068427a9..44db94734 100644 --- a/engine/apps/api/views/live_setting.py +++ b/engine/apps/api/views/live_setting.py @@ -1,8 +1,8 @@ from contextlib import suppress from django.conf import settings -from django.http import HttpResponse -from rest_framework import status, viewsets +from rest_framework import viewsets +from rest_framework.exceptions import NotFound from rest_framework.permissions import IsAuthenticated from telegram import error @@ -32,7 +32,7 @@ class LiveSettingViewSet(PublicPrimaryKeyMixin[LiveSetting], viewsets.ModelViewS def dispatch(self, request, *args, **kwargs): if not settings.FEATURE_LIVE_SETTINGS_ENABLED: - return HttpResponse(status=status.HTTP_404_NOT_FOUND) + raise NotFound() return super().dispatch(request, *args, **kwargs) diff --git a/engine/apps/api/views/on_call_shifts.py b/engine/apps/api/views/on_call_shifts.py index 99b0ec4c2..a2cb7f266 100644 --- a/engine/apps/api/views/on_call_shifts.py +++ b/engine/apps/api/views/on_call_shifts.py @@ -101,8 +101,7 @@ def preview(self, request): user_tz, starting_date, days = get_date_range_from_request(self.request) serializer = self.get_serializer(data=request.data) - if not serializer.is_valid(): - return Response(data=serializer.errors, status=status.HTTP_400_BAD_REQUEST) + serializer.is_valid(raise_exception=True) validated_data = serializer._correct_validated_data( serializer.validated_data["type"], serializer.validated_data diff --git a/engine/apps/api/views/organization.py b/engine/apps/api/views/organization.py index 8b6f5d70b..1218aab55 100644 --- a/engine/apps/api/views/organization.py +++ b/engine/apps/api/views/organization.py @@ -11,6 +11,7 @@ from apps.base.messaging import get_messaging_backend_from_id from apps.mobile_app.auth import MobileAppAuthTokenAuthentication from apps.telegram.client import TelegramClient +from common.api_helpers.exceptions import BadRequest from common.insight_log import EntityEvent, write_resource_insight_log @@ -102,7 +103,7 @@ def get(self, request): backend_id = request.query_params.get("backend") backend = get_messaging_backend_from_id(backend_id) if backend is None: - return Response(status=status.HTTP_400_BAD_REQUEST) + raise BadRequest() code = backend.generate_channel_verification_code(organization) return Response(code) diff --git a/engine/apps/api/views/user.py b/engine/apps/api/views/user.py index 56c46b35b..13a9e90df 100644 --- a/engine/apps/api/views/user.py +++ b/engine/apps/api/views/user.py @@ -13,7 +13,7 @@ from drf_spectacular.utils import PolymorphicProxySerializer, extend_schema, inline_serializer from rest_framework import mixins, serializers, status, viewsets from rest_framework.decorators import action -from rest_framework.exceptions import NotFound +from rest_framework.exceptions import MethodNotAllowed, NotFound from rest_framework.filters import SearchFilter from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response @@ -69,7 +69,7 @@ from apps.telegram.client import TelegramClient from apps.telegram.models import TelegramVerificationCode from apps.user_management.models import Team, User -from common.api_helpers.exceptions import Conflict +from common.api_helpers.exceptions import BadRequest, Conflict, Forbidden, InternalServerError, ServiceUnavailable from common.api_helpers.filters import ByTeamModelFieldFilterMixin, TeamModelMultipleChoiceFilter from common.api_helpers.mixins import PublicPrimaryKeyMixin from common.api_helpers.paginators import HundredPageSizePaginator @@ -450,10 +450,8 @@ def wrong_team_response(self) -> Response: general_team = Team(public_primary_key=None, name="General", email=None, avatar_url=None) - return Response( - data={"error_code": "wrong_team", "owner_team": TeamSerializer(general_team).data}, - status=status.HTTP_403_FORBIDDEN, - ) + # TODO: fix here + raise Forbidden({"error_code": "wrong_team", "owner_team": TeamSerializer(general_team).data}) @extend_schema(responses={status.HTTP_200_OK: resolve_type_hint(typing.List[str])}) @action(detail=False, methods=["get"]) @@ -470,7 +468,7 @@ def get_verification_code(self, request, pk) -> Response: valid = check_recaptcha_internal_api(request, "mobile_verification_code") if not valid: logger.warning("get_verification_code: invalid reCAPTCHA validation") - return Response("failed reCAPTCHA check", status=status.HTTP_400_BAD_REQUEST) + raise BadRequest(detail="failed reCAPTCHA check") logger.info('get_verification_code: pass reCAPTCHA validation"') user = self.get_object() @@ -478,15 +476,13 @@ def get_verification_code(self, request, pk) -> Response: try: phone_backend.send_verification_sms(user) except NumberAlreadyVerified: - return Response("Phone number already verified", status=status.HTTP_400_BAD_REQUEST) + raise BadRequest(detail="Phone number already verified") except PhoneNumberBanned: - return Response("Phone number has been banned", status=status.HTTP_403_FORBIDDEN) + raise Forbidden(detail="Phone number has been banned") except FailedToStartVerification as e: return handle_phone_notificator_failed(e) except ProviderNotSupports: - return Response( - "Phone provider not supports sms verification", status=status.HTTP_500_INTERNAL_SERVER_ERROR - ) + raise InternalServerError(detail="Phone provider does not support sms verification") return Response(status=status.HTTP_200_OK) @action( @@ -499,7 +495,7 @@ def get_verification_call(self, request, pk) -> Response: valid = check_recaptcha_internal_api(request, "mobile_verification_code") if not valid: logger.warning("get_verification_code_via_call: invalid reCAPTCHA validation") - return Response("failed reCAPTCHA check", status=status.HTTP_400_BAD_REQUEST) + raise BadRequest(detail="failed reCAPTCHA check") logger.info('get_verification_code_via_call: pass reCAPTCHA validation"') user = self.get_object() @@ -507,15 +503,13 @@ def get_verification_call(self, request, pk) -> Response: try: phone_backend.make_verification_call(user) except NumberAlreadyVerified: - return Response("Phone number already verified", status=status.HTTP_400_BAD_REQUEST) + raise BadRequest(detail="Phone number already verified") except PhoneNumberBanned: - return Response("Phone number has been banned", status=status.HTTP_403_FORBIDDEN) + raise (Forbidden(detail="Phone number has been banned")) except FailedToStartVerification as e: return handle_phone_notificator_failed(e) except ProviderNotSupports: - return Response( - "Phone provider not supports call verification", status=status.HTTP_500_INTERNAL_SERVER_ERROR - ) + raise InternalServerError(detail="Phone provider does not support call verification") return Response(status=status.HTTP_200_OK) @extend_schema(parameters=[inline_serializer(name="UserVerifyNumber", fields={"token": serializers.CharField()})]) @@ -528,7 +522,7 @@ def verify_number(self, request, pk) -> Response: target_user = self.get_object() code = request.query_params.get("token", None) if not code: - return Response("Invalid verification code", status=status.HTTP_400_BAD_REQUEST) + raise BadRequest(detail="Verification code is required") prev_state = target_user.insight_logs_serialized phone_backend = PhoneBackend() @@ -547,7 +541,7 @@ def verify_number(self, request, pk) -> Response: ) return Response(status=status.HTTP_200_OK) else: - return Response("Verification code is not correct", status=status.HTTP_400_BAD_REQUEST) + raise BadRequest(detail="Verification code is not correct") @action(detail=True, methods=["put"]) def forget_number(self, request, pk) -> Response: @@ -575,11 +569,11 @@ def make_test_call(self, request, pk) -> Response: phone_backend = PhoneBackend() phone_backend.make_test_call(user) except NumberNotVerified: - return Response("Phone number is not verified", status=status.HTTP_400_BAD_REQUEST) + raise BadRequest(detail="Phone number is not verified") except FailedToMakeCall as e: return handle_phone_notificator_failed(e) except ProviderNotSupports: - return Response("Phone provider not supports phone calls", status=status.HTTP_500_INTERNAL_SERVER_ERROR) + raise InternalServerError(detail="Phone provider does not support phone calls") return Response(status=status.HTTP_200_OK) @@ -590,11 +584,11 @@ def send_test_sms(self, request, pk) -> Response: phone_backend = PhoneBackend() phone_backend.send_test_sms(user) except NumberNotVerified: - return Response("Phone number is not verified", status=status.HTTP_400_BAD_REQUEST) + raise BadRequest(detail="Phone number is not verified") except FailedToMakeCall as e: return handle_phone_notificator_failed(e) except ProviderNotSupports: - return Response("Phone provider not supports phone calls", status=status.HTTP_500_INTERNAL_SERVER_ERROR) + raise InternalServerError(detail="Phone provider doesn't support sms") return Response(status=status.HTTP_200_OK) @@ -613,15 +607,10 @@ def send_test_push(self, request, pk) -> Response: try: send_test_push(user, critical) except DeviceNotSet: - return Response( - data="Mobile device not connected", - status=status.HTTP_400_BAD_REQUEST, - ) + raise BadRequest(detail="Mobile device not connected") except Exception as e: logger.info(f"UserView.send_test_push: Unable to send test push due to {e}") - return Response( - data="Something went wrong while sending a test push", status=status.HTTP_500_INTERNAL_SERVER_ERROR - ) + raise InternalServerError(detail="Something went wrong while sending a test push") return Response(status=status.HTTP_200_OK) @extend_schema( @@ -636,7 +625,7 @@ def get_backend_verification_code(self, request, pk) -> Response: backend_id = request.query_params.get("backend") backend = get_messaging_backend_from_id(backend_id) if backend is None: - return Response(status=status.HTTP_400_BAD_REQUEST) + raise BadRequest() code = backend.generate_user_verification_code(user) return Response(code) @@ -655,7 +644,7 @@ def get_telegram_verification_code(self, request, pk) -> Response: user = self.get_object() if user.is_telegram_connected: - return Response("This user is already connected to a Telegram account", status=status.HTTP_400_BAD_REQUEST) + raise BadRequest(detail="This user is already connected to a Telegram account") try: existing_verification_code = user.telegram_verification_code @@ -704,7 +693,7 @@ def unlink_telegram(self, request, pk) -> Response: linked_user_id=user.public_primary_key, ) except TelegramToUserConnector.DoesNotExist: - return Response(status=status.HTTP_400_BAD_REQUEST) + raise BadRequest() return Response(status=status.HTTP_200_OK) @extend_schema( @@ -718,7 +707,7 @@ def unlink_backend(self, request, pk) -> Response: backend_id = request.query_params.get("backend") backend = get_messaging_backend_from_id(backend_id) if backend is None: - return Response(status=status.HTTP_400_BAD_REQUEST) + raise BadRequest() try: backend.unlink_user(user) @@ -730,7 +719,7 @@ def unlink_backend(self, request, pk) -> Response: linked_user_id=user.public_primary_key, ) except ObjectDoesNotExist: - return Response(status=status.HTTP_400_BAD_REQUEST) + raise BadRequest() return Response(status=status.HTTP_200_OK) @extend_schema( @@ -748,10 +737,10 @@ def upcoming_shifts(self, request, pk) -> Response: try: days = int(request.query_params.get("days", UPCOMING_SHIFTS_DEFAULT_DAYS)) except ValueError: - return Response(status=status.HTTP_400_BAD_REQUEST) + raise BadRequest(detail="Invalid days parameter") if days <= 0 or days > UPCOMING_SHIFTS_MAX_DAYS: - return Response(status=status.HTTP_400_BAD_REQUEST) + raise BadRequest(detail="Invalid days parameter") now = timezone.now() # filter user-related schedules @@ -844,7 +833,7 @@ def export_token(self, request, pk) -> Response: except UserScheduleExportAuthToken.DoesNotExist: raise NotFound return Response(status=status.HTTP_204_NO_CONTENT) - return Response(status=status.HTTP_405_METHOD_NOT_ALLOWED) + raise MethodNotAllowed() @extend_schema( responses=inline_serializer( @@ -886,6 +875,6 @@ def filters(self, request): def handle_phone_notificator_failed(exc: BaseFailed) -> Response: if exc.graceful_msg: - return Response(exc.graceful_msg, status=status.HTTP_400_BAD_REQUEST) + raise BadRequest(exc.graceful_msg) else: - return Response("Something went wrong", status=status.HTTP_503_SERVICE_UNAVAILABLE) + raise ServiceUnavailable(detail="Something went wrong") diff --git a/engine/common/api_helpers/exceptions.py b/engine/common/api_helpers/exceptions.py index e0cc1500a..9e86b5f87 100644 --- a/engine/common/api_helpers/exceptions.py +++ b/engine/common/api_helpers/exceptions.py @@ -23,3 +23,13 @@ class Conflict(APIException): status_code = 409 default_detail = "duplicate record found" default_code = "Conflict" + + +class ServiceUnavailable(APIException): + status_code = 503 + default_detail = "Service temporarily unavailable, try again later." + default_code = "service_unavailable" + + +class InternalServerError(APIException): + pass From 2ba6b4be648d1924ee57701f7ff6ef67891095d5 Mon Sep 17 00:00:00 2001 From: Ildar Iskhakov Date: Fri, 8 Nov 2024 19:58:51 +0800 Subject: [PATCH 3/3] Use string as required by the handler --- engine/apps/api/tests/test_alert_group.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/apps/api/tests/test_alert_group.py b/engine/apps/api/tests/test_alert_group.py index 8ee438b6b..b46ab621b 100644 --- a/engine/apps/api/tests/test_alert_group.py +++ b/engine/apps/api/tests/test_alert_group.py @@ -2149,7 +2149,7 @@ def test_alert_group_resolve_resolution_note( response = client.post(url, format="json", **make_user_auth_headers(user, token)) # check that resolution note is required assert response.status_code == status.HTTP_400_BAD_REQUEST - assert response.json()["code"] == AlertGroupAPIError.RESOLUTION_NOTE_REQUIRED.value + assert response.json()["code"] == str(AlertGroupAPIError.RESOLUTION_NOTE_REQUIRED.value) with patch( "apps.alerts.tasks.send_update_resolution_note_signal.send_update_resolution_note_signal.apply_async"