diff --git a/app/e2e-mock-ipc.js b/app/e2e-mock-ipc.js index 0f1a12af..f66a1f66 100644 --- a/app/e2e-mock-ipc.js +++ b/app/e2e-mock-ipc.js @@ -583,6 +583,21 @@ function install({ ipcMain }) { ? 'mlx-community/parakeet-tdt-0.6b-v3' : 'istupakov/parakeet-tdt-0.6b-v3-onnx'; + // Mirrors main.js: a successful check that finds no update settles an earlier + // FAILED check, and it does so in the state the About tab rehydrates from — + // so the T1 spec asserts the contract (check clears it for good), not just a + // local setState. Seeded by STENOAI_E2E_SEED_UPDATE_ERROR. + let seededUpdateError = + process.env.STENOAI_E2E_SEED_UPDATE_ERROR === '1' + ? "Steno couldn't reach the update server. Check your connection — it will try again later." + : null; + + // Only the FIRST get-update-status is delayed, so the About tab's mount-time + // request is still in flight when the check that settles the error completes, + // and its stale reply lands last. Delaying every call would let the check's + // own re-read arrive after it and paper over the bug being guarded against. + let slowUpdateStatusCallsLeft = process.env.STENOAI_E2E_SLOW_UPDATE_STATUS === '1' ? 1 : 0; + const DEFAULTS = { 'get-app-version': { success: true, version: '0.0.0-e2e', name: 'Steno' }, // Read-only display poll for the About tab's "Check for Updates" button @@ -605,6 +620,10 @@ function install({ ipcMain }) { osUpdateEligible: false, }; } + // Up to date, and that settles a previously failed check — main.js + // clears the persisted error in exactly this branch, so the mock does + // too and the spec can assert it stays gone across a remount. + seededUpdateError = null; return { success: true, updateAvailable: false, @@ -624,13 +643,26 @@ function install({ ipcMain }) { // so the About tab's mount-time rehydration (settings-about.t1) can assert // the failure is restored on navigation, not just from the live one-shot // 'update-error' event. - 'get-update-status': () => ({ - success: true, - downloadedVersion: null, - downloadPercent: null, - downloadError: - process.env.STENOAI_E2E_SEED_UPDATE_ERROR === '1' ? 'network unreachable' : null, - }), + 'get-update-status': async () => { + // Answer with the state as of the REQUEST, not as of the reply. That is + // what main.js does (it reads pendingUpdateError when the handler runs), + // and it is what makes STENOAI_E2E_SLOW_UPDATE_STATUS a real race rather + // than a sleep. + const snapshot = seededUpdateError; + if (slowUpdateStatusCallsLeft > 0) { + slowUpdateStatusCallsLeft -= 1; + await new Promise((resolve) => setTimeout(resolve, 1000)); + } + return { + success: true, + downloadedVersion: null, + downloadPercent: null, + // main.js sends the About tab a finished sentence, not the raw + // electron-updater text (update-error-copy.js), so the seed mirrors that + // shape and the spec asserts what a user would actually read. + downloadError: snapshot, + }; + }, // Fires on first paint once signed in (Sidebar + RouteView gate the // Shared notes feature on it). Default to feature-enabled to match the // adapter's default and keep the org-lock spec's UI unchanged. A spec can diff --git a/app/main.js b/app/main.js index f8b9df6b..be7ba7e8 100644 --- a/app/main.js +++ b/app/main.js @@ -57,6 +57,7 @@ const { createTeardownRegistry } = require('./teardown'); const { registerFoldersIpc } = require('./folders-ipc'); const { registerSettingsIpc } = require('./settings-ipc'); const { isSafeToAutoInstall } = require('./update-idle-gate'); +const { describeUpdateError, updateErrorPhase } = require('./update-error-copy'); const { isOSUpdateEligible, MIN_MACOS_FOR_AUTOUPDATE } = require('./update-os-gate'); const processingLog = require('./processing-log'); const { isMeetingApp, allowsDeviceLevelFallback, isMacos14Plus } = require('./meeting-detect'); @@ -6133,6 +6134,15 @@ let pendingDownloadPercent = null; // a failed background update would show nothing. Cleared when a new cycle // starts (check / available / progress) or a download completes. let pendingUpdateError = null; +// Whether that error survives a later successful check. A dropped connection +// is disproved by one; a full disk, a missing update feed or a permission +// problem is not, and clearing those on an unrelated success would hide a +// condition that is still true. See update-error-copy.js. +let pendingUpdateErrorSticky = false; +// True between calling quitAndInstall and the app actually going away, so an +// error that fires in that window is reported as a failed install rather than +// as a failed check. +let installingUpdate = false; // ── Idle auto-install ── // True once an update has finished downloading and is staged for install. @@ -6219,6 +6229,8 @@ async function maybeAutoInstallWhenIdle() { // Bypass the mainWindow 'close' handler's preventDefault+hide (same reason as // the manual install-update path) so quitAndInstall actually quits + applies. isQuitting = true; + // From here on, an updater error is an INSTALL failure, not a failed check. + installingUpdate = true; // isSilent=true, isForceRunAfter=true — install without the wizard and // relaunch the app afterwards. autoUpdater.quitAndInstall(true, true); @@ -6278,11 +6290,31 @@ function setupAutoUpdater() { autoUpdater.autoDownload = true; autoUpdater.autoInstallOnAppQuit = true; + // Drop a stale non-sticky failure AND tell a mounted About tab, so the banner + // never outlives the state it describes. Sticky ones are left alone: a full + // disk, a permission problem or a missing feed is not disproved by starting or + // finding an update, only by bytes actually arriving (download-progress). + // Both callers need exactly this, and having it in one place is what stops the + // two from drifting apart — the earlier version cleared here but only emitted + // there, so the event never fired and About kept showing a settled failure + // next to a running download. + const clearNonStickyUpdateError = () => { + if (!pendingUpdateError || pendingUpdateErrorSticky) return; + pendingUpdateError = null; + if (mainWindow) mainWindow.webContents.send('update-error-cleared'); + }; + autoUpdater.on('checking-for-update', () => { sendDebugLog('Auto-updater: checking for updates...'); // Fresh cycle — clear any stale error from a previous failed check so a // rehydrating About tab doesn't show an error that's now being retried. - pendingUpdateError = null; + // Sticky ones stay: starting a check does not free disk space, grant write + // permission, or give this build an update feed, and those conditions are + // still true until something actually succeeds. They are cleared where that + // happens: a version found and fetched (update-available / download-progress + // / update-downloaded below), or a poll that comes back clean in the + // check-for-updates handler. + clearNonStickyUpdateError(); }); autoUpdater.on('update-available', (info) => { @@ -6290,7 +6322,16 @@ function setupAutoUpdater() { // Matches the renderer's own `setDownloadPercent((p) => p ?? 0)` — marks // a download as started before the first real progress tick arrives. if (pendingDownloadPercent === null) pendingDownloadPercent = 0; - pendingUpdateError = null; + // Only the non-sticky ones. This event says a version was FOUND, i.e. the + // feed was readable — it does not say a single byte was written, so it + // cannot disprove a full disk or a permission problem. Those clear one + // event later, on the first download-progress tick, which does prove it. + // (Clearing them here made the banner vanish and come straight back when + // the unchanged condition failed the download again.) + // + // Usually a no-op, since checking-for-update already ran for this cycle — + // it covers a failure that arrived between the two events. + clearNonStickyUpdateError(); if (mainWindow) { mainWindow.webContents.send('update-available', { version: info.version }); } @@ -6298,12 +6339,26 @@ function setupAutoUpdater() { autoUpdater.on('update-not-available', () => { sendDebugLog('Auto-updater: up to date'); + // A completed cycle with nothing pending — clear even a sticky failure. + // This is the updater itself reporting success, unlike the GitHub poll in + // the check-for-updates handler (our own request, which proves nothing + // about whether the updater can read its feed or write to disk). Nothing is + // waiting to download or install any more, so a banner about a past attempt + // is describing a state that no longer exists — and a condition that really + // is still broken (no update feed) errors before ever reaching this event. + pendingUpdateError = null; + pendingUpdateErrorSticky = false; + // Tell a mounted About tab too — it keeps its own copy, and the events it + // already listens to (available/progress/downloaded) don't fire on a clean + // cycle, so without this the banner would sit there until a remount. + if (mainWindow) mainWindow.webContents.send('update-error-cleared'); }); autoUpdater.on('download-progress', (progress) => { sendDebugLog(`Auto-updater: downloading ${Math.round(progress.percent)}%`); pendingDownloadPercent = Math.round(progress.percent); pendingUpdateError = null; + pendingUpdateErrorSticky = false; if (mainWindow) { mainWindow.webContents.send('update-download-progress', { percent: Math.round(progress.percent) }); } @@ -6313,6 +6368,7 @@ function setupAutoUpdater() { sendDebugLog(`Auto-updater: v${info.version} ready to install`); pendingDownloadPercent = null; pendingUpdateError = null; + pendingUpdateErrorSticky = false; pendingUpdateVersion = info.version; if (mainWindow) { mainWindow.webContents.send('update-downloaded', { version: info.version }); @@ -6328,6 +6384,21 @@ function setupAutoUpdater() { autoUpdater.on('error', (err) => { const msg = (err && err.message) || String(err); + // Which phase failed, captured BEFORE the state is cleared below. Note it + // does NOT consider a staged update: a periodic check can fail long after a + // download succeeded, and calling that a failed download is the lie this + // whole path exists to stop telling. + // + // Known limit: electron-updater's error event does not say which attempt it + // belongs to, so a periodic check that fails WHILE a download is genuinely + // running is attributed to the download. That case was already reported as + // a download failure before this module existed, so the heuristic is not a + // regression — closing it needs per-attempt correlation the library does + // not expose. + const phase = updateErrorPhase({ + downloadInFlight: pendingDownloadPercent !== null, + installing: installingUpdate, + }); // Clear any in-flight download state regardless of which branch below // fires — otherwise a failure after 'update-available' (which seeds // pendingDownloadPercent to 0) leaves get-update-status reporting a @@ -6343,12 +6414,19 @@ function setupAutoUpdater() { sendDebugLog('Auto-updater: no update feed published for this release yet — skipping.'); return; } + // The raw text stays here, where it's useful for diagnosis. What reaches + // the About tab is one sentence naming the phase that failed and whether + // the user has to do anything — see update-error-copy.js. sendDebugLog(`Auto-updater error: ${msg}`); + const { message: userMessage, sticky } = describeUpdateError(msg, { phase }); + // The install attempt (if any) is over — a later error is a fresh cycle. + installingUpdate = false; // Persist so a later About-tab mount can rehydrate it (the event below is // one-shot and only reaches an already-mounted listener). - pendingUpdateError = msg; + pendingUpdateError = userMessage; + pendingUpdateErrorSticky = sticky; if (mainWindow) { - mainWindow.webContents.send('update-error', { message: msg }); + mainWindow.webContents.send('update-error', { message: userMessage }); } }); @@ -6377,6 +6455,8 @@ ipcMain.on('install-update', () => { // quitAndInstall's window-close step actually quits the app. Without this // the app just minimises and Squirrel never gets to apply the update. isQuitting = true; + // From here on, an updater error is an INSTALL failure, not a failed check. + installingUpdate = true; autoUpdater.quitAndInstall(false, true); }); @@ -9438,6 +9518,19 @@ ipcMain.handle('check-for-updates', async () => { if (!IS_E2E && app.isPackaged && osEligible) { autoUpdater.checkForUpdates().catch(() => {}); } + // A check that just succeeded and found nothing settles an earlier FAILED + // check — otherwise a stale banner sits under a fresh "You're on the latest + // version", two contradictory answers to one question. Cleared here, in the + // state the About tab rehydrates from, so it stays cleared across a remount + // rather than only until the user switches tabs. + // + // Only non-sticky errors: this poll is our own GitHub request, so it says + // nothing about whether the updater can write to /Applications or whether + // this build has an update feed at all. Those conditions are still true and + // stay on screen (see update-error-copy.js). + if (result.success && !result.updateAvailable && !pendingUpdateErrorSticky) { + pendingUpdateError = null; + } // Surface eligibility so the About tab can explain why an "update available" // won't auto-install on an under-floor Mac, rather than offering a broken // Restart. (Display-only; the safety is the gated kick above.) diff --git a/app/package.json b/app/package.json index 46f3463d..f727351f 100644 --- a/app/package.json +++ b/app/package.json @@ -19,7 +19,7 @@ "typecheck:renderer": "tsc -p renderer/tsconfig.json --noEmit", "lint:renderer": "eslint --config renderer/eslint.config.mjs renderer/src", "format:renderer": "prettier --write renderer/src", - "test:unit": "node --test processing-log.test.js meeting-detect.test.js notes-file.test.js backend-stream.test.js ipc-contract.test.js shortcut-url.test.js setup-check-parse.test.js diagnostics-forward.test.js analytics-helpers.test.js live-snapshot-sweep.test.js backend-cli.test.js debug-log.test.js teardown.test.js folders-ipc.test.js settings-ipc.test.js regen-title-busy-guard.test.js update-idle-gate.test.js update-os-gate.test.js && vitest run", + "test:unit": "node --test processing-log.test.js meeting-detect.test.js notes-file.test.js backend-stream.test.js ipc-contract.test.js shortcut-url.test.js setup-check-parse.test.js diagnostics-forward.test.js analytics-helpers.test.js live-snapshot-sweep.test.js backend-cli.test.js debug-log.test.js teardown.test.js folders-ipc.test.js settings-ipc.test.js regen-title-busy-guard.test.js update-idle-gate.test.js update-os-gate.test.js update-error-copy.test.js && vitest run", "build": "npm run build:renderer && electron-builder", "pack:unsigned": "npm run build:renderer && electron-builder --dir --config electron-builder.ci.yml", "build-mac": "npm run build:renderer && electron-builder --mac", diff --git a/app/preload.js b/app/preload.js index 9abee106..b82127b5 100644 --- a/app/preload.js +++ b/app/preload.js @@ -379,6 +379,10 @@ const stenoai = { updateDownloadProgress: (cb) => subscribe('update-download-progress', cb), updateDownloaded: (cb) => subscribe('update-downloaded', cb), updateError: (cb) => subscribe('update-error', cb), + // Main cleared a failure the About tab may still be showing (a cycle that + // came back clean). Without this the banner would linger on a mounted tab + // until the user navigated away and back. + updateErrorCleared: (cb) => subscribe('update-error-cleared', cb), googleAuthChanged: (cb) => subscribe('google-auth-changed', cb), outlookAuthChanged: (cb) => subscribe('outlook-auth-changed', cb), shortcutStartRecording: (cb) => subscribe('shortcut-start-recording', cb), diff --git a/app/renderer/src/lib/ipc.ts b/app/renderer/src/lib/ipc.ts index b0bdbe1d..bd5e668d 100644 --- a/app/renderer/src/lib/ipc.ts +++ b/app/renderer/src/lib/ipc.ts @@ -1107,6 +1107,7 @@ export interface StenoaiBridge { updateDownloadProgress: Subscribe; updateDownloaded: Subscribe; updateError: Subscribe; + updateErrorCleared: Subscribe; googleAuthChanged: Subscribe<{ connected: boolean }>; outlookAuthChanged: Subscribe<{ connected: boolean }>; shortcutStartRecording: Subscribe; diff --git a/app/renderer/src/routes/settings/AboutTab.tsx b/app/renderer/src/routes/settings/AboutTab.tsx index 0d23377f..7f76167c 100644 --- a/app/renderer/src/routes/settings/AboutTab.tsx +++ b/app/renderer/src/routes/settings/AboutTab.tsx @@ -56,33 +56,66 @@ export function AboutTab() { // set this — whichever observes completion first wins, and once true a // later-arriving percent-only snapshot is ignored rather than merged. const settledRef = React.useRef(false); + // Separate from settledRef, which is about the progress bar. Every + // authoritative write to the banner bumps this: the live update-error and + // update-error-cleared events, and the re-read after a manual check. The + // mount-time getStatus() below captures it before its request and drops its + // own reply if anything has written since — main answers with the state as of + // the REQUEST, so a newer write has to win regardless of which reply lands + // last. Without it, a stale reply's `e ?? persisted` merge sees the cleared + // null and puts the settled failure straight back on screen. + const errorSeqRef = React.useRef(0); React.useEffect(() => { const offAvailable = ipc().on.updateAvailable(() => { // Confirms the real background updater has started fetching this // version — the progress bar below is about to start moving. + // + // Deliberately does NOT clear the banner: main keeps a sticky failure + // through this event, because finding a version proves the feed was + // readable and nothing else. Clearing here would hide a failure main + // still holds, and a remount would bring it straight back. settledRef.current = false; - setDownloadError(null); setDownloadPercent((p) => p ?? 0); }); const offProgress = ipc().on.updateDownloadProgress((evt) => { + // Bytes are actually arriving — that is what disproves a full disk or a + // permission problem, so this is where main clears both kinds and where + // the banner goes with them. + errorSeqRef.current += 1; + setDownloadError(null); setDownloadPercent(evt.percent); }); const offDownloaded = ipc().on.updateDownloaded((evt) => { settledRef.current = true; + // Main clears the failure here too (both kinds — the bytes are on disk). + // Usually download-progress got there first, but a resumed or cached + // transfer can make this the first event this tab sees, and then the + // banner would sit next to "Restart to Update". + errorSeqRef.current += 1; + setDownloadError(null); setDownloadPercent(null); setDownloadedVersion(evt.version); }); const offError = ipc().on.updateError((evt) => { settledRef.current = true; + errorSeqRef.current += 1; setDownloadPercent(null); setDownloadError(evt.message); }); + // Main settled an earlier failure (a cycle came back clean). Nothing else + // fires on that path, so without this the banner would stay on a tab that + // happens to be open. + const offErrorCleared = ipc().on.updateErrorCleared(() => { + errorSeqRef.current += 1; + setDownloadError(null); + }); return () => { offAvailable(); offProgress(); offDownloaded(); offError(); + offErrorCleared(); }; }, []); @@ -98,6 +131,9 @@ export function AboutTab() { // the progress bar alongside it. React.useEffect(() => { let cancelled = false; + // Captured before the request: anything that writes the banner while this + // is in flight makes the reply stale, whichever order they land in. + const seqAtRequest = errorSeqRef.current; void ipc() .updates.getStatus() .then((result) => { @@ -105,7 +141,7 @@ export function AboutTab() { if (result.downloadedVersion) { settledRef.current = true; setDownloadedVersion((v) => v ?? result.downloadedVersion); - } else if (result.downloadError) { + } else if (result.downloadError && errorSeqRef.current === seqAtRequest) { // A failed background update persists in main; restore it so // returning to About still shows the failure. Terminal for this // cycle (like a completed download), so mark settled — a stale @@ -141,6 +177,22 @@ export function AboutTab() { }); } else { setCheckState({ kind: 'up-to-date' }); + // A check that just succeeded settles an earlier failed one, so a stale + // banner doesn't sit under a fresh "You're on the latest version" as two + // contradictory answers to the same question. Main owns that decision — + // it knows whether the failure was disproved by this check or is still + // true (a full disk, a permission problem) — so re-read rather than + // clearing locally, and stay in sync with what a remount would show. + // Same staleness rule as the mount effect, in the other direction: if a + // live update-error lands while this read is in flight, that error is + // newer than the state main answered with, and overwriting it would + // wipe a failure that just happened. + const seqAtRequest = errorSeqRef.current; + const status = await ipc().updates.getStatus(); + if (status.success && errorSeqRef.current === seqAtRequest) { + errorSeqRef.current += 1; + setDownloadError(status.downloadError); + } } } catch (e) { setCheckState({ @@ -238,9 +290,14 @@ export function AboutTab() { )} + {/* main.js sends a finished sentence that already names what failed + (check vs download) — see update-error-copy.js. Rendering it verbatim + is deliberate: the old "Update download failed: {raw}" prefix claimed + a download even when the check never got that far, and pasted + net:: codes and bundle paths into the UI. */} {downloadError && (
- Update download failed: {downloadError} + {downloadError}
)} diff --git a/app/update-error-copy.js b/app/update-error-copy.js new file mode 100644 index 00000000..5a9f0b96 --- /dev/null +++ b/app/update-error-copy.js @@ -0,0 +1,202 @@ +'use strict'; + +/** + * User-facing copy for auto-updater failures. + * + * electron-updater's `error` event carries developer text: `net::` codes, HTTP + * status lines, absolute paths inside the app bundle. That string used to go + * straight into the About tab, which produced messages like + * "Update download failed: ENOENT: no such file or directory, open + * '/Applications/Steno.app/Contents/Resources/app-update.yml'" — a stack-trace + * fragment in an interface that otherwise speaks in sentences, and one that + * says "download" even when nothing was ever downloaded. + * + * So main.js maps the error to prose before it reaches the renderer, and keeps + * the raw text in the debug log where it is actually useful. Pure and + * unit-tested, mirroring update-idle-gate.js / update-os-gate.js. + * + * "Prose", not literally one sentence: rule 2 below often needs a second short + * clause to say what happens next, and splitting that off reads better than + * cramming it in. What is guaranteed is that no developer text survives — no + * errno, no net:: code, no path. + * + * Three rules the copy follows: + * - Name the phase that actually failed. A check that never got off the ground + * is not a failed download, and a failed install is neither. + * - Say whether the user has to do anything. Most of these resolve themselves + * on the next scheduled check, and saying so is the difference between a + * warning and a chore. + * - Mark whether a later successful check DISPROVES the failure. A dropped + * connection does; a full disk, a missing update feed or a permission + * problem does not, and clearing those on an unrelated success would hide a + * condition that is still true (`sticky`). + */ + +const PHASE_CHECK = 'check'; +const PHASE_DOWNLOAD = 'download'; +const PHASE_INSTALL = 'install'; + +// Permission failures on the install step — typically an app installed +// somewhere the user cannot write, or a locked bundle. FIRST, because an +// EACCES on app-update.yml is a permission problem, not a missing feed. +const PERMISSION_RE = /(EACCES|EPERM|permission denied|not permitted|read-?only file system|EROFS)/i; + +// Integrity failures. Deliberately distinct from a transfer error: the bytes +// arrived, they just could not be trusted, and nothing was installed. +const INTEGRITY_RE = /(sha512|checksum|signature|not signed|integrity)/i; + +const DISK_RE = /(ENOSPC|no space left|not enough space)/i; + +// The bundle was built without an update feed (a `--dir` pack, or a build whose +// publish config was stripped). Users never see this; developers testing a local +// build do, and the honest answer is that this build cannot update at all. +// Requires the "missing" part too, so an unrelated error that merely names the +// file doesn't land here. +const NOT_CONFIGURED_RE = + /(ENOENT|no such file|cannot find|not found)[^]*app-update\.yml|app-update\.yml[^]*(ENOENT|no such file|cannot find|not found)/i; + +// The server answered, just not usefully. Checked BEFORE the transport branch: +// Chromium reports these as net::ERR_HTTP_RESPONSE_CODE_FAILURE, which would +// otherwise read as "check your connection" for what is a server-side problem. +const HTTP_STATUS_RE = + /(ERR_HTTP_RESPONSE_CODE_FAILURE|HTTP (4\d\d|5\d\d)|status(?: code)?:? (4\d\d|5\d\d)|\b(403|404|500|502|503)\b)/i; + +// Transport-level failures: no route to the update server, or the connection +// died mid-transfer. Node errno strings and Chromium net:: codes, since +// electron-updater surfaces both depending on which layer failed. Deliberately +// no bare "network" keyword — it matches file paths. +const NETWORK_RE = + /(ENOTFOUND|ECONNREFUSED|ECONNRESET|ECONNABORTED|ETIMEDOUT|EAI_AGAIN|ENETDOWN|ENETUNREACH|EHOSTUNREACH|net::ERR_|socket hang up|network (is )?(unreachable|error|timed out)|timed? ?out)/i; + +/** + * Which phase an updater error belongs to. main.js owns the live state; this + * only decides, so the decision is unit-testable. + * + * `installing` wins over `downloadInFlight`: applying a staged update is the + * later, more specific phase. Note that a staged update on its own means + * NOTHING — a periodic check can fail long after a download succeeded, and + * calling that a failed download is exactly the bug this module fixes. + * + * @param {Object} o + * @param {boolean} [o.downloadInFlight] a transfer was actually running + * @param {boolean} [o.installing] quitAndInstall had been called + * @returns {'check'|'download'|'install'} + */ +function updateErrorPhase({ downloadInFlight = false, installing = false } = {}) { + if (installing) return PHASE_INSTALL; + if (downloadInFlight) return PHASE_DOWNLOAD; + return PHASE_CHECK; +} + +/** + * @param {unknown} rawMessage the electron-updater error message + * @param {Object} [o] + * @param {'check'|'download'|'install'} [o.phase] + * @param {string} [o.platform] defaults to the running platform; injectable so + * the platform-specific copy is testable on either OS + * @returns {{ message: string, sticky: boolean }} one sentence for the About + * tab, plus whether a later successful check leaves it standing + */ +function describeUpdateError( + rawMessage, + { phase = PHASE_CHECK, platform = process.platform } = {} +) { + const msg = typeof rawMessage === 'string' ? rawMessage : String(rawMessage || ''); + + if (PERMISSION_RE.test(msg)) { + // Two things vary here, and getting either wrong makes the sentence a lie. + // + // The phase: a permission failure while fetching is about writing the + // downloaded file into the cache, not about where the app lives, so the + // Applications-folder hint would send the user somewhere that cannot help. + // Reading the feed (check) and swapping the bundle (install) are both about + // the app's own location, so there the hint is the actual fix. + // + // The platform: there is no Applications folder on Windows. CLAUDE.md's + // cross-platform rule applies to copy as much as to code — a macOS-only + // instruction must not reach a Windows user. + // Naming the wrong phase is the exact failure this module exists to stop, + // so all three get their own verb: nothing is installed when a check or a + // download fails. + const what = + phase === PHASE_DOWNLOAD + ? 'save the update' + : phase === PHASE_CHECK + ? 'check for updates' + : 'install the update'; + // The hint is about where the app itself lives, which is what a check or an + // install trips over. A download writes to the cache, so it would be advice + // that cannot help. + const hint = + platform === 'darwin' && phase !== PHASE_DOWNLOAD + ? ' Try moving Steno to your Applications folder.' + : ''; + return { + message: `Steno doesn't have permission to ${what}.${hint}`, + sticky: true, + }; + } + if (INTEGRITY_RE.test(msg)) { + return { + message: "The update couldn't be verified, so it wasn't installed. Steno will try again later.", + sticky: false, + }; + } + if (DISK_RE.test(msg)) { + return { + message: 'There is not enough disk space to download the update.', + sticky: true, + }; + } + if (NOT_CONFIGURED_RE.test(msg)) { + return { message: 'This build is not set up for automatic updates.', sticky: true }; + } + if (HTTP_STATUS_RE.test(msg)) { + return { + message: "The update server didn't respond as expected. Steno will try again later.", + sticky: false, + }; + } + if (NETWORK_RE.test(msg)) { + // Three phases, three answers. Lumping install in with download was the + // same mislabelling this module exists to stop, one level down: applying a + // staged update touches no network, so a transport error surfacing then is + // not an interrupted transfer and telling the user to wait for a retry of + // one would be wrong. + if (phase === PHASE_INSTALL) { + return { + message: "The update couldn't be installed. Restart Steno to try again.", + sticky: true, + }; + } + return { + message: + phase === PHASE_CHECK + ? "Steno couldn't reach the update server. Check your connection — it will try again later." + : 'The update download was interrupted. Steno will try again later.', + sticky: false, + }; + } + + if (phase === PHASE_INSTALL) { + return { + message: "The update couldn't be installed. Restart Steno to try again.", + sticky: true, + }; + } + return { + message: + phase === PHASE_DOWNLOAD + ? "The update didn't finish downloading. Steno will try again later." + : "Steno couldn't check for updates. It will try again later.", + sticky: false, + }; +} + +module.exports = { + describeUpdateError, + updateErrorPhase, + PHASE_CHECK, + PHASE_DOWNLOAD, + PHASE_INSTALL, +}; diff --git a/app/update-error-copy.test.js b/app/update-error-copy.test.js new file mode 100644 index 00000000..f82250f7 --- /dev/null +++ b/app/update-error-copy.test.js @@ -0,0 +1,220 @@ +'use strict'; + +const { test } = require('node:test'); +const assert = require('node:assert'); + +const { + describeUpdateError, + updateErrorPhase, + PHASE_CHECK, + PHASE_DOWNLOAD, + PHASE_INSTALL, +} = require('./update-error-copy'); + +const check = (msg) => describeUpdateError(msg, { phase: PHASE_CHECK }); +const download = (msg) => describeUpdateError(msg, { phase: PHASE_DOWNLOAD }); +const install = (msg) => describeUpdateError(msg, { phase: PHASE_INSTALL }); + +// ── phase decision ───────────────────────────────────────────────────────── +// The bug this replaces: "a staged update exists" was treated as "a download +// failed", so every later check error was mislabelled. + +test('a staged update does not make a later check error a download error', () => { + assert.strictEqual(updateErrorPhase({ downloadInFlight: false }), PHASE_CHECK); +}); + +test('a transfer in flight is a download error', () => { + assert.strictEqual(updateErrorPhase({ downloadInFlight: true }), PHASE_DOWNLOAD); +}); + +test('installing wins over everything else', () => { + assert.strictEqual(updateErrorPhase({ downloadInFlight: true, installing: true }), PHASE_INSTALL); + assert.strictEqual(updateErrorPhase({ installing: true }), PHASE_INSTALL); +}); + +test('no state at all reads as a check', () => { + assert.strictEqual(updateErrorPhase(), PHASE_CHECK); + assert.strictEqual(updateErrorPhase({}), PHASE_CHECK); +}); + +// ── nothing developer-shaped reaches the user ────────────────────────────── + +const DEV_SHAPED = /net::|ENOENT|ECONN|EACCES|ENOSPC|sha512|\/|\bat \b/; + +const RAW_SAMPLES = [ + "ENOENT: no such file or directory, open '/Applications/Steno.app/Contents/Resources/app-update.yml'", + 'net::ERR_INTERNET_DISCONNECTED', + 'getaddrinfo ENOTFOUND github.com', + 'sha512 checksum mismatch, expected AAA, got BBB', + 'ENOSPC: no space left on device, write', + "EACCES: permission denied, rename '/Applications/Steno.app'", + 'Error: socket hang up', + 'net::ERR_HTTP_RESPONSE_CODE_FAILURE (404)', + 'HTTP 503', + '', + null, + undefined, +]; + +test('never leaks developer-shaped text into the UI', () => { + for (const raw of RAW_SAMPLES) { + for (const out of [check(raw), download(raw), install(raw)]) { + assert.doesNotMatch(out.message, DEV_SHAPED, `leaked from: ${raw}`); + assert.match(out.message, /\.$/, `not a sentence: ${out.message}`); + assert.strictEqual(typeof out.sticky, 'boolean'); + } + } +}); + +// ── the phase must be named honestly ─────────────────────────────────────── + +test('a failed check is not reported as a failed download', () => { + assert.match(check('net::ERR_INTERNET_DISCONNECTED').message, /reach the update server/i); + assert.doesNotMatch(check('net::ERR_INTERNET_DISCONNECTED').message, /download/i); + assert.doesNotMatch(check('something nobody predicted').message, /download/i); +}); + +test('a failed download says so', () => { + assert.match(download('net::ERR_CONNECTION_RESET').message, /download/i); + assert.match(download('something nobody predicted').message, /download/i); +}); + +test('a failed install says so, and does not blame the download', () => { + const out = install('something nobody predicted'); + assert.match(out.message, /install/i); + assert.doesNotMatch(out.message, /download/i); + assert.strictEqual(out.sticky, true, 'a failed install is not disproved by a later check'); +}); + +// ── branch precedence (each was a plausible mis-routing) ─────────────────── + +test('a permission error on the feed file is a permission problem, not a missing feed', () => { + const out = check("EACCES: permission denied, open '/x/Contents/Resources/app-update.yml'"); + assert.match(out.message, /permission/i); + assert.strictEqual(out.sticky, true); +}); + +test('an HTTP status failure is not reported as a connection problem', () => { + for (const raw of ['net::ERR_HTTP_RESPONSE_CODE_FAILURE (404)', 'HTTP 503', 'status code: 500']) { + const out = check(raw); + assert.match(out.message, /update server didn't respond/i, `misrouted: ${raw}`); + assert.doesNotMatch(out.message, /connection/i, `misrouted: ${raw}`); + } +}); + +test('a path that merely contains the word network is not a connection problem', () => { + const out = check("cannot read '/Users/x/Library/Application Support/network-cache/blob'"); + assert.doesNotMatch(out.message, /connection|reach the update server/i); + assert.match(out.message, /couldn't check for updates/i); +}); + +test('a missing update feed is reported as an unconfigured build, whatever the phase', () => { + const out = check("ENOENT: no such file or directory, open '/x/Resources/app-update.yml'"); + assert.match(out.message, /not set up for automatic updates/i); + assert.strictEqual(out.sticky, true); + // Phase-independent: this build can't update either way. + assert.strictEqual(download('app-update.yml not found').message, out.message); +}); + +test('an integrity failure says nothing was installed', () => { + assert.match(download('sha512 mismatch').message, /verified/i); + assert.match(download("code signature didn't match").message, /verified/i); +}); + +test('out of disk space is actionable and does not promise a retry', () => { + const out = download('ENOSPC: no space left on device'); + assert.match(out.message, /disk space/i); + assert.doesNotMatch(out.message, /try again/i); + assert.strictEqual(out.sticky, true); +}); + +// ── sticky: which failures survive a later successful check ──────────────── + +test('transient failures are cleared by a successful check', () => { + for (const out of [ + check('net::ERR_INTERNET_DISCONNECTED'), + check('HTTP 503'), + check('something nobody predicted'), + download('sha512 mismatch'), + ]) { + assert.strictEqual(out.sticky, false, out.message); + } +}); + +test('conditions a check cannot disprove stay on screen', () => { + for (const out of [ + check("ENOENT ... app-update.yml"), + download('ENOSPC: no space left on device'), + install("EPERM: operation not permitted"), + install('something nobody predicted'), + ]) { + assert.strictEqual(out.sticky, true, out.message); + } +}); + +test('a non-string error does not throw', () => { + assert.doesNotThrow(() => describeUpdateError({ code: 42 })); + assert.doesNotThrow(() => describeUpdateError()); +}); + +// ── permission copy is phase- and platform-specific ──────────────────────── +// One sentence used to cover every permission failure: it named the install +// step and told the user to move the app to /Applications. Wrong for a failure +// while fetching (nothing is being installed), and impossible to act on for a +// Windows user (no such folder). + +const PERM = "EACCES: permission denied, rename '/x/Steno.app'"; + +test('a permission failure while downloading is not reported as an install problem', () => { + const out = describeUpdateError(PERM, { phase: PHASE_DOWNLOAD, platform: 'darwin' }); + assert.match(out.message, /save the update/i); + assert.doesNotMatch(out.message, /install/i); + assert.doesNotMatch(out.message, /Applications folder/i); + assert.strictEqual(out.sticky, true); +}); + +test('the Applications-folder hint is macOS-only', () => { + for (const phase of [PHASE_CHECK, PHASE_INSTALL]) { + const mac = describeUpdateError(PERM, { phase, platform: 'darwin' }); + assert.match(mac.message, /Applications folder/i, `missing on darwin in ${phase}`); + + const win = describeUpdateError(PERM, { phase, platform: 'win32' }); + assert.match(win.message, /permission/i, `wrong copy on win32 in ${phase}`); + assert.doesNotMatch(win.message, /Applications folder/i, `macOS hint leaked to win32 in ${phase}`); + } +}); + +test('a permission failure during a check does not claim an install was attempted', () => { + for (const platform of ['darwin', 'win32']) { + const out = describeUpdateError(PERM, { phase: PHASE_CHECK, platform }); + assert.match(out.message, /check for updates/i, `wrong verb on ${platform}`); + assert.doesNotMatch(out.message, /install/i, `claims an install on ${platform}`); + } +}); + +test('permission copy stays sticky on every platform and phase', () => { + for (const platform of ['darwin', 'win32']) { + for (const phase of [PHASE_CHECK, PHASE_DOWNLOAD, PHASE_INSTALL]) { + assert.strictEqual( + describeUpdateError(PERM, { phase, platform }).sticky, + true, + `${platform}/${phase} lost sticky — a permission problem is not disproved by a later check`, + ); + } + } +}); + +test('a transport error during install is not called an interrupted download', () => { + const out = describeUpdateError('net::ERR_CONNECTION_RESET', { phase: PHASE_INSTALL }); + assert.doesNotMatch(out.message, /download/i); + assert.match(out.message, /couldn't be installed/i); + // An install that failed stays until something proves otherwise — a later + // clean check says nothing about whether the swap can be applied. + assert.strictEqual(out.sticky, true); +}); + +test('the same transport error still reads as a transfer problem while downloading', () => { + const out = describeUpdateError('net::ERR_CONNECTION_RESET', { phase: PHASE_DOWNLOAD }); + assert.match(out.message, /download was interrupted/i); + assert.strictEqual(out.sticky, false); +}); diff --git a/e2e/specs/settings-about.t1.spec.ts b/e2e/specs/settings-about.t1.spec.ts index 60ac4270..7bf855b8 100644 --- a/e2e/specs/settings-about.t1.spec.ts +++ b/e2e/specs/settings-about.t1.spec.ts @@ -72,5 +72,83 @@ test('About tab rehydrates a persisted failed background update on mount', async const aboutSection = page.locator('[data-settings-tab="about"]'); await expect(aboutSection).toBeVisible(); - await expect(aboutSection.getByText(/Update download failed: network unreachable/)).toBeVisible(); + await expect( + aboutSection.getByText(/Steno couldn't reach the update server/), + ).toBeVisible(); +}); + +test('a successful check clears a stale update failure', async ({ launchApp }) => { + // The two states come from different sources — the button from the GitHub + // poll, the banner from the background updater — so a failed cycle used to + // leave "Update download failed…" sitting under a fresh "You're on the latest + // version": two contradictory answers to the same question. A check that just + // succeeded settles the earlier failure. + const { page } = await launchApp({ + mockIpc: true, + env: { STENOAI_E2E_SEED_UPDATE_ERROR: '1' }, + }); + + await page.evaluate(() => { + window.location.hash = '#/settings?tab=about'; + }); + + const aboutSection = page.locator('[data-settings-tab="about"]'); + const failure = aboutSection.getByText(/Steno couldn't reach the update server/); + await expect(failure).toBeVisible(); + + await aboutSection.getByRole('button', { name: 'Check for Updates' }).click(); + await expect( + aboutSection.getByRole('button', { name: "You're on the latest version" }), + ).toBeVisible(); + await expect(failure).toHaveCount(0); + + // And it stays cleared: main owns the persisted error, so leaving About and + // coming back must not rehydrate the banner the check just settled. A + // renderer-local clear would fail here. + await page.evaluate(() => { + window.location.hash = '#/settings?tab=general'; + }); + await expect(page.locator('[data-settings-tab="about"]')).toHaveCount(0); + await page.evaluate(() => { + window.location.hash = '#/settings?tab=about'; + }); + await expect(aboutSection).toBeVisible(); + await expect(aboutSection.getByText(/couldn't reach the update server/)).toHaveCount(0); +}); + +test('a stale status reply cannot restore a failure the check just settled', async ({ + launchApp, +}) => { + // The banner has two sources that can disagree: the mount-time getStatus() + // and the re-read after a manual check. Main answers each with the state as + // of the REQUEST, so if the mount request is slow enough to land after the + // check settled the failure, its older answer would put the banner back — + // under a fresh "You're on the latest version", which is exactly the + // contradiction this whole path removes. STENOAI_E2E_SLOW_UPDATE_STATUS + // delays only the first status call, so the stale reply is guaranteed to + // arrive last instead of racing. + const { page } = await launchApp({ + mockIpc: true, + env: { STENOAI_E2E_SEED_UPDATE_ERROR: '1', STENOAI_E2E_SLOW_UPDATE_STATUS: '1' }, + }); + + await page.evaluate(() => { + window.location.hash = '#/settings?tab=about'; + }); + + const aboutSection = page.locator('[data-settings-tab="about"]'); + await expect(aboutSection).toBeVisible(); + + // Click while the mount request is still in flight — the banner has not even + // appeared yet, which is the point. + await aboutSection.getByRole('button', { name: 'Check for Updates' }).click(); + await expect( + aboutSection.getByRole('button', { name: "You're on the latest version" }), + ).toBeVisible(); + + // Outlast the delayed reply, then assert it changed nothing. + await page.waitForTimeout(1500); + await expect( + aboutSection.getByText(/Steno couldn't reach the update server/), + ).toHaveCount(0); });