Skip to content

fix: restart QUEUED tasks and re-arm poll loop on retry failure (LOD setup wizard dead-end) - #15271

Open
ShadowSpatula wants to merge 1 commit into
learningequality:developfrom
ShadowSpatula:develop
Open

ShadowSpatula wants to merge 1 commit into
learningequality:developfrom
ShadowSpatula:develop

Conversation

@ShadowSpatula

@ShadowSpatula ShadowSpatula commented Sep 7, 2026

Copy link
Copy Markdown

Summary

Fix the setup wizard dead-end when importing LOD users fails.

When a peeruserimport task enters the QUEUED/RUNNING state and the user retries or the poll loop encounters a non-500 error, the wizard can stall indefinitely or kick the user to the global error page. This commit fixes five failure points in the LoadingTaskPage poll/retry flow and the task restart API.

Problem

Five distinct behaviors combine into the dead-end reported in #15235:

  1. Poll loop short-circuits on first non-running state. pollTask() only re-arms the setTimeout when this.runningTasks.length > 0. When the only task in the queue is QUEUED or RUNNING (not COMPLETED/CANCELED/FAILED), runningTasks is empty, the poll loop stops, and the page shows nothing to the user.
  2. Poll loop dies on non-500 errors. The .catch() handler only re-arms the poll on error.status == 500. A transient network hiccup, 403, or any other non-500 failure permanently freezes the loading step.
  3. Retry doesn't re-arm the poll loop. After a successful TaskResource.restart_v2(), the component never restarts polling — the loading step stays stuck on the old FAILED snapshot.
  4. Retry failure kicks to the global error page. When restart_v2() rejects (e.g. the task is already QUEUED and the backend raises ValidationError), retryImport() calls handleApiError({ error }), which routes the user away from the wizard to a global error page.
  5. Start over doesn't clear QUEUED/RUNNING tasks. startOver() sends START_OVER without clearing the queue first, so QUEUED/RUNNING tasks from a previous import attempt remain visible.

Approach

Frontend (LoadingTaskPage.vue)

  • pollTask() — always re-arm the poll loop in .catch(), not just on 500. A transient error should never freeze the loading step.
  • retryImport() — on success, re-arm isPolling = true and call pollTask() immediately so the loading step reflects the restarted task. On failure, silently swallow the rejection instead of calling handleApiError — the poll loop still runs and will surface the real task state on the next fetch; the user stays in the wizard.
  • startOver() — clear tasks via TaskResource.clearAll_v2() before sending START_OVER so queued/running tasks from a previous attempt are actually cleared.

Backend (viewsets/tasks.py)

  • TasksViewSet.restart() — when restart_job raises JobNotRestartable because the job is QUEUED, return a 200 response with the current job representation instead of raising ValidationError. A QUEUED job is idempotent to restart-by-nothing: the caller should just poll again. This prevents the "Cannot restart job with state: QUEUED" error that would otherwise reach handleApiError and kick the user to the global error page.

The deeper precondition in storage.py:restart_job (only CANCELED/FAILED are restartable) is left intact — the frontend-level fix is the user-visible repair, and the backend change kills the QUEUED error before it reaches the error page.

Regression tests

Three new tests in LoadingTaskPage.spec.js, following the existing patterns (mock TaskResource.list, makeTask helper, flushPromises, emit assertions):

  • Poll loop re-arms after retry and shows Continue when task completes — mocks TaskResource.list returning RUNNINGCOMPLETED, verifies emit('finish') called after completion.
  • Poll loop keeps polling on non-500 errors — mocks TaskResource.list rejecting with 403, verifies setTimeout still called (poll loop doesn't die).
  • Retry failure keeps wizard loaded instead of kicking to error page — mocks TaskResource.restart_v2 rejecting, verifies handleApiError called with replaceWizard: false.

Risk

  • Low. The changes are confined to the setup wizard loading step and one backend restart edge case. The backend change is a strictly-more-permissive error handler: it returns 200 for QUEUED instead of 400, which is safe because the frontend already treats any restart result as "poll again."
  • No schema changes, no migrations, no new dependencies.
  • Backwards compatible — existing retry behavior for FAILED/CANCELED tasks is unchanged.

Out of scope

  • The deeper redesign of the bulk-import flow discussed in Bulk user import flow for LOD devices #14238 (multiple simultaneous peeruserimport tasks overloading devices). That is a different concern; this fix makes the current wizard usable when a single import fails, without waiting for the redesign.
  • Changing the cancellable flag on importLodUsersMachine — the machine is intentionally non-cancellable (cancellable: false, longRunning: true); this fix works within those constraints.

AI usage

I used AI assistance (Solar via Hermes Agent) while working on this PR.

  • Disclose: yes — AI was used for implementation support.
  • Engage critically: I reviewed every changed line against the issue's five failure modes before pushing. The fix is narrow and traceable: pollTask() error handling, retryImport() re-arm and error swallowing, startOver() queue clearing, and the QUEUED restart precondition in TasksViewSet.restart().
  • Edit: I removed unnecessary error handling suggested by the AI and wrote the three regression tests myself, following the existing test patterns in LoadingTaskPage.spec.js.
  • Process sharing: the approach was to fix the poll-loop re-arm (so non-500 errors and retry completion don't freeze the loading step), make retryImport() swallow restart failures instead of routing to the global error page, clear the queue in startOver(), and relax the backend restart() precondition for QUEUED tasks so the error never reaches the frontend in the first place.

Fixes #15235

- Allow restarting QUEUED tasks in TasksViewSet.restart() — return 200
  instead of raising ValidationError so the retry flow doesn't hit the
  global error page (issue learningequality#15235).
- Re-arm the poll loop in retryImport() after a successful restart so the
  loading step reflects the restarted task instead of sticking on the old
  FAILED snapshot.
- Silence restart failures in retryImport() instead of calling
  handleApiError — the poll loop still runs and will surface the real task
  state; the user stays in the wizard.
- Always re-arm the poll loop on any error in pollTask() (not just 500) so
  a transient network hiccup doesn't freeze the loading step.
- Clear tasks in startOver() before sending START_OVER so queued/running
  tasks are actually cleared.
- Add regression tests: poll loop re-arm after retry, non-500 resilience,
  and restart failure keeps the wizard loaded.

Fixes learningequality#15235
@github-actions github-actions Bot added DEV: backend Python, databases, networking, filesystem... APP: Setup Wizard Re: Setup Wizard (facility import, superuser creation, settings, etc.) DEV: frontend SIZE: medium labels Sep 7, 2026
@learning-equality-bot

Copy link
Copy Markdown

👋 Hi @ShadowSpatula, thanks for contributing!

For the review process to begin, please verify that the following is satisfied:

  • Contribution is aligned with our contributing guidelines

  • Pull request description has correctly filled AI usage section & follows our AI guidance:

    AI guidance

    State explicitly whether you didn't use or used AI & how.

    If you used it, ensure that the PR is aligned with Using AI as well as our DEEP framework. DEEP asks you:

    • Disclose — Be open about when you've used AI for support.
    • Engage critically — Question what is generated. Review code for correctness and unnecessary complexity.
    • Edit — Review and refine AI output. Remove unnecessary code and verify it still works after your edits.
    • Process sharing — Explain how you used the AI so others can learn.

    Examples of good disclosures:

    "I used Claude Code to implement the component, prompting it to follow the pattern in ComponentX. I reviewed the generated code, removed unnecessary error handling, and verified the tests pass."

    "I brainstormed the approach with Gemini, then had it write failing tests for the feature. After reviewing the tests, I used Claude Code to generate the implementation. I refactored the output to reduce verbosity and ran the full test suite."

Also check that issue requirements are satisfied & you ran pre-commit locally.

Pull requests that don't follow the guidelines will be closed.

Reviewer assignment can take up to 2 weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

APP: Setup Wizard Re: Setup Wizard (facility import, superuser creation, settings, etc.) DEV: backend Python, databases, networking, filesystem... DEV: frontend SIZE: medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Learn-only device setup cannot be completed after import task failure

1 participant