Retry Canvas submission on retry

Canvas submissions are sent when the first annotations is made during a
"session" on a gradable assignment.

We have some error handling and display error dialogs accordingly but due to the indirect
nature of the action is tricky to handle transactional errors like timeouts.

This commits adds a "flag" on the response of API calls to our own frontend marking
the request as "retryable" and the frontend.

lms/services/exceptions.py

@@ -1,5 +1,7 @@
 import logging
 
+from requests.exceptions import Timeout
+
 from lms.models.oauth2_token import Service
 
 log = logging.getLogger(__name__)
@@ -25,6 +27,7 @@ class ExternalRequestError(Exception):
     :type message: str
 
     :arg refreshable: True if the error can be fixed by refreshing an access token
+    :arg retryable: True if the frontend should retry the request as an attempt to fix the error
     :arg refresh_route:
         If `refreshable` is True, the name of the API route that should be
         invoked to perform the token refresh. If `None`, the default route for
@@ -50,6 +53,7 @@ def __init__(  # noqa: PLR0913
         response=None,
         validation_errors=None,
         refreshable=False,  # noqa: FBT002
+        retryable=False,  # noqa: FBT002
         refresh_route: str | None = None,
         refresh_service: Service | None = None,
     ):
@@ -59,6 +63,7 @@ def __init__(  # noqa: PLR0913
         self.response = response
         self.validation_errors = validation_errors
         self.refreshable = refreshable
+        self.retryable = retryable
         self.refresh_route = refresh_route
         self.refresh_service = refresh_service
 
@@ -92,6 +97,11 @@ def response_body(self) -> str | None:
         """Return the response body."""
         return getattr(self.response, "text", None)
 
+    @property
+    def is_timeout(self) -> bool:
+        """Return True if the error was caused by a timeout."""
+        return isinstance(self.__cause__, Timeout)
+
     def __repr__(self) -> str:
         return _repr_external_request_exception(self)
 

lms/static/scripts/frontend_apps/components/BasicLTILaunchApp.tsx

@@ -254,6 +254,7 @@ export default function BasicLTILaunchApp() {
           authToken,
           path: '/api/lti/submissions',
           data: submissionParams,
+          maxRetries: 2,
         });
       } catch (e) {
         // If reporting the submission failed, show a modal error dialog above

lms/static/scripts/frontend_apps/components/test/BasicLTILaunchApp-test.js

@@ -453,6 +453,7 @@ describe('BasicLTILaunchApp', () => {
       const apiCall = fakeApiCall.getCall(0);
       assert.deepEqual(apiCall.args[0], {
         authToken: 'dummyAuthToken',
+        maxRetries: 2,
         path: '/api/lti/submissions',
         data: {
           submitted_at: undefined,
@@ -596,6 +597,7 @@ describe('BasicLTILaunchApp', () => {
               const apiCall = fakeApiCall.getCall(0);
               assert.deepEqual(apiCall.args[0], {
                 authToken: 'dummyAuthToken',
+                maxRetries: 2,
                 path: '/api/lti/submissions',
                 data: {
                   submitted_at: undefined,
@@ -638,6 +640,7 @@ describe('BasicLTILaunchApp', () => {
               const apiCall = fakeApiCall.getCall(0);
               assert.deepEqual(apiCall.args[0], {
                 authToken: 'dummyAuthToken',
+                maxRetries: 2,
                 path: '/api/lti/submissions',
                 data: {
                   submitted_at: '2022-04-28T13:25:34Z',

lms/static/scripts/frontend_apps/utils/api.ts

@@ -84,6 +84,10 @@ function delay(ms: number) {
   return new Promise(resolve => setTimeout(resolve, ms));
 }
 
+function isRetryableResponse(status: number, data: any): boolean {
+  return status === 409 || (data && data.retryable);
+}
+
 /**
  * Make an API call to the LMS app backend.
  */
@@ -127,7 +131,10 @@ export async function apiCall<Result = unknown>(
   const resultJSON = await result.json();
 
   if (result.status >= 400 && result.status < 600) {
-    if (result.status === 409 && retryCount < maxRetries) {
+    if (
+      isRetryableResponse(result.status, resultJSON) &&
+      retryCount < maxRetries
+    ) {
       await delay(retryDelay);
       return apiCall({ ...options, retryCount: retryCount + 1 });
     }

lms/views/api/exceptions.py

@@ -295,6 +295,10 @@ def __json__(self, request):
                     )
                 body["refresh"] = {"method": "POST", "path": path}
 
+        # Expose whether or not the exception is retryable to the frontend.
+        if getattr(request.exception, "retryable", False):
+            body["retryable"] = True
+
         return body
 
 

lms/views/api/grading.py

@@ -117,6 +117,11 @@ def record_canvas_speedgrader_submission(self):
             )
 
         except ExternalRequestError as err:
+            if err.is_timeout:
+                # We'll inform the frontend that this is a retryable error for timeouts.
+                err.retryable = True
+                raise
+
             if (
                 err.status_code == 422
                 # This is LTI1.3 only. In LTI1.1 canvas behaves differently and allows this type of submissions.

tests/unit/lms/views/api/exceptions_test.py

@@ -246,6 +246,11 @@ class TestErrorBody:
     def test_json(self, pyramid_request, error_body, expected):
         assert error_body.__json__(pyramid_request) == expected
 
+    def test_json_retryable_exception(self, pyramid_request):
+        pyramid_request.exception.retryable = True
+
+        assert ErrorBody().__json__(pyramid_request) == {"retryable": True}
+
     @pytest.mark.usefixtures("with_refreshable_exception")
     def test_json_includes_refresh_info_if_the_exception_is_refreshable(
         self, pyramid_request, oauth2_token_service

tests/unit/lms/views/api/grading_test.py

@@ -3,6 +3,7 @@
 
 import pytest
 from h_matchers import Any
+from requests.exceptions import Timeout
 
 from lms.services.exceptions import ExternalRequestError, SerializableError
 from lms.services.lti_grading.interface import GradingResult
@@ -41,6 +42,16 @@ def test_it_does_not_record_result_if_score_already_exists(
 
         lti_grading_service.record_result.assert_not_called()
 
+    def test_it_timeout(self, lti_grading_service, pyramid_request):
+        exception = ExternalRequestError()
+        exception.__cause__ = Timeout()
+        lti_grading_service.read_result.side_effect = exception
+
+        with pytest.raises(ExternalRequestError) as exc_info:
+            GradingViews(pyramid_request).record_canvas_speedgrader_submission()
+
+        assert exc_info.value.retryable
+
     def test_it_max_attempts(self, pyramid_request, lti_grading_service):
         lti_grading_service.read_result.side_effect = ExternalRequestError(
             response=Mock(