An instructor reports that a Canvas assignment **when auto-approved** does not get an override posted to canvas. However manual request approvals d...#467
Open
cycomachead wants to merge 1 commit into
Conversation
Previously, CanvasFacade#provision_extension silently swallowed non-400 error responses (like 401, 403, or 422) by wrapping the error body in an Override object. This caused auto-approvals to be falsely marked as successful in the database even when no override was posted to Canvas. Now, the facade explicitly raises a CanvasAPIError for any non-success status codes (except for recoverable 400 "taken" errors). This ensures that Request#approve catches the failure, leaves the request in a pending state, and logs the specific error from Canvas for debugging. Co-authored-by: Claude Code <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
General Info
Changes
Fixes a bug where Canvas assignment overrides were silently not created during auto-approval, leaving the request falsely marked as "approved" in the database with no corresponding override in Canvas.
Root cause:
CanvasFacade#provision_extensionhad inverted logic for handling non-success responses. Any status code that wasn't400(e.g.401,403,422,5xx) would fall through and wrap the error body in anOverrideobject, returning it as if the override was successfully created. Back inRequest#approve, this fake success caused the request to be markedapprovedwith anil external_extension_id— no override ever existed in Canvas.Manual approvals appeared to work because they happened to succeed with
201; the silent failure only surfaced on assignments where Canvas rejected the override creation with a non-400 status.Fix: Rewrote the response-handling logic in
provision_extensionto be explicit about the three real cases:"taken"error → recoverable; fetch and update the existing override (unchanged behavior) ✅CanvasAPIErrorwith the HTTP status and Canvas response bodyNow when Canvas rejects an override,
Request#approve's existingrescuecatches the error, returnsfalse, and leaves the request in a pending state rather than a phantom approved state. The specific Canvas error is also surfaced in logs for debugging.Testing
401case, asserting thatprovision_extensionraisesCanvasAPIError(matchingHTTP 401) instead of silently returning a fake override.Documentation
No documentation changes required.
Checklist
Superconductor Ticket Implementation | App Preview | Guided Review