Skip to content

Commit

Permalink
Add exception and logging when backends return 200 but don't send id …
Browse files Browse the repository at this point in the history
…on exam attempt creation

We have seen ProctorTrack (third party service) would responde with 200 saying the exam
attempt is successful, but then the attempt ID is not sent to us in response.
See https://openedx.atlassian.net/browse/CR-1277
  • Loading branch information
Matt Hughes authored and schenedx committed Sep 27, 2019
1 parent ec890e5 commit aed10d5
Show file tree
Hide file tree
Showing 9 changed files with 111 additions and 38 deletions.
2 changes: 1 addition & 1 deletion edx_proctoring/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
55 changes: 46 additions & 9 deletions edx_proctoring/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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: '
Expand Down
9 changes: 7 additions & 2 deletions edx_proctoring/backends/rest.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
BackendProviderCannotRegisterAttempt,
BackendProviderCannotRetireUser,
BackendProviderOnboardingException,
BackendProviderSentNoAttemptID,
)
from edx_proctoring.statuses import ProctoredExamStudentAttemptStatus, SoftwareSecureReviewStatus
from edx_rest_api_client.client import OAuthAPIClient
Expand Down Expand Up @@ -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):
"""
Expand Down
2 changes: 1 addition & 1 deletion edx_proctoring/backends/software_secure.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 8 additions & 1 deletion edx_proctoring/backends/tests/test_backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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):
Expand Down
45 changes: 23 additions & 22 deletions edx_proctoring/backends/tests/test_rest.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
BackendProviderCannotRegisterAttempt,
BackendProviderCannotRetireUser,
BackendProviderOnboardingException,
BackendProviderSentNoAttemptID,
)
from edx_proctoring.statuses import ProctoredExamStudentAttemptStatus

Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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')
Expand All @@ -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'],
Expand Down
13 changes: 12 additions & 1 deletion edx_proctoring/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
12 changes: 12 additions & 0 deletions edx_proctoring/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
)
from edx_proctoring.constants import DEFAULT_CONTACT_EMAIL
from edx_proctoring.exceptions import (
BackendProviderSentNoAttemptID,
ProctoredExamAlreadyExists,
ProctoredExamNotFoundException,
StudentExamAttemptAlreadyExistsException,
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down

0 comments on commit aed10d5

Please sign in to comment.