diff --git a/openedx/core/djangoapps/enrollments/tests/test_views.py b/openedx/core/djangoapps/enrollments/tests/test_views.py index cffaced0fbfe..9309cbd5949c 100644 --- a/openedx/core/djangoapps/enrollments/tests/test_views.py +++ b/openedx/core/djangoapps/enrollments/tests/test_views.py @@ -2643,3 +2643,151 @@ def test_count_reflects_user_enrollment_count(self): assert response.status_code == status.HTTP_200_OK expected = CourseEnrollment.objects.filter(user=self.user).count() assert response.data['count'] == expected + + +# --------------------------------------------------------------------------- +# ADR 0029 – Standardize Error Responses +# --------------------------------------------------------------------------- + +_REQUIRED_ERROR_FIELDS = ("type", "title", "status", "detail", "instance") + + +@skip_unless_lms +class TestStandardizedErrorHandlerInstalled(APITestCase): + """ADR 0029 – verify the central exception handler is wired in settings.""" + + def test_exception_handler_is_standardized(self): + from django.conf import settings as django_settings + handler = django_settings.REST_FRAMEWORK.get("EXCEPTION_HANDLER", "") + assert "standardized_error_exception_handler" in handler, ( + "ADR 0029: EXCEPTION_HANDLER must point to standardized_error_exception_handler" + ) + + +@skip_unless_lms +class TestEnrollmentViewErrorShape(APITestCase): + """ + ADR 0029 – error response shape regression tests for EnrollmentView. + + Verifies that error responses conform to the standardized JSON envelope + after migrating from inline Response({"message": ...}) returns. + """ + + def setUp(self): + super().setUp() + self.user = UserFactory.create() + self.client.force_authenticate(user=self.user) + + def test_invalid_course_key_via_post_returns_validation_error_shape(self): + """POST with an invalid course key in the body must return 400 with the ADR 0029 envelope.""" + url = reverse("courseenrollments") # EnrollmentListView POST + response = self.client.post( + url, + data={"course_details": {"course_id": "not-a-valid-key"}}, + content_type="application/json", + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + for field in _REQUIRED_ERROR_FIELDS: + assert field in response.data, f"ADR 0029: missing error field '{field}'" + + def test_validation_error_type_uri(self): + """The ``type`` field must be the ADR 0029 validation URI for a 400 response.""" + url = reverse("courseenrollments") + response = self.client.post( + url, + data={"course_details": {"course_id": "not-a-valid-key"}}, + content_type="application/json", + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.data.get("type") == "https://docs.openedx.org/errors/validation" + + def test_instance_field_is_request_path(self): + """The ``instance`` field must equal the request path.""" + url = reverse("courseenrollments") + response = self.client.post( + url, + data={"course_details": {"course_id": "not-a-valid-key"}}, + content_type="application/json", + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.data.get("instance") == url + + def test_unauthorized_user_lookup_returns_404_shape(self): + """A user looking up another user's enrollment gets 404 with ADR 0029 envelope.""" + other = UserFactory.create() + url = reverse( + "courseenrollment", + kwargs={"username": other.username, "course_id": "course-v1:edX+DemoX+Demo_Course"}, + ) + response = self.client.get(url) + assert response.status_code == status.HTTP_404_NOT_FOUND + for field in _REQUIRED_ERROR_FIELDS: + assert field in response.data, f"ADR 0029: missing error field '{field}'" + assert response.data.get("type") == "https://docs.openedx.org/errors/not-found" + + +@skip_unless_lms +class TestEnrollmentViewSetCreateErrorShape(APITestCase): + """ + ADR 0029 – error response shape regression tests for EnrollmentViewSet.create. + """ + + def setUp(self): + super().setUp() + self.user = UserFactory.create() + self.client.force_authenticate(user=self.user) + self.url = reverse("enrollment-list") + + def test_missing_course_id_returns_validation_envelope(self): + """POST without course_details.course_id must return 400 ADR 0029 envelope.""" + response = self.client.post(self.url, data={}, format="json") + assert response.status_code == status.HTTP_400_BAD_REQUEST + for field in _REQUIRED_ERROR_FIELDS: + assert field in response.data, f"ADR 0029: missing error field '{field}'" + assert response.data.get("type") == "https://docs.openedx.org/errors/validation" + + def test_invalid_course_key_returns_validation_envelope(self): + """POST with an invalid course key must return 400 ADR 0029 envelope.""" + response = self.client.post( + self.url, + data={"course_details": {"course_id": "not-a-valid-key"}}, + format="json", + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + for field in _REQUIRED_ERROR_FIELDS: + assert field in response.data, f"ADR 0029: missing error field '{field}'" + + +@skip_unless_lms +class TestEnrollmentViewSetAllowedErrorShape(APITestCase): + """ + ADR 0029 – error response shape for EnrollmentViewSet.allowed (admin action). + """ + + def setUp(self): + super().setUp() + self.admin = UserFactory.create(is_staff=True, is_superuser=True) + self.client.force_authenticate(user=self.admin) + self.url = reverse("enrollment-allowed") + + def test_missing_fields_returns_validation_envelope(self): + """POST without required fields must return 400 ADR 0029 envelope.""" + response = self.client.post(self.url, data={}, format="json") + assert response.status_code == status.HTTP_400_BAD_REQUEST + for field in _REQUIRED_ERROR_FIELDS: + assert field in response.data, f"ADR 0029: missing error field '{field}'" + assert response.data.get("type") == "https://docs.openedx.org/errors/validation" + assert "errors" in response.data, "ADR 0029: validation response must include 'errors'" + assert isinstance(response.data["errors"], dict) + + def test_delete_nonexistent_returns_not_found_envelope(self): + """DELETE for a non-existent enrollment_allowed must return 404 ADR 0029 envelope.""" + response = self.client.delete( + self.url, + data={"email": "nobody@example.com", "course_id": "course-v1:edX+Demo+2099"}, + format="json", + ) + assert response.status_code == status.HTTP_404_NOT_FOUND + for field in _REQUIRED_ERROR_FIELDS: + assert field in response.data, f"ADR 0029: missing error field '{field}'" + assert response.data.get("type") == "https://docs.openedx.org/errors/not-found" diff --git a/openedx/core/djangoapps/enrollments/views.py b/openedx/core/djangoapps/enrollments/views.py index 8f82b957b5b4..f3377ccffeaa 100644 --- a/openedx/core/djangoapps/enrollments/views.py +++ b/openedx/core/djangoapps/enrollments/views.py @@ -24,6 +24,10 @@ from opaque_keys.edx.keys import CourseKey # lint-amnesty, pylint: disable=wrong-import-order from rest_framework import permissions, status, viewsets # lint-amnesty, pylint: disable=wrong-import-order from rest_framework.decorators import action # lint-amnesty, pylint: disable=wrong-import-order +from rest_framework.exceptions import APIException # ADR 0029 # lint-amnesty, pylint: disable=wrong-import-order +from rest_framework.exceptions import NotFound # ADR 0029 # lint-amnesty, pylint: disable=wrong-import-order +from rest_framework.exceptions import PermissionDenied as DRFPermissionDenied # ADR 0029 # lint-amnesty, pylint: disable=wrong-import-order +from rest_framework.exceptions import ValidationError as DRFValidationError # ADR 0029 # lint-amnesty, pylint: disable=wrong-import-order from rest_framework.generics import ListAPIView # lint-amnesty, pylint: disable=wrong-import-order from rest_framework.response import Response # lint-amnesty, pylint: disable=wrong-import-order from rest_framework.throttling import UserRateThrottle # lint-amnesty, pylint: disable=wrong-import-order @@ -59,6 +63,7 @@ from openedx.core.djangoapps.user_api.models import UserRetirementStatus from openedx.core.djangoapps.user_api.preferences.api import update_email_opt_in from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser +from openedx.core.lib.api.exceptions import Conflict # ADR 0029 from openedx.core.lib.api.permissions import ApiKeyHeaderPermission, ApiKeyHeaderPermissionIsAuthenticated from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin from openedx.core.lib.exceptions import CourseNotFoundError @@ -225,29 +230,21 @@ def get(self, request, course_id=None, username=None): ): # Return a 404 instead of a 403 (Unauthorized). If one user is looking up # other users, do not let them deduce the existence of an enrollment. - return Response(status=status.HTTP_404_NOT_FOUND) + raise NotFound() try: course_key = CourseKey.from_string(course_id) except InvalidKeyError: - return Response( - status=status.HTTP_400_BAD_REQUEST, - data={"message": f"No course '{course_id}' found for enrollment"}, - ) + raise DRFValidationError(f"No course '{course_id}' found for enrollment") # noqa: B904 try: enrollment = CourseEnrollment.objects.get(user__username=username, course_id=course_key) except CourseEnrollment.DoesNotExist: return Response(None) except CourseEnrollmentError: - return Response( - status=status.HTTP_400_BAD_REQUEST, - data={ - "message": ( - "An error occurred while retrieving enrollments for user " - "'{username}' in course '{course_id}'" - ).format(username=username, course_id=course_id) - }, + raise DRFValidationError( # noqa: B904 + "An error occurred while retrieving enrollments for user " + f"'{username}' in course '{course_id}'" ) serializer = self.serializer_class(enrollment) @@ -299,13 +296,10 @@ def get(self, request): if course_id: roles_data = [role for role in roles_data if str(role.course_id) == course_id] except Exception: # pylint: disable=broad-except - return Response( - status=status.HTTP_400_BAD_REQUEST, - data={ - "message": ("An error occurred while retrieving roles for user '{username}").format( - username=request.user.username - ) - }, + raise DRFValidationError( # noqa: B904 + "An error occurred while retrieving roles for user '{username}'".format( + username=request.user.username + ) ) serializer = self.serializer_class({ "roles": list(roles_data), @@ -401,17 +395,11 @@ def get(self, request, course_id=None): try: course_key = CourseKey.from_string(course_id) except InvalidKeyError: - return Response( - status=status.HTTP_400_BAD_REQUEST, - data={"message": f"No course found for course ID '{course_id}'"}, - ) + raise DRFValidationError(f"No course found for course ID '{course_id}'") # noqa: B904 try: course_overview = CourseOverview.get_from_id(course_key) except CourseOverview.DoesNotExist: - return Response( - status=status.HTTP_400_BAD_REQUEST, - data={"message": f"No course found for course ID '{course_id}'"}, - ) + raise NotFound(f"No course found for course ID '{course_id}'") # noqa: B904 include_expired = bool(request.GET.get("include_expired", "")) serializer = self.serializer_class(course_overview, include_expired=include_expired) return Response(serializer.data) @@ -476,11 +464,12 @@ def post(self, request): return Response(status=status.HTTP_204_NO_CONTENT) return Response(api.unenroll_user_from_all_courses(username)) except KeyError: - return Response("Username not specified.", status=status.HTTP_404_NOT_FOUND) + raise DRFValidationError("Username not specified.") # noqa: B904 except UserRetirementStatus.DoesNotExist: - return Response("No retirement request status for username.", status=status.HTTP_404_NOT_FOUND) + raise NotFound("No retirement request status for username.") # noqa: B904 except Exception as exc: # pylint: disable=broad-except - return Response(str(exc), status=status.HTTP_500_INTERNAL_SERVER_ERROR) + log.exception("Unexpected error during unenrollment for user") + raise APIException("An unexpected error occurred during unenrollment.") # noqa: B904 # ADR 0028 – consolidated from EnrollmentListView, UnenrollmentView, EnrollmentAllowedView @@ -573,17 +562,12 @@ def create(self, request): course_id = request.data.get("course_details", {}).get("course_id") if not course_id: - return Response( - status=status.HTTP_400_BAD_REQUEST, - data={"message": "Course ID must be specified to create a new enrollment."}, - ) + raise DRFValidationError("Course ID must be specified to create a new enrollment.") try: course_id = CourseKey.from_string(course_id) except InvalidKeyError: - return Response( - status=status.HTTP_400_BAD_REQUEST, data={"message": f"No course '{course_id}' found for enrollment"} - ) + raise DRFValidationError(f"No course '{course_id}' found for enrollment") # noqa: B904 mode = request.data.get("mode") @@ -595,20 +579,17 @@ def create(self, request): and not has_api_key_permissions and not GlobalStaff().has_user(request.user) ): - return Response(status=status.HTTP_404_NOT_FOUND) + raise NotFound() if not username: email = request.data.get("email") if email: if not has_api_key_permissions and not GlobalStaff().has_user(request.user): - return Response(status=status.HTTP_404_NOT_FOUND) + raise NotFound() try: username = User.objects.get(email=email).username except ObjectDoesNotExist: - return Response( - status=status.HTTP_406_NOT_ACCEPTABLE, - data={"message": f"The user with the email address {email} does not exist."}, - ) + raise NotFound(f"The user with the email address {email} does not exist.") else: username = request.user.username @@ -617,21 +598,14 @@ def create(self, request): and not has_api_key_permissions and not GlobalStaff().has_user(request.user) ): - return Response( - status=status.HTTP_403_FORBIDDEN, - data={ - "message": "User does not have permission to create enrollment with mode [{mode}].".format( - mode=mode - ) - }, + raise DRFPermissionDenied( + "User does not have permission to create enrollment with mode [{mode}].".format(mode=mode) ) try: user = User.objects.get(username=username) except ObjectDoesNotExist: - return Response( - status=status.HTTP_406_NOT_ACCEPTABLE, data={"message": f"The user {username} does not exist."} - ) + raise NotFound(f"The user {username} does not exist.") embargo_response = embargo_api.get_embargo_response(request, course_id, user) @@ -641,9 +615,8 @@ def create(self, request): try: is_active = request.data.get("is_active") if is_active is not None and not isinstance(is_active, bool): - return Response( - status=status.HTTP_400_BAD_REQUEST, - data={"message": ("'{value}' is an invalid enrollment activation status.").format(value=is_active)}, + raise DRFValidationError( + ("'{value}' is an invalid enrollment activation status.").format(value=is_active) ) explicit_linked_enterprise = request.data.get("linked_enterprise_customer") @@ -670,11 +643,8 @@ def create(self, request): enrollment_attributes = request.data.get("enrollment_attributes") force_enrollment = request.data.get("force_enrollment") if force_enrollment is not None and not isinstance(force_enrollment, bool): - return Response( - status=status.HTTP_400_BAD_REQUEST, - data={ - "message": ("'{value}' is an invalid force enrollment status.").format(value=force_enrollment) - }, + raise DRFValidationError( + ("'{value}' is an invalid force enrollment status.").format(value=force_enrollment) ) force_enrollment = force_enrollment and GlobalStaff().has_user(request.user) enrollment = api.get_enrollment(username, str(course_id)) @@ -690,14 +660,14 @@ def create(self, request): enrollment["mode"], mode ) log.warning(msg) - return Response(status=status.HTTP_400_BAD_REQUEST, data={"message": msg}) + raise DRFValidationError(msg) if missing_attrs: msg = "Missing enrollment attributes: requested mode={} required attributes={}".format( mode, REQUIRED_ATTRIBUTES.get(mode) ) log.warning(msg) - return Response(status=status.HTTP_400_BAD_REQUEST, data={"message": msg}) + raise DRFValidationError(msg) response = api.update_enrollment( username, @@ -735,35 +705,21 @@ def create(self, request): return Response(response) except InvalidEnrollmentAttribute as error: - return Response( - status=status.HTTP_400_BAD_REQUEST, - data={ - "message": str(error), - "localizedMessage": str(error), - } - ) + exc = DRFValidationError(str(error)) + exc.user_message = str(error) + raise exc from error except EnrollmentNotAllowed as error: - return Response( - status=status.HTTP_403_FORBIDDEN, - data={ - "message": str(error), - "localizedMessage": str(error), - } - ) - except CourseModeNotFoundError as error: - return Response( - status=status.HTTP_400_BAD_REQUEST, - data={ - "message": ( - "The [{mode}] course mode is expired or otherwise unavailable for course run [{course_id}]." - ).format(mode=mode, course_id=course_id), - "course_details": error.data, - }, + exc = DRFPermissionDenied(str(error)) + exc.user_message = str(error) + raise exc from error + except CourseModeNotFoundError: + raise DRFValidationError( # noqa: B904 + "The [{mode}] course mode is expired or otherwise unavailable for course run [{course_id}].".format( + mode=mode, course_id=course_id + ) ) except CourseNotFoundError: - return Response( - status=status.HTTP_400_BAD_REQUEST, data={"message": f"No course '{course_id}' found for enrollment"} - ) + raise DRFValidationError(f"No course '{course_id}' found for enrollment") # noqa: B904 except CourseEnrollmentExistsError as error: log.warning("An enrollment already exists for user [%s] in course run [%s].", username, course_id) return Response(data=error.enrollment) @@ -773,21 +729,13 @@ def create(self, request): username, course_id, ) - return Response( - status=status.HTTP_400_BAD_REQUEST, - data={ - "message": ( - "An error occurred while creating the new course enrollment for user " - "'{username}' in course '{course_id}'" - ).format(username=username, course_id=course_id) - }, + raise DRFValidationError( # noqa: B904 + "An error occurred while creating the new course enrollment for user " + f"'{username}' in course '{course_id}'" ) except CourseUserGroup.DoesNotExist: log.exception("Missing cohort [%s] in course run [%s]", cohort_name, course_id) - return Response( - status=status.HTTP_400_BAD_REQUEST, - data={"message": "An error occured while adding to cohort [%s]" % cohort_name}, - ) + raise DRFValidationError("An error occured while adding to cohort [%s]" % cohort_name) # noqa: B904 finally: if has_api_key_permissions: try: @@ -831,11 +779,12 @@ def unenroll(self, request): return Response(status=status.HTTP_204_NO_CONTENT) return Response(api.unenroll_user_from_all_courses(username)) except KeyError: - return Response("Username not specified.", status=status.HTTP_404_NOT_FOUND) + raise DRFValidationError("Username not specified.") # noqa: B904 except UserRetirementStatus.DoesNotExist: - return Response("No retirement request status for username.", status=status.HTTP_404_NOT_FOUND) + raise NotFound("No retirement request status for username.") # noqa: B904 except Exception as exc: # pylint: disable=broad-except - return Response(str(exc), status=status.HTTP_500_INTERNAL_SERVER_ERROR) + log.exception("Unexpected error during unenrollment for user") + raise APIException("An unexpected error occurred during unenrollment.") # noqa: B904 @action( detail=False, @@ -861,20 +810,15 @@ def allowed(self, request): serializer = self.get_serializer(data=request.data) if not serializer.is_valid(): - return Response(status=status.HTTP_400_BAD_REQUEST, data=serializer.errors) + raise DRFValidationError(serializer.errors) if request.method == "POST": try: enrollment_allowed = serializer.save() except IntegrityError: - return Response( - status=status.HTTP_409_CONFLICT, - data={ - "message": ( - f"An enrollment allowed with email {serializer.validated_data.get('email')} " - f"and course {serializer.validated_data.get('course_id')} already exists." - ) - }, + raise Conflict( # noqa: B904 + f"An enrollment allowed with email {serializer.validated_data.get('email')} " + f"and course {serializer.validated_data.get('course_id')} already exists." ) return Response( status=status.HTTP_201_CREATED, @@ -888,9 +832,8 @@ def allowed(self, request): CourseEnrollmentAllowed.objects.get(email=email, course_id=course_id).delete() return Response(status=status.HTTP_204_NO_CONTENT) except ObjectDoesNotExist: - return Response( - status=status.HTTP_404_NOT_FOUND, - data={"message": f"An enrollment allowed with email {email} and course {course_id} doesn't exists."}, + raise NotFound( # noqa: B904 + f"An enrollment allowed with email {email} and course {course_id} doesn't exist." ) @@ -1131,17 +1074,12 @@ def post(self, request): course_id = request.data.get("course_details", {}).get("course_id") if not course_id: - return Response( - status=status.HTTP_400_BAD_REQUEST, - data={"message": "Course ID must be specified to create a new enrollment."}, - ) + raise DRFValidationError("Course ID must be specified to create a new enrollment.") try: course_id = CourseKey.from_string(course_id) except InvalidKeyError: - return Response( - status=status.HTTP_400_BAD_REQUEST, data={"message": f"No course '{course_id}' found for enrollment"} - ) + raise DRFValidationError(f"No course '{course_id}' found for enrollment") # noqa: B904 mode = request.data.get("mode") @@ -1156,7 +1094,7 @@ def post(self, request): ): # Return a 404 instead of a 403 (Unauthorized). If one user is looking up # other users, do not let them deduce the existence of an enrollment. - return Response(status=status.HTTP_404_NOT_FOUND) + raise NotFound() # A provided user has priority over a provided email. # Fallback on request user if neither is provided. @@ -1165,14 +1103,11 @@ def post(self, request): if email: # Only server-to-server or staff users can use the email for the request. if not has_api_key_permissions and not GlobalStaff().has_user(request.user): - return Response(status=status.HTTP_404_NOT_FOUND) + raise NotFound() try: username = User.objects.get(email=email).username except ObjectDoesNotExist: - return Response( - status=status.HTTP_406_NOT_ACCEPTABLE, - data={"message": f"The user with the email address {email} does not exist."}, - ) + raise NotFound(f"The user with the email address {email} does not exist.") else: username = request.user.username @@ -1181,22 +1116,15 @@ def post(self, request): and not has_api_key_permissions and not GlobalStaff().has_user(request.user) ): - return Response( - status=status.HTTP_403_FORBIDDEN, - data={ - "message": "User does not have permission to create enrollment with mode [{mode}].".format( - mode=mode - ) - }, + raise DRFPermissionDenied( + "User does not have permission to create enrollment with mode [{mode}].".format(mode=mode) ) try: # Lookup the user, instead of using request.user, since request.user may not match the username POSTed. user = User.objects.get(username=username) except ObjectDoesNotExist: - return Response( - status=status.HTTP_406_NOT_ACCEPTABLE, data={"message": f"The user {username} does not exist."} - ) + raise NotFound(f"The user {username} does not exist.") embargo_response = embargo_api.get_embargo_response(request, course_id, user) @@ -1207,9 +1135,8 @@ def post(self, request): is_active = request.data.get("is_active") # Check if the requested activation status is None or a Boolean if is_active is not None and not isinstance(is_active, bool): - return Response( - status=status.HTTP_400_BAD_REQUEST, - data={"message": ("'{value}' is an invalid enrollment activation status.").format(value=is_active)}, + raise DRFValidationError( + ("'{value}' is an invalid enrollment activation status.").format(value=is_active) ) explicit_linked_enterprise = request.data.get("linked_enterprise_customer") @@ -1237,11 +1164,8 @@ def post(self, request): force_enrollment = request.data.get("force_enrollment") # Check if the force enrollment status is None or a Boolean if force_enrollment is not None and not isinstance(force_enrollment, bool): - return Response( - status=status.HTTP_400_BAD_REQUEST, - data={ - "message": ("'{value}' is an invalid force enrollment status.").format(value=force_enrollment) - }, + raise DRFValidationError( + ("'{value}' is an invalid force enrollment status.").format(value=force_enrollment) ) # Only a staff user role can enroll a user forcefully force_enrollment = force_enrollment and GlobalStaff().has_user(request.user) @@ -1261,14 +1185,14 @@ def post(self, request): enrollment["mode"], mode ) log.warning(msg) - return Response(status=status.HTTP_400_BAD_REQUEST, data={"message": msg}) + raise DRFValidationError(msg) if missing_attrs: msg = "Missing enrollment attributes: requested mode={} required attributes={}".format( mode, REQUIRED_ATTRIBUTES.get(mode) ) log.warning(msg) - return Response(status=status.HTTP_400_BAD_REQUEST, data={"message": msg}) + raise DRFValidationError(msg) response = api.update_enrollment( username, @@ -1310,35 +1234,21 @@ def post(self, request): return Response(response) except InvalidEnrollmentAttribute as error: - return Response( - status=status.HTTP_400_BAD_REQUEST, - data={ - "message": str(error), - "localizedMessage": str(error), - } - ) + exc = DRFValidationError(str(error)) + exc.user_message = str(error) + raise exc from error except EnrollmentNotAllowed as error: - return Response( - status=status.HTTP_403_FORBIDDEN, - data={ - "message": str(error), - "localizedMessage": str(error), - } - ) - except CourseModeNotFoundError as error: - return Response( - status=status.HTTP_400_BAD_REQUEST, - data={ - "message": ( - "The [{mode}] course mode is expired or otherwise unavailable for course run [{course_id}]." - ).format(mode=mode, course_id=course_id), - "course_details": error.data, - }, + exc = DRFPermissionDenied(str(error)) + exc.user_message = str(error) + raise exc from error + except CourseModeNotFoundError: + raise DRFValidationError( # noqa: B904 + "The [{mode}] course mode is expired or otherwise unavailable for course run [{course_id}].".format( + mode=mode, course_id=course_id + ) ) except CourseNotFoundError: - return Response( - status=status.HTTP_400_BAD_REQUEST, data={"message": f"No course '{course_id}' found for enrollment"} - ) + raise DRFValidationError(f"No course '{course_id}' found for enrollment") # noqa: B904 except CourseEnrollmentExistsError as error: log.warning("An enrollment already exists for user [%s] in course run [%s].", username, course_id) return Response(data=error.enrollment) @@ -1348,22 +1258,14 @@ def post(self, request): username, course_id, ) - return Response( - status=status.HTTP_400_BAD_REQUEST, - data={ - "message": ( - "An error occurred while creating the new course enrollment for user " - "'{username}' in course '{course_id}'" - ).format(username=username, course_id=course_id) - }, + raise DRFValidationError( # noqa: B904 + "An error occurred while creating the new course enrollment for user " + f"'{username}' in course '{course_id}'" ) except CourseUserGroup.DoesNotExist: log.exception("Missing cohort [%s] in course run [%s]", cohort_name, course_id) - return Response( - status=status.HTTP_400_BAD_REQUEST, - data={"message": "An error occured while adding to cohort [%s]" % cohort_name}, - ) + raise DRFValidationError("An error occured while adding to cohort [%s]" % cohort_name) # noqa: B904 finally: # Assumes that the ecommerce service uses an API key to authenticate. if has_api_key_permissions: @@ -1580,19 +1482,14 @@ def post(self, request): """ serializer = self.serializer_class(data=request.data) if not serializer.is_valid(): - return Response(status=status.HTTP_400_BAD_REQUEST, data=serializer.errors) + raise DRFValidationError(serializer.errors) try: enrollment_allowed = serializer.save() except IntegrityError: - return Response( - status=status.HTTP_409_CONFLICT, - data={ - "message": ( - f"An enrollment allowed with email {serializer.validated_data.get('email')} " - f"and course {serializer.validated_data.get('course_id')} already exists." - ) - }, + raise Conflict( # noqa: B904 + f"An enrollment allowed with email {serializer.validated_data.get('email')} " + f"and course {serializer.validated_data.get('course_id')} already exists." ) return Response(status=status.HTTP_201_CREATED, data=self.serializer_class(enrollment_allowed).data) @@ -1629,7 +1526,7 @@ def delete(self, request): """ serializer = self.serializer_class(data=request.data) if not serializer.is_valid(): - return Response(status=status.HTTP_400_BAD_REQUEST, data=serializer.errors) + raise DRFValidationError(serializer.errors) email = serializer.validated_data.get("email") course_id = serializer.validated_data.get("course_id") @@ -1638,7 +1535,6 @@ def delete(self, request): CourseEnrollmentAllowed.objects.get(email=email, course_id=course_id).delete() return Response(status=status.HTTP_204_NO_CONTENT) except ObjectDoesNotExist: - return Response( - status=status.HTTP_404_NOT_FOUND, - data={"message": f"An enrollment allowed with email {email} and course {course_id} doesn't exists."}, + raise NotFound( # noqa: B904 + f"An enrollment allowed with email {email} and course {course_id} doesn't exist." ) diff --git a/openedx/core/lib/api/exceptions.py b/openedx/core/lib/api/exceptions.py new file mode 100644 index 000000000000..11dc19d2a18e --- /dev/null +++ b/openedx/core/lib/api/exceptions.py @@ -0,0 +1,151 @@ +""" +ADR 0029 – Standardized error-response exception handler and helpers. + +Installs a platform-level DRF ``EXCEPTION_HANDLER`` that converts every +API exception into a single, consistent JSON envelope:: + + { + "type": "https://docs.openedx.org/errors/", + "title": "Validation Error", + "status": 400, + "detail": "The request body failed validation.", + "instance": "/api/enrollment/v1/enrollment/", + "user_message": "...", # optional – present only when set on exc + "errors": {...} # optional – present only for ValidationError + } + +The handler chains through the existing ``ignored_error_exception_handler`` +so that error logging / monitoring added by that handler is preserved. +""" + +from rest_framework.exceptions import APIException, ValidationError +from rest_framework.response import Response + + +# --------------------------------------------------------------------------- +# Public exception classes +# --------------------------------------------------------------------------- + +class Conflict(APIException): + """HTTP 409 Conflict — ADR 0029.""" + + status_code = 409 + default_detail = "A conflict occurred." + default_code = "conflict" + + +# --------------------------------------------------------------------------- +# Central handler +# --------------------------------------------------------------------------- + +def standardized_error_exception_handler(exc, context): + """ + ADR 0029 – platform-level DRF exception handler. + + Chains through ``ignored_error_exception_handler`` so that its error + logging / monitoring is preserved, then reformats the response to the + ADR 0029 envelope shape. + + Returns a generic 500 body for unhandled exceptions so that stack + traces are never leaked to callers. + """ + # Chain through the existing handler to preserve monitoring side-effects. + from openedx.core.lib.request_utils import ignored_error_exception_handler + response = ignored_error_exception_handler(exc, context) + + if response is None: + # Unhandled exception (e.g. IntegrityError, unexpected 500). + # Always return a generic body — never expose stack traces. + return Response( + { + "type": "https://docs.openedx.org/errors/internal", + "title": "Internal Server Error", + "status": 500, + "detail": "An unexpected error occurred. Please try again later.", + }, + status=500, + ) + + request = context.get("request") + body = { + "type": f"https://docs.openedx.org/errors/{_error_type(exc)}", + "title": _error_title(exc), + "status": response.status_code, + "detail": _flatten_detail(response.data), + } + if request: + body["instance"] = request.path + if hasattr(exc, "user_message") and exc.user_message: + body["user_message"] = exc.user_message + if isinstance(exc, ValidationError) and hasattr(exc, "detail"): + body["errors"] = _normalize_validation_errors(exc.detail) + + response.data = body + response["Content-Type"] = "application/json" + return response + + +# --------------------------------------------------------------------------- +# Private helpers +# --------------------------------------------------------------------------- + +def _error_type(exc): + """Map a DRF exception to an ADR 0029 error-type slug.""" + from rest_framework.exceptions import ( + AuthenticationFailed, NotAuthenticated, NotFound, PermissionDenied, + Throttled, ValidationError, + ) + if isinstance(exc, (NotAuthenticated, AuthenticationFailed)): + return "authn" + if isinstance(exc, PermissionDenied): + return "authz" + if isinstance(exc, NotFound): + return "not-found" + if isinstance(exc, ValidationError): + return "validation" + if isinstance(exc, Throttled): + return "rate-limited" + if isinstance(exc, Conflict): + return "conflict" + return "internal" + + +def _error_title(exc): + """Return a short, developer-facing title for the exception class.""" + from rest_framework.exceptions import ( + AuthenticationFailed, NotAuthenticated, NotFound, PermissionDenied, + Throttled, ValidationError, + ) + _TITLES = { + NotAuthenticated: "Authentication Required", + AuthenticationFailed: "Authentication Failed", + PermissionDenied: "Permission Denied", + NotFound: "Not Found", + ValidationError: "Validation Error", + Throttled: "Too Many Requests", + Conflict: "Conflict", + } + return _TITLES.get(type(exc), "Internal Server Error") + + +def _flatten_detail(data): + """Extract a single string from DRF's response.data for the ``detail`` field.""" + if isinstance(data, str): + return data + if isinstance(data, dict) and "detail" in data: + return str(data["detail"]) + if isinstance(data, list) and data: + return str(data[0]) + return str(data) + + +def _normalize_validation_errors(detail): + """Normalize DRF ValidationError detail into ``{field: [msg, ...]}`` form.""" + if isinstance(detail, dict): + return { + field: [str(e) for e in (errs if isinstance(errs, list) else [errs])] + for field, errs in detail.items() + } + if isinstance(detail, list): + return {"non_field_errors": [str(e) for e in detail]} + return {"non_field_errors": [str(detail)]} diff --git a/openedx/envs/common.py b/openedx/envs/common.py index 5d7c105025ed..1247116e397c 100644 --- a/openedx/envs/common.py +++ b/openedx/envs/common.py @@ -820,7 +820,7 @@ def add_optional_apps(optional_apps, installed_apps): 'DEFAULT_RENDERER_CLASSES': ( 'rest_framework.renderers.JSONRenderer', ), - 'EXCEPTION_HANDLER': 'openedx.core.lib.request_utils.ignored_error_exception_handler', + 'EXCEPTION_HANDLER': 'openedx.core.lib.api.exceptions.standardized_error_exception_handler', # ADR 0029 'PAGE_SIZE': 10, 'URL_FORMAT_OVERRIDE': None, 'DEFAULT_THROTTLE_RATES': {