Retry Canvas submission on timeout

[marcospri]

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" allowing the FE to retry them.

Testing

• Apply a diff like

diff --git a/lms/views/api/grading.py b/lms/views/api/grading.py
index 7b072a36d..430d0f0cc 100644
--- a/lms/views/api/grading.py
+++ b/lms/views/api/grading.py
@@ -103,6 +103,9 @@ class GradingViews:
         # absence of a submission from an ungraded submission. Non-Canvas LMSes in
         # theory require a grade).
 
+        if not self.request.headers.get("Retry-Count"):
+            raise ExternalRequestError(retryable=True)
+
         lis_result_sourcedid = self.parsed_params["lis_result_sourcedid"]
         try:
             # If we already have a score, then we've already recorded this info

to simulate a retry-success scenario.

• Go to https://hypothesis.instructure.com/courses/319/assignments/3336 logged in as a student

• Check the devtools, make an annotation.

• You'll see to API request to submissions and a log line like:

2025-02-19 11:14:15,976 DEBUG [lms.views.helpers._logging:13][MainThread] Request to /api/lti/submissions succeeded after 1 retries

on the server logs.

[marcospri]

The default, 10, seems excessive

[marcospri]

This looks like duplicating the same value but I reckon the header and the retryCount are relatively independent, one is part of the request and the other is part of our own code logic.

But I might be missing a nice trick.

[acelaya]

A few lines above there's this block:

const headers: Record<string, string> = {
  Authorization: authToken,
};

You could update it and set the Retry-Count header there, if retryCount > 0.

const headers: Record<string, string> = {
  Authorization: authToken,
};
+if (retryCount > 0) {
+  headers['Retry-Count'] = `${retryCount}`;
+}

That would reduce the amount of nested object spreading, and the duplication you mentioned above.

[marcospri]

Only applying this and (err.retryable = True) on this view.

If we merge this approach I'll create an issue to track generalizing this approach for other views.

[marcospri]

This feels like a lot of ...s

[acelaya]

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

And the frontend... What? Is this a cliffhanger for season 2? 😂

[acelaya]

A few lines above there's this block:

const headers: Record<string, string> = {
  Authorization: authToken,
};

You could update it and set the Retry-Count header there, if retryCount > 0.

const headers: Record<string, string> = {
  Authorization: authToken,
};
+if (retryCount > 0) {
+  headers['Retry-Count'] = `${retryCount}`;
+}

That would reduce the amount of nested object spreading, and the duplication you mentioned above.

[acelaya]

I reckon none of the following conditions would match for a timeout, so you could just set err.retryable = err.is_timeout, but this is definitely more predictable.

[acelaya]

Being pedantic here, but I think the common practice is to prefix custom headers with X-, as in X-Retry-Count.

[marcospri]

I did out-pedantic myself before reading this:

https://www.rfc-editor.org/rfc/rfc6648.html#section-3

 3.  SHOULD NOT prefix their parameter names with "X-" or similar
       constructs.

I reckon the rationale there is that you start naming things X-, then become common usage but they endup stuck with the X-. I don't think there's are risk of that happening here so I don't have a strong opinion.

[acelaya]

Oh, wow!

I think the rational around adding the X- prefix is making sure you don't end up using a header that eventually becomes part of the standard, but I don't think there's a high risk that Retry-Count is added to the spec any time soon, without us noticing.

Let's go with Retry-Count then.