diff --git a/js/common/web-file-transfer.js b/js/common/web-file-transfer.js index ab2a010..e0d5df1 100644 --- a/js/common/web-file-transfer.js +++ b/js/common/web-file-transfer.js @@ -50,9 +50,36 @@ class FileTransferClient { } } + // Build a ProtocolError-shaped error that callers can recognize as + // "the device's filesystem is currently held by something else" + // (typically USB MSC). Tagged identically to the runtime PUT 409/500 + // path so `saveFileContents()` can show the same actionable dialog + // whether the check trips on the cached writable flag or on the + // actual response from the device. + _writeProtectedError() { + const err = new ProtocolError("File System is Read Only."); + err.status = 409; + err.writeProtected = true; + err.hint = "The board's filesystem is currently locked, " + + "usually because CIRCUITPY is mounted on a " + + "computer over USB. Disconnect the USB cable, " + + "or disable USB Mass Storage in boot.py, then " + + "reset the board and try saving again. " + + "(Ejecting the drive in your OS may not be " + + "enough on its own.)"; + err.helpUrl = "https://learn.adafruit.com/getting-started-with-web-workflow-using-the-code-editor/device-setup#disabling-usb-mass-storage-3125964"; + err.helpLabel = "Disabling USB Mass Storage (Adafruit Learn)"; + return err; + } + async _checkWritable() { + // Force a re-read of the writable flag so the user can recover + // without disconnecting: if they just released the drive (or + // disabled USB MSC and reset), the next save attempt should + // succeed, not bounce off a stale `false` cache. + this._writable = null; if (await this.readOnly()) { - throw new Error("File System is Read Only. Try disabling the USB Drive."); + throw this._writeProtectedError(); } } @@ -72,7 +99,8 @@ class FileTransferClient { options.headers['Content-Type'] = "application/octet-stream"; } - await this._fetch(`/fs${path}`, options); + const response = await this._fetch(`/fs${path}`, options); + return response.ok; } // Makes the directory and any missing parents @@ -120,7 +148,32 @@ class FileTransferClient { } if (!response.ok) { - throw new ProtocolError(response.statusText); + // Attach the status code + a friendly hint when we recognize + // the failure mode, so callers can branch on it (e.g. show an + // actionable message and skip retries that won't help). + const err = new ProtocolError(response.statusText || `HTTP ${response.status}`); + err.status = response.status; + err.method = (fetchOptions.method || "GET").toUpperCase(); + err.path = location; + // /fs/ PUT against a write-protected filesystem currently returns + // 500 on shipped CircuitPython firmware. A fix is pending to + // return 409 Conflict (matching DELETE / MOVE / mkdir-PUT in + // the same file). Treat both the same way until enough users + // are on the patched firmware that 500 can be left generic. + const isFsWrite = err.method === "PUT" && + typeof location === "string" && + location.startsWith("/fs/"); + if (isFsWrite && (response.status === 409 || response.status === 500)) { + // Reuse the same wording/hint as the cached-flag + // _checkWritable() path, so users see one consistent + // message regardless of which layer caught the lock. + const wp = this._writeProtectedError(); + err.writeProtected = true; + err.hint = wp.hint; + err.helpUrl = wp.helpUrl; + err.helpLabel = wp.helpLabel; + } + throw err; } return response; diff --git a/js/script.js b/js/script.js index 6b35335..fa3d7ab 100644 --- a/js/script.js +++ b/js/script.js @@ -229,8 +229,11 @@ async function newFile() { async function saveRunFile() { if (await checkConnected()) { + // workflow.saveFile() now propagates the real save result -- only + // soft-restart / re-import once the PUT actually succeeded. Otherwise + // we would reboot the board running the old code.py while the editor + // still had the unsaved edits (issue #460). if (await workflow.saveFile()) { - setSaved(true); await workflow.runCurrentCode(); } } @@ -328,7 +331,14 @@ async function checkReadOnly() { await showMessage(readOnly); return false; } else if (readOnly) { - await showMessage("Warning: File System is in read only mode. Disable the USB drive to allow write access."); + // Concise connect-time notice that the filesystem is read-only, + // with a link to the Learn guide for users who want the fix now. + const learnUrl = "https://learn.adafruit.com/getting-started-with-web-workflow-using-the-code-editor/device-setup#disabling-usb-mass-storage-3125964"; + await showMessage( + "Filesystem is read-only — you can browse files, but saving " + + "will fail until USB Mass Storage is released. " + + `How to fix.` + ); } return true; } @@ -535,47 +545,110 @@ async function loadEditor() { } var editor; -var currentTimeout = null; -var saveRetryCount = 0; const MAX_SAVE_RETRIES = 3; +const SAVE_RETRY_DELAY_MS = 2000; +let saveInFlight = false; -// Save the File Contents and update the UI +function sleep(ms) { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +// Save the File Contents and update the UI. Returns true on success, false +// on final failure (after all retries). Retries inline so callers (Save+Run, +// hotkeys, dialogs) can actually await the outcome -- previously this used +// a fire-and-forget setTimeout, which let Save+Run soft-restart the board +// before the PUT had succeeded (issue #460). async function saveFileContents(path) { - // If this is a different file, we write everything - if (path !== workflow.currentFilename) { - unchanged = 0; - } - let doc = editor.state.doc; - let offset = 0; - let contents = doc.sliceString(0); - if (workflow.partialWrites) { - offset = unchanged; - console.log("sync starting at", unchanged, "to", editor.state.doc.length); + if (saveInFlight) { + // Re-entrant save (e.g. user mashing Ctrl-S / Save+Run). The first + // call will report success/failure; the second would race the same + // bytes onto the wire and confuse partialWrites bookkeeping. + console.log("saveFileContents: already in flight, ignoring re-entry"); + return false; } - let oldUnchanged = unchanged; - unchanged = doc.length; + saveInFlight = true; try { - if (await workflow.writeFile(path, contents, offset)) { - setFilename(workflow.currentFilename); - setSaved(true); - saveRetryCount = 0; - } else { - await showMessage(`Saving file '${workflow.currentFilename}' failed.`); - } - } catch (e) { - console.error("write failed", e, e.stack); - unchanged = Math.min(oldUnchanged, unchanged); - if (currentTimeout != null) { - clearTimeout(currentTimeout); + // If this is a different file, we write everything + if (path !== workflow.currentFilename) { + unchanged = 0; } - saveRetryCount++; - if (saveRetryCount < MAX_SAVE_RETRIES) { - console.log(`Save retry ${saveRetryCount} of ${MAX_SAVE_RETRIES}...`); - currentTimeout = setTimeout(() => saveFileContents(path), 2000); - } else { - saveRetryCount = 0; - await showMessage(`Saving file '${workflow.currentFilename}' failed after multiple attempts. Check your connection and try again.`); + let doc = editor.state.doc; + let contents = doc.sliceString(0); + let baseUnchanged = unchanged; + let docLengthAtStart = doc.length; + + for (let attempt = 1; attempt <= MAX_SAVE_RETRIES; attempt++) { + // Recompute offset each attempt -- if onTextChange fired between + // retries, `unchanged` may have shrunk and we need to resend more. + let offset = 0; + if (workflow.partialWrites) { + offset = Math.min(baseUnchanged, unchanged); + console.log("sync starting at", offset, "to", editor.state.doc.length); + } + // Optimistically mark the bytes-being-sent as unchanged. If the + // write throws we'll roll back to baseUnchanged for the next try. + unchanged = docLengthAtStart; + try { + if (await workflow.writeFile(path, contents, offset)) { + setFilename(workflow.currentFilename); + setSaved(true); + return true; + } + // writeFile returned a falsy value without throwing -- treat + // as a soft failure and surface a message immediately. + await showMessage(`Saving file '${workflow.currentFilename}' failed.`); + setSaved(false); + return false; + } catch (e) { + console.error(`write failed (attempt ${attempt} of ${MAX_SAVE_RETRIES})`, e, e.stack); + unchanged = Math.min(baseUnchanged, unchanged); + // If the device cleanly told us the filesystem is held by + // someone else (most commonly USB-MSC: the host has + // CIRCUITPY mounted), retrying won't help -- surface an + // actionable hint immediately and bail. Older CircuitPython + // firmware returns 500 for this case, newer firmware + // returns 409 Conflict; web-file-transfer.js tags both + // with `writeProtected` so we can treat them the same way. + if (e && e.writeProtected) { + setSaved(false); + const learnUrl = e.helpUrl || "https://learn.adafruit.com/getting-started-with-web-workflow-using-the-code-editor/device-setup#disabling-usb-mass-storage-3125964"; + const learnLabel = e.helpLabel || "Disabling USB Mass Storage (Adafruit Learn)"; + // MessageModal renders via innerHTML, so real markup + // (sections, list, link) is fine. Sections separate the + // 'what happened', 'why', and 'how to fix' so users can + // scan instead of parsing a wall of prose. + await showMessage( + `

Could not save '${workflow.currentFilename}'.

` + + `

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

` + + `

To fix:

` + + `` + + `

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

` + + `

${learnLabel}

` + + `

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

` + ); + return false; + } + if (attempt < MAX_SAVE_RETRIES) { + await sleep(SAVE_RETRY_DELAY_MS); + // Bail out if the user disconnected mid-retry. + if (!workflow || !workflow.connectionStatus()) { + setSaved(false); + return false; + } + } + } } + // All retries exhausted. Leave the editor marked dirty so the user + // knows the file on the board is still stale. + setSaved(false); + await showMessage(`Saving file '${workflow.currentFilename}' failed after multiple attempts. Check your connection and try again.`); + return false; + } finally { + saveInFlight = false; } } @@ -611,19 +684,13 @@ async function onTextChange(update) { unchanged = 0; } - if (currentTimeout != null) { - clearTimeout(currentTimeout); - } - setSaved(false); } function disconnectCallback() { - if (currentTimeout != null) { - clearTimeout(currentTimeout); - currentTimeout = null; - } - saveRetryCount = 0; + // saveInFlight is intentionally not forced here -- the in-flight + // saveFileContents loop checks connectionStatus() between retries and + // exits cleanly on its own, then clears the flag in its finally block. updateUIConnected(false); } diff --git a/js/workflows/workflow.js b/js/workflows/workflow.js index b472640..1e0ee7d 100644 --- a/js/workflows/workflow.js +++ b/js/workflows/workflow.js @@ -442,8 +442,14 @@ except ImportError: // canceled or rejected) is treated the same as null and does not get // forwarded to writeFile, where it would crash in _splitPath. See #327. if (path != null) { - await this._saveFileContents(path); - return true; + // Propagate the actual save result so Save+Run and other callers + // can avoid taking follow-up actions (soft-restart, import) when + // the underlying PUT failed. _saveFileContents returns false on + // exhausted retries; treating only an explicit `false` as failure + // keeps backwards compatibility with older saveFileFunc callbacks + // that returned undefined on success (issue #460). + const result = await this._saveFileContents(path); + return result !== false; } return false; } diff --git a/sass/layout/_layout.scss b/sass/layout/_layout.scss index fa9dc22..2924be8 100644 --- a/sass/layout/_layout.scss +++ b/sass/layout/_layout.scss @@ -318,7 +318,32 @@ display: none; &.prompt { - max-height: 365px; + max-height: 80vh; + // Flex column so the buttons stay pinned to the bottom and the + // content area can scroll when a dialog has rich/multi-section + // markup that overflows the modal height. Three-class selector + // beats `.popup-modal.is--visible` so the `display: flex` + // wins on specificity. + &.is--visible { + display: flex; + flex-direction: column; + } + #message { + overflow-y: auto; + flex: 1 1 auto; + min-height: 0; + } + .buttons { + flex: 0 0 auto; + } + } + + // The message dialog often shows multi-section explanatory prose + // (e.g. the save-failure / read-only-filesystem help). Cap the + // width so paragraphs wrap at a comfortable reading measure rather + // than stretching the full viewport on a wide monitor. + &[data-popup-modal="message"] { + max-width: min(480px, 90vw); } &.shadow {