feat: raise descriptive error when proctoring provider cannot remove an attempt

CHANGELOG.rst

@@ -13,6 +13,21 @@ Change Log
 
 Unreleased
 ~~~~~~~~~~
+[6.1.0] - 2026-07-28
+
+* Make proctored exam attempt removal fail loudly when the proctoring provider errors,
+  instead of a silent local-only reset or an unhandled 500:
+
+  * Add an explicit request timeout to the REST proctoring backend (default 30s,
+    overridable per backend via a ``timeout`` key in ``PROCTORING_BACKENDS``).
+  * Raise a typed ``BackendProviderCannotRemoveAttempt`` (HTTP 502) both when the
+    provider is unreachable/times out and when it responds with an HTTP error status,
+    returning a clear message (including the provider's HTTP status) and logging the raw
+    provider response for debugging. Callers can then return a descriptive error to the
+    instructor instead of a silent local-only reset or an unhandled 500. Raising on a
+    provider HTTP error is on by default and overridable per backend via a
+    ``raise_on_remove_error`` key in ``PROCTORING_BACKENDS``.
+
 [5.2.1] - 2025-12-05
 * Remove all references to Proctortrack
 

edx_proctoring/__init__.py

@@ -3,4 +3,4 @@
 """
 
 # Be sure to update the version number in edx_proctoring/package.json
-__version__ = '6.0.0'
+__version__ = '6.1.0'

edx_proctoring/apps.py

@@ -30,13 +30,15 @@
     'organization',
     'passing_statuses',
     'ping_interval',
+    'raise_on_remove_error',
     'secret_key',
     'secret_key_id',
     'send_email',
     'software_download_url',
     'supports_onboarding',
     'tech_support_email',
     'tech_support_phone',
+    'timeout',
     'token_expiration_time',
     'verbose_name',
     'video_review_aes_key',

edx_proctoring/backends/rest.py

@@ -19,6 +19,7 @@
 from edx_proctoring.backends.backend import ProctoringBackendProvider
 from edx_proctoring.exceptions import (
     BackendProviderCannotRegisterAttempt,
+    BackendProviderCannotRemoveAttempt,
     BackendProviderCannotRetireUser,
     BackendProviderOnboardingException,
     BackendProviderOnboardingProfilesException,
@@ -41,6 +42,15 @@ class BaseRestProctoringProvider(ProctoringBackendProvider):
     has_dashboard = True
     supports_onboarding = True
     passing_statuses = (SoftwareSecureReviewStatus.clean,)
+    # Timeout (in seconds) applied to every outbound request to the provider so
+    # that a slow or unavailable provider cannot hang the request indefinitely.
+    # Operators can override this per backend via a ``timeout`` key in the
+    # backend's ``PROCTORING_BACKENDS`` configuration.
+    timeout = 30
+    # Whether a provider HTTP error while removing an attempt should raise a descriptive
+    # error (so it is surfaced to the instructor) rather than being logged and ignored.
+    # Overridable per backend via a ``raise_on_remove_error`` key in ``PROCTORING_BACKENDS``.
+    raise_on_remove_error = True
 
     @property
     def exam_attempt_url(self):
@@ -227,12 +237,18 @@ def stop_exam_attempt(self, exam, attempt):
 
     def remove_exam_attempt(self, exam, attempt):
         """
-        Removes the exam attempt on the backend provider's server
+        Removes the exam attempt on the backend provider's server.
+
+        Raises BackendProviderCannotRemoveAttempt if the provider responds with an
+        HTTP error status (unless ``raise_on_remove_error`` is disabled for this backend),
+        so the caller can surface a descriptive error instead of silently leaving the
+        attempt on the provider's side.
         """
         response = self._make_attempt_request(
             exam,
             attempt,
-            method='DELETE')
+            method='DELETE',
+            raise_on_error=self.raise_on_remove_error)
         return response.get('status', None) == 'deleted'
 
     def mark_erroneous_exam_attempt(self, exam, attempt):
@@ -369,9 +385,14 @@ def _get_language_headers(self):
             lang_header = f'{current_lang};{default_lang}'
         return {'Accept-Language': lang_header}
 
-    def _make_attempt_request(self, exam, attempt, method='POST', status=None, **payload):
+    def _make_attempt_request(self, exam, attempt, method='POST', status=None, raise_on_error=False, **payload):
         """
-        Calls backend attempt API
+        Calls backend attempt API.
+
+        When ``raise_on_error`` is True, a provider HTTP error response (status >= 400)
+        raises BackendProviderCannotRemoveAttempt, surfacing the provider's own message
+        when it supplies one and a generic message otherwise. This is distinct from the
+        provider being unreachable/timing out, where ``session.request`` itself raises.
         """
         if not attempt:
             return {}
@@ -384,10 +405,23 @@ def _make_attempt_request(self, exam, attempt, method='POST', status=None, **pay
         if method == 'GET':
             headers.update(self._get_language_headers())
         log.debug('Making %r attempt request at %r', method, url)
-        response = self.session.request(method, url, json=payload, headers=headers)
+        response = self.session.request(method, url, json=payload, headers=headers, timeout=self.timeout)
         try:
             data = response.json()
         except ValueError:
             log.exception("Decoding attempt %r -> %r", attempt, response.content)
             data = {}
+        if raise_on_error and not response.ok:
+            # Log the raw provider response (status + body) for support/debugging; the
+            # provider's error schema is not standardised, so we do not try to parse a
+            # message out of it and instead surface a clean, provider-status-aware error.
+            log.error(
+                'Proctoring provider returned HTTP %s while removing attempt %r: %r',
+                response.status_code, attempt, response.content,
+            )
+            raise BackendProviderCannotRemoveAttempt(
+                'The proctoring provider could not remove this attempt '
+                f'(provider returned HTTP {response.status_code}). '
+                'Please try again or contact support.'
+            )
         return data

edx_proctoring/backends/tests/test_rest.py

@@ -12,9 +12,11 @@
 from django.test import TestCase, override_settings
 from django.utils import translation
 
+from edx_proctoring.apps import BACKEND_CONFIGURATION_ALLOW_LIST
 from edx_proctoring.backends.rest import BaseRestProctoringProvider
 from edx_proctoring.exceptions import (
     BackendProviderCannotRegisterAttempt,
+    BackendProviderCannotRemoveAttempt,
     BackendProviderCannotRetireUser,
     BackendProviderOnboardingException,
     BackendProviderOnboardingProfilesException,
@@ -277,6 +279,104 @@ def test_remove_attempt_no_attempt(self):
         status = self.provider.remove_exam_attempt(self.backend_exam['external_id'], None)
         self.assertFalse(status)
 
+    def test_make_attempt_request_passes_timeout(self):
+        """
+        Every outbound attempt request must include an explicit timeout so that a
+        slow/unavailable provider cannot hang the request indefinitely.
+        """
+        attempt_id = 2
+        with patch.object(self.provider.session, 'request') as request_mock:
+            request_mock.return_value.json.return_value = {'status': 'deleted'}
+            self.provider.remove_exam_attempt(self.backend_exam['external_id'], attempt_id)
+        _, kwargs = request_mock.call_args
+        self.assertEqual(kwargs['timeout'], self.provider.timeout)
+
+    def test_default_timeout(self):
+        """The backend applies a sane default request timeout when none is configured."""
+        self.assertEqual(self.provider.timeout, 30)
+
+    def test_timeout_is_configurable(self):
+        """
+        Operators can override the request timeout per backend. The value arrives as a
+        constructor kwarg sourced from the backend's PROCTORING_BACKENDS configuration,
+        so the key must also be present in the backend configuration allow list.
+        """
+        provider = BaseRestProctoringProvider('client_id', 'client_secret', timeout=60)
+        self.assertEqual(provider.timeout, 60)
+        self.assertIn('timeout', BACKEND_CONFIGURATION_ALLOW_LIST)
+
+    def test_remove_attempt_provider_unavailable(self):
+        """
+        A provider connection error must propagate out of the backend rather than
+        being swallowed, so the caller can surface a descriptive error.
+        """
+        attempt_id = 2
+        with patch.object(self.provider.session, 'request', side_effect=ConnectionError('boom')):
+            with self.assertRaises(ConnectionError):
+                self.provider.remove_exam_attempt(self.backend_exam['external_id'], attempt_id)
+
+    @responses.activate
+    def test_remove_attempt_provider_error_raises_with_status(self):
+        """
+        A provider HTTP error response (>= 400) raises BackendProviderCannotRemoveAttempt
+        with a clean, provider-status-aware message and an instructor-facing 502.
+        """
+        attempt_id = 2
+        responses.add(
+            responses.DELETE,
+            url=self.provider.exam_attempt_url.format(exam_id=self.backend_exam['external_id'], attempt_id=attempt_id),
+            json={'detail': 'Attempt is locked'},
+            status=400,
+        )
+        with self.assertRaises(BackendProviderCannotRemoveAttempt) as ctx:
+            self.provider.remove_exam_attempt(self.backend_exam['external_id'], attempt_id)
+        self.assertIn('HTTP 400', str(ctx.exception))
+        self.assertIn('could not remove this attempt', str(ctx.exception))
+        self.assertEqual(ctx.exception.http_status, 502)
+
+    @responses.activate
+    def test_remove_attempt_provider_error_no_body(self):
+        """
+        A provider HTTP error with no response body still raises with the clean
+        provider-status-aware message.
+        """
+        attempt_id = 2
+        responses.add(
+            responses.DELETE,
+            url=self.provider.exam_attempt_url.format(exam_id=self.backend_exam['external_id'], attempt_id=attempt_id),
+            status=502,
+        )
+        with self.assertRaises(BackendProviderCannotRemoveAttempt) as ctx:
+            self.provider.remove_exam_attempt(self.backend_exam['external_id'], attempt_id)
+        self.assertIn('HTTP 502', str(ctx.exception))
+        self.assertEqual(ctx.exception.http_status, 502)
+
+    def test_raise_on_remove_error_default_and_configurable(self):
+        """
+        The raise-on-remove-error behaviour defaults to True and is overridable per
+        backend via PROCTORING_BACKENDS, so the key must be in the allow list.
+        """
+        self.assertTrue(self.provider.raise_on_remove_error)
+        self.assertIn('raise_on_remove_error', BACKEND_CONFIGURATION_ALLOW_LIST)
+        provider = BaseRestProctoringProvider('client_id', 'client_secret', raise_on_remove_error=False)
+        self.assertFalse(provider.raise_on_remove_error)
+
+    @responses.activate
+    def test_remove_attempt_provider_error_not_raised_when_disabled(self):
+        """
+        When raise_on_remove_error is disabled for a backend, a provider HTTP error is
+        not raised; removal returns False (the pre-existing log-and-continue behaviour).
+        """
+        self.provider.raise_on_remove_error = False
+        attempt_id = 2
+        responses.add(
+            responses.DELETE,
+            url=self.provider.exam_attempt_url.format(exam_id=self.backend_exam['external_id'], attempt_id=attempt_id),
+            status=502,
+        )
+        status = self.provider.remove_exam_attempt(self.backend_exam['external_id'], attempt_id)
+        self.assertFalse(status)
+
     def test_on_review_callback(self):
         """
         on_review_callback should just return the payload

edx_proctoring/exceptions.py

@@ -95,6 +95,15 @@ class BackendProviderSentNoAttemptID(BackendProviderCannotRegisterAttempt):
     """
 
 
+class BackendProviderCannotRemoveAttempt(ProctoredBaseException):
+    """
+    Raised when a back-end provider cannot remove an attempt, e.g. because the
+    provider is slow, unreachable, or returned an error while trying to delete
+    the attempt on their side.
+    """
+    http_status = status.HTTP_502_BAD_GATEWAY
+
+
 class BackendProviderOnboardingException(ProctoredBaseException):
     """
     Raised when a back-end provider cannot register an attempt

edx_proctoring/handlers.py

@@ -7,6 +7,7 @@
 
 from edx_proctoring import api, constants, models
 from edx_proctoring.backends import get_backend_provider
+from edx_proctoring.exceptions import BackendProviderCannotRemoveAttempt
 from edx_proctoring.runtime import get_runtime_service
 from edx_proctoring.statuses import ProctoredExamStudentAttemptStatus, SoftwareSecureReviewStatus
 from edx_proctoring.utils import emit_event, locate_attempt_by_attempt_code
@@ -138,7 +139,40 @@ def on_attempt_changed(sender, instance, signal, **kwargs):
         if instance.proctored_exam.is_proctored:
             backend = get_backend_provider(name=instance.proctored_exam.backend)
             if backend:
-                result = backend.remove_exam_attempt(instance.proctored_exam.external_id, instance.external_id)
+                log_context = {
+                    'attempt_id': instance.id,
+                    'external_id': instance.external_id,
+                    'exam_id': instance.proctored_exam.id,
+                    'user_id': instance.user_id,
+                    'backend': instance.proctored_exam.backend,
+                }
+                log_message = (
+                    'Failed to remove attempt_id=%(attempt_id)s (external_id=%(external_id)s) for '
+                    'exam_id=%(exam_id)s user_id=%(user_id)s from backend=%(backend)s due to a '
+                    'provider error.'
+                )
+                try:
+                    result = backend.remove_exam_attempt(
+                        instance.proctored_exam.external_id, instance.external_id
+                    )
+                except BackendProviderCannotRemoveAttempt:
+                    # The provider responded with an HTTP error and the backend already built a
+                    # descriptive (possibly provider-supplied) message. Log and re-raise it as-is
+                    # so that message reaches the instructor-facing MFE unchanged.
+                    log.exception(log_message, log_context)
+                    raise
+                except Exception as exc:  # pylint: disable=broad-exception-caught
+                    # The provider is slow/unavailable (connection error or timeout). This must not
+                    # surface as an unhandled 500 or silently leave the attempt half-removed. Log
+                    # with enough context for monitoring/Sentry and re-raise a typed exception so the
+                    # instructor-facing MFE receives a descriptive ``detail`` message.
+                    log.exception(log_message, log_context)
+                    raise BackendProviderCannotRemoveAttempt(
+                        # Translators: shown to an instructor when an exam attempt could not be reset
+                        # because the external proctoring provider is temporarily unavailable.
+                        'The proctoring provider is temporarily unavailable, so this attempt could not '
+                        'be fully reset. Please try again in a few minutes.'
+                    ) from exc
                 if not result:
                     log.error(
                         'Failed to remove attempt_id=%s from backend=%s',

edx_proctoring/tests/test_handlers.py

@@ -6,9 +6,11 @@
 import ddt
 from httmock import HTTMock
 
+from django.db import transaction
 from django.db.models.signals import pre_delete, pre_save
 
 from edx_proctoring.api import update_attempt_status
+from edx_proctoring.exceptions import BackendProviderCannotRemoveAttempt
 from edx_proctoring.models import ProctoredExam, ProctoredExamStudentAttempt
 from edx_proctoring.statuses import ProctoredExamStudentAttemptStatus
 from edx_proctoring.tests.test_services import MockInstructorService
@@ -47,6 +49,54 @@ def test_backend_fails_to_delete_attempt(self, logger_mock):
             log_format_string = 'Failed to remove attempt_id=%s from backend=%s'
             logger_mock.assert_any_call(log_format_string, 1, self.backend_name)
 
+    @patch('logging.Logger.exception')
+    @patch('edx_proctoring.handlers.get_backend_provider')
+    def test_provider_unavailable_raises_and_keeps_attempt(self, get_backend_mock, logger_mock):
+        """
+        If the provider raises (slow/unavailable) while removing the attempt, a typed
+        BackendProviderCannotRemoveAttempt must be raised, the failure logged with
+        context, and the local attempt must NOT be deleted.
+        """
+        attempt_id = self.attempt.id
+        backend = get_backend_mock.return_value
+        backend.remove_exam_attempt.side_effect = ConnectionError('provider down')
+
+        # ``delete()`` opens an inner atomic(savepoint=False); when the pre_delete
+        # handler raises, that marks the connection as needing rollback. Wrapping the
+        # call in an explicit atomic block creates a savepoint that absorbs the
+        # rollback, so the connection is usable again for the assertion below.
+        with self.assertRaises(BackendProviderCannotRemoveAttempt) as context:
+            with transaction.atomic():
+                self.attempt.delete_exam_attempt()
+
+        self.assertIn('temporarily unavailable', str(context.exception))
+        self.assertEqual(context.exception.http_status, 502)
+        logger_mock.assert_called_once()
+        # the attempt must still exist because the removal failed
+        self.assertTrue(ProctoredExamStudentAttempt.objects.filter(id=attempt_id).exists())
+
+    @patch('logging.Logger.exception')
+    @patch('edx_proctoring.handlers.get_backend_provider')
+    def test_provider_error_response_preserves_message_and_keeps_attempt(self, get_backend_mock, logger_mock):
+        """
+        If the backend raises BackendProviderCannotRemoveAttempt (the provider responded
+        with an HTTP error), the handler re-raises it unchanged so the provider-supplied
+        message reaches the caller, and the local attempt must NOT be deleted.
+        """
+        attempt_id = self.attempt.id
+        backend = get_backend_mock.return_value
+        backend.remove_exam_attempt.side_effect = BackendProviderCannotRemoveAttempt('Attempt is locked on provider')
+
+        with self.assertRaises(BackendProviderCannotRemoveAttempt) as context:
+            with transaction.atomic():
+                self.attempt.delete_exam_attempt()
+
+        # the provider's message is preserved (not overwritten by the generic one)
+        self.assertIn('Attempt is locked on provider', str(context.exception))
+        self.assertEqual(context.exception.http_status, 502)
+        logger_mock.assert_called_once()
+        self.assertTrue(ProctoredExamStudentAttempt.objects.filter(id=attempt_id).exists())
+
     @ddt.data(None, MockInstructorService())
     @patch('edx_proctoring.handlers.get_runtime_service')
     @patch('edx_proctoring.tests.test_services.MockInstructorService.complete_student_attempt')

package.json

@@ -1,7 +1,7 @@
 {
   "name": "@edx/edx-proctoring",
   "//": "Note that the version format is slightly different than that of the Python version when using prereleases.",
-  "version": "6.0.0",
+  "version": "6.1.0",
   "main": "edx_proctoring/static/index.js",
   "scripts": {
     "test": "gulp test"