diff --git a/edx_proctoring/__init__.py b/edx_proctoring/__init__.py index 8de5a36be38..baf5ab86e70 100644 --- a/edx_proctoring/__init__.py +++ b/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 diff --git a/edx_proctoring/api.py b/edx_proctoring/api.py index 03473ab6c59..306c7125827 100644 --- a/edx_proctoring/api.py +++ b/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: ' diff --git a/edx_proctoring/backends/rest.py b/edx_proctoring/backends/rest.py index ca45cf15e36..189dc11d4bb 100644 --- a/edx_proctoring/backends/rest.py +++ b/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): """ diff --git a/edx_proctoring/backends/software_secure.py b/edx_proctoring/backends/software_secure.py index 25c17581e12..7ac5a697179 100644 --- a/edx_proctoring/backends/software_secure.py +++ b/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 diff --git a/edx_proctoring/backends/tests/test_backend.py b/edx_proctoring/backends/tests/test_backend.py index e586c807abd..ba944100218 100644 --- a/edx_proctoring/backends/tests/test_backend.py +++ b/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,6 +34,7 @@ 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): """ @@ -37,6 +42,8 @@ def register_exam_attempt(self, exam, context): """ 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): diff --git a/edx_proctoring/backends/tests/test_rest.py b/edx_proctoring/backends/tests/test_rest.py index a628ed8d9dd..eadcd01b629 100644 --- a/edx_proctoring/backends/tests/test_rest.py +++ b/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,12 +196,6 @@ 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']), @@ -207,19 +203,13 @@ def test_register_exam_attempt_failure(self): 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']), @@ -227,10 +217,21 @@ def test_attempt_failure_onboarding(self, 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'], diff --git a/edx_proctoring/exceptions.py b/edx_proctoring/exceptions.py index c767194b176..06c8240def0 100644 --- a/edx_proctoring/exceptions.py +++ b/edx_proctoring/exceptions.py @@ -76,6 +76,17 @@ 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): """ @@ -83,7 +94,7 @@ class BackendProviderOnboardingException(ProctoredBaseException): because of missing/failed onboarding requirements """ def __init__(self, status): - ProctoredBaseException.__init__(self, status) + super(BackendProviderOnboardingException, self).__init__(self, status) self.status = status diff --git a/edx_proctoring/tests/test_api.py b/edx_proctoring/tests/test_api.py index c83f77d67f6..b6308633602 100644 --- a/edx_proctoring/tests/test_api.py +++ b/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 diff --git a/package.json b/package.json index 6959422de82..3ec15ca0a2e 100644 --- a/package.json +++ b/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",