Fall back to AJAX on a Moodle course-context validation error#64
Merged
Conversation
list_courses() only fell back from the webservice transport to AJAX on an invalid/expired token (TransportUnavailableError); any other TransportError was re-raised immediately, per #40's stricter, deliberately-reviewed replacement of the previous bare except Exception. Confirmed in CI (PR #62, two separate runs) that a generic, non-course-scoped call like core_course_get_courses can still fail with 'You cannot execute functions in the course context (course id:N) ... Context does not exist' -- a course-context validation quirk unrelated to the token -- causing list_courses() (and anything built on it) to error out instead of transparently recovering via AJAX, which re-derives context from the current session rather than a cached webservice-side lookup. Broadens the webservice transport's fallback-worthy marker list to also cover this message. Added a failing-first regression test reproducing the exact CI error message, confirmed red against the pre-fix code via git stash, green after.
9 tasks
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.
Summary
list_courses()'s webservice-to-AJAX fallback only triggered on an invalid/expired token. Broadens it to also cover a real, CI-observed Moodle course-context validation quirk, so callers transparently recover via AJAX instead of erroring out.Linked issue
Closes #63
Specification
See #63 for the full write-up and CI evidence (two separate failed runs, three different tests affected, all on PR #62).
Implementation details
src/py_moodle/transport/webservice.py: renamed the single_INVALID_TOKEN_MARKERto a_TRANSPORT_UNAVAILABLE_MARKERStuple, adding"cannot execute functions in the course context"(the stable, generic part of the Moodle message — the course id and exact sub-message vary) alongside the existing invalid-token marker. Both now raiseTransportUnavailableError(fall back to AJAX) instead ofTransportError(raise immediately).tests/unit/test_transport.py: two new regression tests reproducing the exact error message seen in CI, at both thewebservice_transport.call()level and the publiclist_courses()level.Test plan
git stashon just the source file) — reproduced the exact CI traceback shape.test_transport.py.pytest tests/unitsuite (199 tests) andmake lintare still green.Commands run:
Backward compatibility
Fully backward compatible. Only broadens which errors trigger the existing AJAX fallback path (already exercised by
test_list_courses_falls_back_to_ajax_on_invalid_token); does not changelist_courses()'s signature, return shape, or any other function's behavior. Genuine, non-context, non-tokenTransportErrors (e.g. permissions denials) still raise immediately, unchanged, per the existingtest_webservice_call_raises_transport_error_on_other_moodle_exceptiontest.Risk assessment
The marker-string approach matches this file's existing, already-reviewed convention (
_INVALID_TOKEN_MARKER) rather than introducing a new classification mechanism. As with any English-substring match on a Moodle error message, it's not immune to locale/wording drift in a future Moodle release — same caveat that already applied to the invalid-token marker before this PR.Manual verification
Can't reproduce this specific race against a live Moodle instance from this environment on demand (it's timing-dependent), but the CI evidence in #63 is the real-world reproduction. The regression test in
test_transport.pyreproduces the exact error message deterministically without needing live Moodle or a race.Notes for reviewers
This was discovered while verifying PR #62 (test de-flaking), not something #62 introduced — #62 is unrelated test-fixture code; this is a production
src/py_moodle/transport/webservice.pyfix. Kept as its own focused PR rather than folded into #62.