Skip to content

Honor save failures and surface USB-MSC hint when the filesystem is locked#498

Merged
makermelissa merged 8 commits into
circuitpython:mainfrom
makermelissa-piclaw:fix/issue-460-save-run-await
May 19, 2026
Merged

Honor save failures and surface USB-MSC hint when the filesystem is locked#498
makermelissa merged 8 commits into
circuitpython:mainfrom
makermelissa-piclaw:fix/issue-460-save-run-await

Conversation

@makermelissa-piclaw
Copy link
Copy Markdown
Contributor

@makermelissa-piclaw makermelissa-piclaw commented May 19, 2026

Summary

Fixes #460 — "Cannot Save code.py Edits on Adafruit Qualia ESP32-S3."

The root cause, after walking through the firmware code, is that the
board's filesystem is write-locked by USB Mass Storage
: when the host
computer has CIRCUITPY mounted, CircuitPython can't write through FatFS
even though the web workflow happily accepts the request. The current
firmware reports this as HTTP 500 Internal Server Error, which looks
like a server crash and gave the editor nothing actionable to show.
Confirmed by repro: the failure disappears the moment USB MSC is
disabled in boot.py.

This PR fixes the editor's behavior in three ways:

  1. Stop pretending failed saves succeeded. The previous
    saveFileContents() retried via a fire-and-forget setTimeout, so
    workflow.saveFile() returned true immediately while retries ran
    in the background. saveRunFile() then soft-restarted the board
    while the PUT hadn't actually landed yet, running the previously
    saved code.py.
  2. Show an actionable hint when the device tells us the filesystem
    is locked, instead of the generic "failed after multiple attempts."
  3. Skip pointless retries when the failure is "host owns the disk"
    — retrying can't help until the user ejects.

A companion firmware PR (adafruit/circuitpython#11017)
brings the file-writing PUT in line with DELETE / MOVE / mkdir-PUT in
the same file, which already reply 409 Conflict for
FR_WRITE_PROTECTED. The editor handles both 500 (old firmware) and
409 (new firmware) the same way, so users on any CP version benefit
from the better error message.

Root cause

The prior PR #472 capped the runaway retry loop (the "editor reloads
every 1-2 seconds" symptom) but the deeper bug was still there:

  • saveFileContents() retried via setTimeout(saveFileContents, 2000)
    inside the catch block.
  • The outer call returned normally, so workflow.saveFile()
    unconditionally returned true.
  • saveRunFile() saw a truthy result and called
    workflow.runCurrentCode()repl.softRestart().
  • The board rebooted into the OLD code.py while retries fired against
    a rebooting device, all failed, and the editor showed
    "Saving file failed after multiple attempts" — but had already
    flipped its UI back to "saved," so the unsaved buffer was lost too.

Two bugs stacked: a misleading server response from the firmware AND
the editor lying about save success to the rest of the app.

Changes

js/script.js

  • saveFileContents() is now a real awaitable retry loop:

    • 3 attempts, 2 s apart, using await sleep(...) instead of
      fire-and-forget setTimeout.

    • Returns true on success, false on exhausted retries (or on
      disconnect mid-retry).

    • saveInFlight flag refuses re-entrant invocations so mashing
      Ctrl-S / Save+Run doesn't race two PUTs onto the wire and corrupt
      partialWrites bookkeeping.

    • On any failure the editor stays marked dirty (setSaved(false)),
      so the unsaved buffer is recoverable.

    • New: when the device cleanly says the filesystem is held by
      someone else (409 Conflict or 500 Internal Server Error on a
      /fs/ PUT), skip the remaining retries and show:

      Could not save '/code.py'.

      The board's filesystem is locked, usually because CIRCUITPY is
      mounted on a computer over USB.

      To fix:

      • Disconnect the USB cable, or
      • Disable USB Mass Storage in boot.py, then reset the board.

      Note: ejecting the drive in your OS isn't always enough on its own.

      Disabling USB Mass Storage (Adafruit Learn)

      Your edits are still here — save again once the filesystem is writable.

      The dialog renders headers, list, and link via real HTML
      (MessageModal uses innerHTML), so it scans cleanly instead of
      being a wall of prose.

      The corresponding connect-time read-only warning was shortened
      to a single line so it doesn't shout at every connect:

      Filesystem is read-only — you can browse files, but saving
      will fail until USB Mass Storage is released.
      How to fix.

      (The "eject may not be enough" caveat reflects real-world
      behavior on macOS: Finder's Eject command sometimes unmounts the
      filesystem locally without sending the SCSI START_STOP_UNIT
      eject command, so the device-side blockdev_unlock() never
      fires and STA_PROTECT stays set.)

  • saveRunFile() drops the redundant setSaved(true);
    saveFileContents() already handles it on success, and we only
    reach runCurrentCode() when the save actually worked.

  • disconnectCallback() no longer juggles a retry timeout — the
    inline loop owns its own state now and bails out cleanly when
    connectionStatus() flips false between attempts.

js/workflows/workflow.js

  • saveFile() propagates the real result from _saveFileContents()
    instead of unconditionally returning true. Uses result !== false
    so any legacy saveFileFunc callback returning undefined still
    counts as success.

js/common/web-file-transfer.js

  • _fetch() attaches numeric status, method, and path to the thrown
    ProtocolError, and tags /fs/ PUT failures with status 409 or
    500 as writeProtected = true with a human-readable hint.
    Additive — callers that ignore the new fields get the same
    ProtocolError they used to.
  • writeFile() returns response.ok so the workflow layer sees real
    success / failure (matches what makeDir() already does).

Behavior after the fix

Scenario Before After
PUT succeeds first try UI marks saved ✅ UI marks saved ✅
Host has CIRCUITPY mounted (current firmware → 500) "Failed after multiple attempts" after ~6s of retries; board soft-restarted with stale code ❌ Immediate "unplug USB / disable USB MSC in boot.py" message; no retries; no soft-restart ✅
Host has CIRCUITPY mounted (with #11017 → 409) n/a Same as above ✅
Transient network glitch Retries 2× (sometimes worked, sometimes restarted before completion) Retries 2× inline; Save+Run waits for real success ✅
Persistent network failure UI flipped to "saved," unsaved buffer lost, soft-restart anyway ❌ Editor stays dirty, no restart, generic error dialog, buffer recoverable ✅
User mashes Ctrl-S during save Two overlapping PUTs, partialWrites offsets confused Second call no-ops with a console message
Disconnect mid-retry disconnectCallback had to clean up a global timeout Inline loop notices !connectionStatus() and bails

Why detect both 409 and 500

CircuitPython firmware versions that ship today return 500 for the
write-protected case. The companion firmware PR
(adafruit/circuitpython#11017)
will return 409 Conflict to match the rest of the web workflow's
error contract. Users will be on mixed firmware versions for years,
so the editor handles both the same way. The detection is gated on
the request being a PUT to /fs/, so generic 500s elsewhere are
unaffected.

Testing

  • npm run build (vite) passes clean.
  • node --check on all edited files passes.
  • Manual reproduction:
    • Before: Mount CIRCUITPY on host, hit Save+Run → editor showed
      "Saving file failed after multiple attempts," board soft-restarted
      with stale code.
    • After: Same flow → immediate "Eject CIRCUITPY / disable USB
      MSC in boot.py" message, no soft-restart, buffer stays in the
      editor.
    • With USB MSC disabled in boot.py: save works on the first try
      as expected.
  • Manually traced BLE (partialWrites=true), web workflow
    (partialWrites=false), checkSaved() unsaved dialog, and the
    Ctrl-S / Save+Run / Mod-r hotkey paths through the refactored code.

Refs adafruit/circuitpython#11017

Closes #460

Refactor saveFileContents to use an awaitable retry loop and propagate
its real success/failure to callers. Previously the catch block did a
fire-and-forget setTimeout for retries while the outer call returned
normally, so workflow.saveFile() always reported success even when the
PUT had failed. That let saveRunFile() proceed to runCurrentCode()
(soft-restart for code.py, import for everything else) before the file
on the device was actually updated -- exactly the 'edits disappear
after Save+Run' symptom from issue circuitpython#460.

js/script.js:
- saveFileContents() now retries inline (3 attempts, 2s apart) and
  returns true on success / false on exhausted retries. It also
  early-exits if the workflow disconnects mid-retry, and refuses
  re-entrant invocations via a saveInFlight flag.
- On final failure it leaves the editor marked dirty (setSaved(false))
  so the user can see the file on the board is still stale, instead of
  silently flipping the UI back to 'saved'.
- saveRunFile() drops the redundant setSaved(true); saveFileContents()
  already handles it on success, and we only reach runCurrentCode()
  when the save actually worked.
- disconnectCallback() no longer juggles the retry timeout (the inline
  loop owns its own state now).

js/workflows/workflow.js:
- saveFile() returns the underlying _saveFileContents() result instead
  of unconditionally returning true. Uses result !== false so legacy
  callbacks that return undefined still count as success.

Build verified with vite. The Qualia-specific root cause (RGB display
contention starving the web server) is a firmware/board concern; this
PR makes sure the editor stops pretending writes succeeded when they
did not, and keeps the unsaved buffer recoverable.

Closes circuitpython#460
…locked

When the host has CIRCUITPY mounted over USB Mass Storage, the
CircuitPython side of FatFS can't write through the block device and
the web workflow PUT /fs/<file> fails. Older firmware returns HTTP 500
for this (lumped into the generic 'result != FR_OK' branch); a pending
firmware fix (adafruit/circuitpython#11017) will return HTTP 409
Conflict to match DELETE / MOVE / mkdir-PUT in the same file. Users
will be on mixed firmware versions for years, so the editor should
handle both the same way.

Changes:

- web-file-transfer.js _fetch(): instead of throwing a bare
  ProtocolError(response.statusText), attach the numeric status,
  method, and path to the error. When the failure is a 409 OR 500
  on a /fs/ PUT, tag the error with .writeProtected = true and a
  human-readable .hint string. Generic fetch errors are unaffected.

- web-file-transfer.js writeFile(): return response.ok so the
  workflow layer sees real success / failure (matches what makeDir
  already does).

- script.js saveFileContents(): when the caught error is tagged
  .writeProtected, skip the remaining retries (they can't help --
  the host owns the disk) and show an actionable message naming
  USB-MSC and boot.py. Leaves the editor dirty so the unsaved
  buffer is recoverable once the user releases the drive.

Backwards compatible with existing firmware: same retry loop applies
to anything that isn't a recognized write-protected response, and the
new error fields are additive (callers that ignore them get the same
ProtocolError instance they used to).

Refs adafruit/circuitpython#11017
Closes circuitpython#460
@makermelissa-piclaw makermelissa-piclaw changed the title Make Save+Run actually wait for the save and stop on failure Honor save failures and surface USB-MSC hint when the filesystem is locked May 19, 2026
Testing on macOS showed that simply ejecting the CIRCUITPY drive in
Finder does not always release the SCSI lock CircuitPython sees as
STA_PROTECT -- the OS may unmount the filesystem locally without
issuing the SCSI START_STOP_UNIT eject command that would trigger
the board-side blockdev_unlock(). The save still fails until the
USB cable is disconnected or USB MSC is disabled in boot.py and the
board is reset.

Reword the user-facing hint to:

- Lead with what actually works (unplug USB / disable MSC in boot.py
  + reset).
- Mention 'eject in your OS may not be enough on its own' so users
  who already tried eject don't bounce off the dialog.
- Stop telling them to 'save again once the drive is released' --
  releasing the OS-level mount isn't the meaningful state change;
  the filesystem being writable on the board is.

No code path changes -- this is purely message wording.
…ction

Add a clickable link in the write-protected save-failure dialog to
the canonical Adafruit Learn guide section that walks users through
disabling USB MSC in boot.py:

https://learn.adafruit.com/getting-started-with-web-workflow-using-the-code-editor/device-setup#disabling-usb-mass-storage-3125964

Implementation:

- web-file-transfer.js: split the help URL out of the prose hint into
  separate err.helpUrl + err.helpLabel fields. Keeps the hint string
  pure text and lets the dialog decide on markup.
- script.js: when rendering the dialog, append a real <a> tag with
  target=_blank rel=noopener noreferrer. MessageModal uses innerHTML
  so the anchor is clickable; opening in a new tab avoids nav-ing
  away from the editor and losing the unsaved buffer.

Falls back to the plain text hint when helpUrl is absent, so callers
that don't set it (or future writeProtected variants) still render
sensibly.
Add console.warn at two points:
- web-file-transfer.js _fetch: log method+path+status on any non-OK
  response, so we can see what the firmware actually sent back.
- script.js saveFileContents catch block: log the relevant fields on
  the caught error (name/message/status/method/path/writeProtected),
  so we can see whether the heuristic in _fetch tagged it correctly.

Temporary; remove once we've confirmed the field paths fire on real
hardware. Cheap (warn-level, only runs on actual errors).
…teProtected dialog

The previous PR handled HTTP 409/500 on PUT /fs/* by tagging the
ProtocolError with .writeProtected and showing an actionable dialog
in saveFileContents. But there are two earlier paths users hit FIRST
that were still using the old generic wording:

1. At connect, checkReadOnly() in script.js queries /fs/ for the
   writable flag. If false, it popped a one-line warning that said
   'Disable the USB drive' without telling users how, what 'eject
   may not be enough' means, or where to read more.

2. On save, writeFile() calls _checkWritable() FIRST. If the cached
   _writable flag is false, _checkWritable threw a plain Error (not
   a ProtocolError, no .writeProtected tag), so the catch block in
   saveFileContents never recognized it as the write-protected case
   and fell through to 'Saving file failed after multiple attempts.'

This commit:

- Factors the writeProtected ProtocolError shape into a new
  _writeProtectedError() helper on FileTransferClient, so both
  _checkWritable and the _fetch 409/500 handler return identical
  tagged errors (same .hint, .helpUrl, .helpLabel).
- _checkWritable now invalidates the cached _writable flag before
  re-checking. Means users who release the drive can save again
  without disconnecting/reconnecting the workflow.
- Updates checkReadOnly() in script.js to use the same wording +
  clickable Learn-guide link as the per-save dialog. Connect-time
  popup is now genuinely informative instead of cryptic.
- Removes the debug console.warn lines from the previous commit
  now that we've confirmed the path (the read-only branch was
  bypassing the tagged ProtocolError, not a bug in _fetch).

After this change, users see the same actionable message at all
three trip points (connect, pre-save check, server-side response).
The message names USB MSC, points at boot.py, warns that eject
alone is unreliable on macOS, and links to the Learn guide section
that walks them through disabling it.
Feedback from in-editor testing: the connect-time read-only warning
was too wordy for a popup that appears on every connect to a
read-only board, and the save-failure dialog was a wall of prose
that's hard to scan at the moment a user actually has a problem.

Connect-time warning is now a single line with a 'How to fix' link:

    Filesystem is read-only - you can browse files, but saving will
    fail until USB Mass Storage is released. [How to fix].

Save-failure dialog is now structured with real <p>, <strong>, <ul>,
<code>, <em>, and <a> markup (MessageModal renders via innerHTML):

    Could not save '/code.py'.

    The board's filesystem is locked, usually because CIRCUITPY is
    mounted on a computer over USB.

    To fix:
      - Disconnect the USB cable, or
      - Disable USB Mass Storage in boot.py, then reset the board.

    Note: ejecting the drive in your OS isn't always enough on its own.

    [Disabling USB Mass Storage (Adafruit Learn)]

    Your edits are still here - save again once the filesystem is
    writable.

CSS already handles anchor styling in #message; the <ul> uses a small
inline margin to keep bullet indent compact inside the modal.
The save-failure dialog now renders multi-section help text (headers,
list, link, several paragraphs), which exceeds the existing
max-height: 365px on .popup-modal.prompt. The message div was
overflowing past the modal box and pushing the OK button below the
visible area.

Restructure .popup-modal.prompt as a flex column:

- max-height: 80vh (more room when needed, still capped)
- Content area (#message) gets flex: 1 1 auto + overflow-y: auto, so
  long content scrolls inside the box.
- Buttons get flex: 0 0 auto, so they stay pinned at the bottom of
  the modal regardless of content size.
- The display: flex on .popup-modal.prompt.is--visible has three-class
  specificity, beating the existing .popup-modal.is--visible
  display: block rule without an !important.

Separately, cap the message dialog at max-width min(480px, 90vw) via
attribute selector so multi-paragraph prose wraps at a comfortable
reading measure (~60-65ch) rather than stretching across the full
viewport on wide monitors. Other prompt dialogs (unsaved, upload-type,
connection-type, ok-cancel) are short and unaffected.
Copy link
Copy Markdown
Collaborator

@makermelissa makermelissa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good after testing a number of times locally.

@makermelissa makermelissa merged commit 3df0be3 into circuitpython:main May 19, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cannot Save code.py Edits on Adafruit Qualia ESP32-S3 for TTL RGB-666 Displays

2 participants