Merge pull request #605 from edx/matthugs/schen/draft-proctortrack-is…

…sue-visibility-CR-1266

Add better exceptions when backends return 200 but don't send id

edx_proctoring/__init__.py

@@ -5,6 +5,6 @@
 from __future__ import absolute_import
 
 # Be sure to update the version number in edx_proctoring/package.json
-__version__ = '2.1.0'
+__version__ = '2.1.1'
 
 default_app_config = 'edx_proctoring.apps.EdxProctoringConfig'  # pylint: disable=invalid-name

edx_proctoring/api.py

@@ -25,13 +25,21 @@
 
 from edx_proctoring import constants
 from edx_proctoring.backends import get_backend_provider
-from edx_proctoring.exceptions import (BackendProviderOnboardingException, ProctoredExamAlreadyExists,
-                                       ProctoredExamIllegalStatusTransition, ProctoredExamNotActiveException,
-                                       ProctoredExamNotFoundException, ProctoredExamPermissionDenied,
-                                       ProctoredExamReviewPolicyAlreadyExists,
-                                       ProctoredExamReviewPolicyNotFoundException,
-                                       StudentExamAttemptAlreadyExistsException,
-                                       StudentExamAttemptDoesNotExistsException, StudentExamAttemptedAlreadyStarted)
+from edx_proctoring.exceptions import (
+    BackendProviderCannotRegisterAttempt,
+    BackendProviderOnboardingException,
+    BackendProviderSentNoAttemptID,
+    ProctoredExamAlreadyExists,
+    ProctoredExamIllegalStatusTransition,
+    ProctoredExamNotActiveException,
+    ProctoredExamNotFoundException,
+    ProctoredExamPermissionDenied,
+    ProctoredExamReviewPolicyAlreadyExists,
+    ProctoredExamReviewPolicyNotFoundException,
+    StudentExamAttemptAlreadyExistsException,
+    StudentExamAttemptDoesNotExistsException,
+    StudentExamAttemptedAlreadyStarted,
+)
 from edx_proctoring.models import (ProctoredExam, ProctoredExamReviewPolicy, ProctoredExamSoftwareSecureReview,
                                    ProctoredExamStudentAllowance, ProctoredExamStudentAttempt)
 from edx_proctoring.runtime import get_runtime_service
@@ -629,8 +637,37 @@ def create_exam_attempt(exam_id, user_id, taking_as_proctored=False):
                 exam,
                 context=context,
             )
-        except BackendProviderOnboardingException as exc:
-            force_status = exc.status
+        except BackendProviderSentNoAttemptID as ex:
+            log_message = (
+                'Failed to get the attempt ID for {user_id}'
+                'in {exam_id} from the backend because the backend'
+                'did not provide the id in API response, even when the'
+                'HTTP response status is {status}, '
+                'Response: {response}'.format(
+                    user_id=user_id,
+                    exam_id=exam_id,
+                    response=six.text_type(ex),
+                    status=ex.http_status
+                )
+            )
+            log.error(log_message)
+            raise ex
+        except BackendProviderCannotRegisterAttempt as ex:
+            log_message = (
+                'Failed to create attempt for {user_id} '
+                'in {exam_id} because backend was unable '
+                'to register the attempt. Status: {status}, '
+                'Reponse: {response}'.format(
+                    user_id=user_id,
+                    exam_id=exam_id,
+                    response=six.text_type(ex),
+                    status=ex.http_status,
+                )
+            )
+            log.error(log_message)
+            raise ex
+        except BackendProviderOnboardingException as ex:
+            force_status = ex.status
             log_msg = (
                 'Failed to create attempt for {user_id} '
                 'in {exam_id} because of onboarding failure: '

edx_proctoring/backends/rest.py

@@ -21,6 +21,7 @@
     BackendProviderCannotRegisterAttempt,
     BackendProviderCannotRetireUser,
     BackendProviderOnboardingException,
+    BackendProviderSentNoAttemptID,
 )
 from edx_proctoring.statuses import ProctoredExamStudentAttemptStatus, SoftwareSecureReviewStatus
 from edx_rest_api_client.client import OAuthAPIClient
@@ -187,13 +188,17 @@ def register_exam_attempt(self, exam, context):
         log.debug('Creating exam attempt for %r at %r', exam['external_id'], url)
         response = self.session.post(url, json=payload)
         if response.status_code != 200:
-            raise BackendProviderCannotRegisterAttempt(response.content)
+            raise BackendProviderCannotRegisterAttempt(response.content, response.status_code)
+        status_code = response.status_code
         response = response.json()
         log.debug(response)
         onboarding_status = response.get('status', None)
         if onboarding_status in ProctoredExamStudentAttemptStatus.onboarding_errors:
             raise BackendProviderOnboardingException(onboarding_status)
-        return response['id']
+        attempt_id = response.get('id', None)
+        if not attempt_id:
+            raise BackendProviderSentNoAttemptID(response, status_code)
+        return attempt_id
 
     def start_exam_attempt(self, exam, attempt):
         """

edx_proctoring/backends/software_secure.py

@@ -94,7 +94,7 @@ def register_exam_attempt(self, exam, context):
                 )
             )
             log.error(err_msg)
-            raise BackendProviderCannotRegisterAttempt(err_msg)
+            raise BackendProviderCannotRegisterAttempt(err_msg, status)
 
         # get the external ID that Software Secure has defined
         # for this attempt

edx_proctoring/backends/tests/test_backend.py

@@ -14,7 +14,11 @@
 from edx_proctoring.backends.backend import ProctoringBackendProvider
 from edx_proctoring.backends.null import NullBackendProvider
 from edx_proctoring.backends.mock import MockProctoringBackendProvider
-from edx_proctoring.exceptions import BackendProviderCannotRetireUser, BackendProviderOnboardingException
+from edx_proctoring.exceptions import (
+    BackendProviderCannotRetireUser,
+    BackendProviderOnboardingException,
+    BackendProviderSentNoAttemptID,
+)
 
 # pragma pylint: disable=useless-super-delegation
 
@@ -30,13 +34,16 @@ class TestBackendProvider(ProctoringBackendProvider):
     last_retire_user = None
     attempt_error = None
     last_attempt_remove = None
+    no_attempt_id_error = None
 
     def register_exam_attempt(self, exam, context):
         """
         Called when the exam attempt has been created but not started
         """
         if self.attempt_error:
             raise BackendProviderOnboardingException(self.attempt_error)
+        elif self.no_attempt_id_error:
+            raise BackendProviderSentNoAttemptID(self.no_attempt_id_error, http_status=200)
         return 'testexternalid'
 
     def start_exam_attempt(self, exam, attempt):

edx_proctoring/backends/tests/test_rest.py

@@ -17,6 +17,7 @@
     BackendProviderCannotRegisterAttempt,
     BackendProviderCannotRetireUser,
     BackendProviderOnboardingException,
+    BackendProviderSentNoAttemptID,
 )
 from edx_proctoring.statuses import ProctoredExamStudentAttemptStatus
 
@@ -45,6 +46,12 @@ def setUp(self):
             'is_proctored': True,
             'is_practice': False,
         }
+        self.register_exam_context = {
+            'attempt_code': '2',
+            'obs_user_id': 'abcdefghij',
+            'full_name': 'user name',
+            'lms_host': 'http://lms.com'
+        }
 
     @responses.activate
     def test_get_software_download_url(self):
@@ -174,18 +181,13 @@ def test_bad_exam_save(self):
 
     @responses.activate
     def test_register_exam_attempt(self):
-        context = {
-            'attempt_code': '2',
-            'obs_user_id': 'abcdefghij',
-            'full_name': 'user name',
-            'lms_host': 'http://lms.com'
-        }
         responses.add(
             responses.POST,
             url=self.provider.create_exam_attempt_url.format(exam_id=self.backend_exam['external_id']),
-            json={'id': 2}
+            json={'id': 2},
+            status=200
         )
-        attempt_external_id = self.provider.register_exam_attempt(self.backend_exam, context)
+        attempt_external_id = self.provider.register_exam_attempt(self.backend_exam, self.register_exam_context)
         request = json.loads(responses.calls[1].request.body)
         self.assertEqual(attempt_external_id, 2)
         self.assertEqual(request['status'], 'created')
@@ -194,43 +196,42 @@ def test_register_exam_attempt(self):
 
     @responses.activate
     def test_register_exam_attempt_failure(self):
-        context = {
-            'attempt_code': '2',
-            'obs_user_id': 'abcdefghij',
-            'full_name': 'user name',
-            'lms_host': 'http://lms.com'
-        }
         responses.add(
             responses.POST,
             url=self.provider.create_exam_attempt_url.format(exam_id=self.backend_exam['external_id']),
             json={'error': 'something'},
             status=400
         )
         with self.assertRaises(BackendProviderCannotRegisterAttempt):
-            self.provider.register_exam_attempt(self.backend_exam, context)
+            self.provider.register_exam_attempt(self.backend_exam, self.register_exam_context)
 
     @ddt.data(
         *ProctoredExamStudentAttemptStatus.onboarding_errors
     )
     @responses.activate
     def test_attempt_failure_onboarding(self, failure_status):
-        context = {
-            'attempt_code': '2',
-            'obs_user_id': 'abcdefghij',
-            'full_name': 'user name',
-            'lms_host': 'http://lms.com'
-        }
         responses.add(
             responses.POST,
             url=self.provider.create_exam_attempt_url.format(exam_id=self.backend_exam['external_id']),
             json={'status': failure_status},
             status=200
         )
         with self.assertRaises(BackendProviderOnboardingException) as exc_manager:
-            self.provider.register_exam_attempt(self.backend_exam, context)
+            self.provider.register_exam_attempt(self.backend_exam, self.register_exam_context)
         exception = exc_manager.exception
         assert exception.status == failure_status
 
+    @responses.activate
+    def test_attempt_no_id_returned(self):
+        responses.add(
+            responses.POST,
+            url=self.provider.create_exam_attempt_url.format(exam_id=self.backend_exam['external_id']),
+            json={'error': 'something'},
+            status=200
+        )
+        with self.assertRaises(BackendProviderSentNoAttemptID):
+            self.provider.register_exam_attempt(self.backend_exam, self.register_exam_context)
+
     @ddt.data(
         ['start_exam_attempt', 'start'],
         ['stop_exam_attempt', 'stop'],

edx_proctoring/exceptions.py

@@ -76,14 +76,25 @@ class BackendProviderCannotRegisterAttempt(ProctoredBaseException):
     Raised when a back-end provider cannot register an attempt
     """
 
+    def __init__(self, content, http_status):
+        super(BackendProviderCannotRegisterAttempt, self).__init__(self, content)
+        self.http_status = http_status
+
+
+class BackendProviderSentNoAttemptID(BackendProviderCannotRegisterAttempt):
+    """
+    Raised when a back-end provider returns a JSON without exam ID
+    in response to new exam attempt registration
+    """
+
 
 class BackendProviderOnboardingException(ProctoredBaseException):
     """
     Raised when a back-end provider cannot register an attempt
     because of missing/failed onboarding requirements
     """
     def __init__(self, status):
-        ProctoredBaseException.__init__(self, status)
+        super(BackendProviderOnboardingException, self).__init__(self, status)
         self.status = status
 
 

edx_proctoring/tests/test_api.py

@@ -57,6 +57,7 @@
 )
 from edx_proctoring.constants import DEFAULT_CONTACT_EMAIL
 from edx_proctoring.exceptions import (
+    BackendProviderSentNoAttemptID,
     ProctoredExamAlreadyExists,
     ProctoredExamNotFoundException,
     StudentExamAttemptAlreadyExistsException,
@@ -560,6 +561,17 @@ def test_attempt_onboarding_error(self, onboarding_error):
         assert attempt['status'] == onboarding_error
         test_backend.attempt_error = None
 
+    def test_attempt_without_attempt_id(self):
+        """
+        Test that the exam attempt is throwing exceptions when the attempt id is not included in the
+        API response from proctoring backend
+        """
+        test_backend = get_backend_provider(name='test')
+        test_backend.no_attempt_id_error = 'No id returned'
+        with self.assertRaises(BackendProviderSentNoAttemptID):
+            create_exam_attempt(self.proctored_exam_id, self.user_id, taking_as_proctored=True)
+        test_backend.no_attempt_id_error = None
+
     def test_no_existing_attempt(self):
         """
         Make sure we get back a None when calling get_exam_attempt_by_id() with a non existing attempt

package.json

@@ -2,7 +2,7 @@
   "name": "@edx/edx-proctoring",
   "//": "Be sure to update the version number in edx_proctoring/__init__.py",
   "//": "Note that the version format is slightly different than that of the Python version when using prereleases.",
-  "version": "2.1.0",
+  "version": "2.1.1",
   "main": "edx_proctoring/static/index.js",
   "repository": {
     "type": "git",