diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index d54fa441..609bdb1d 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -323,6 +323,13 @@ jobs: fi env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + # Where Help → Report a Problem sends reports. A Sentry DSN is a + # write-only public key, so baking it into the build is how Sentry is + # meant to be used — it lives in the repo secrets rather than the + # source so it can be rotated without a release, and so a fork builds + # with reporting simply off. Absent, reports are written to the user's + # data directory instead of sent, and the dialog says so. + SPYDE_SENTRY_DSN: ${{ secrets.SPYDE_SENTRY_DSN }} # macOS notarization (electron-builder's notarize:true reads these). Empty # on non-mac legs / secret-less runs → notarization is simply skipped there. # APPLE_API_KEY is a PATH, set by the "Stage notarization API key" step. diff --git a/CLAUDE.md b/CLAUDE.md index 62473926..6b83c6f8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -135,6 +135,15 @@ A Direct Electron `.csb` file is a **sparse event stream, not a frame stack** - `log_stream.py`: tags each log record with a subsystem `area` (`_area_for` / `_AREA_RULES`) and streams it to the renderer's Log panel (which has search + area filter). - `process_guard.py`: reaps orphaned Dask worker subprocesses on exit (Windows Job Object). +### Update handoff + problem reports (`packages/shell-main/`) +Both live in the shared shell, so all three DE apps get them. + +**The update handoff is a race, and losing it strands the user.** electron-updater spawns the installer FIRST and only then asks the app to quit, so for a second or two both are alive — and the Windows installer opens by refusing to touch a directory anything is still running out of. Two halves keep that from dead-ending in "SpyDE cannot be closed. Please close it manually and click Retry" (whose Retry re-runs the identical check, so the only way out was uninstalling by hand): +- `updater.ts` `quitAndInstall()` tree-kills the Python sidecar **synchronously** (`stopBackend({immediate: true})` — the ordinary path arms a 1.5 s timer that an exiting process never lives to fire) and force-exits after the handoff rather than trusting Electron's graceful quit to win. +- `electron/build/installer.nsh` replaces electron-builder's stock app-running check via the `customCheckAppRunning` macro — kills whole process **trees** by pid, waits between rounds, and gives the app several seconds before asking the user anything. **Do not delete this file**: electron-builder picks it up silently as the `nsis.include` default, so removing it restores the stock behaviour with no build error. + +**Problem reports are user-initiated and go to Sentry.** `errorReport.ts` collects OS / app + runtime versions / GPU / managed-Python-env state / the last updater status / the tail of the backend's output, and posts it as a Sentry envelope written against the ingest protocol directly (`sentryEnvelope.ts`, unit-tested) rather than via `@sentry/electron` — the SDK's value is automatic crash capture, which the "nothing without a click" decision rules out anyway, and skipping it keeps a native crash handler out of the notarized macOS build. `problemLog.ts` is the bounded ring of failures recorded as they happen (its own module purely to keep `updater.ts` → ring → `errorReport.ts` → `updater.ts` from being a cycle); it never leaves the machine on its own. The DSN comes from `SPYDE_SENTRY_DSN` at build time (repo secret → `electron.vite.config.ts` `define`); with none, reports are still written to `/reports/` and the dialog says so — that is the offline instrument-PC path, not a degraded one. + ## Testing Tests are **Qt-free** (no `pytest-qt`, no `QApplication`). They build a real `Session` (with a 1-worker Dask cluster) and assert on the JSON messages it emits + the signal-tree/plot state. Fixtures live in `spyde/tests/migrated/conftest.py`: diff --git a/electron/build/installer.nsh b/electron/build/installer.nsh new file mode 100644 index 00000000..63d4cb50 --- /dev/null +++ b/electron/build/installer.nsh @@ -0,0 +1,100 @@ +; installer.nsh -- SpyDE's replacement for electron-builder's "is the app still +; running?" check. electron-builder picks this file up automatically from +; buildResources (the `nsis.include` default), and defining +; `customCheckAppRunning` overrides the stock macro in both the installer and +; the uninstaller. +; +; WHY THIS EXISTS +; +; The stock check (app-builder-lib/templates/nsis/include/ +; allowOnlyOneInstallerInstance.nsh) asks PowerShell for every process whose +; executable path sits under $INSTDIR, stops them one at a time with +; Stop-Process, re-checks with no pause, and after two failed rounds ends in a +; "SpyDE cannot be closed. Please close it manually and click Retry" box whose +; Retry re-runs the identical check. That is a dead end for the user: nothing +; they can do in the dialog changes the answer, and the only way out is to +; uninstall by hand. +; +; Three things make it trip during a real SpyDE update: +; +; * Stop-Process ends ONE process. SpyDE runs a Python sidecar which itself +; runs Dask workers, so killing a parent leaves children that are found +; again on the next round. +; * The install directory holds more than the app binary -- resources\python +; carries uv and a vendored git -- so the path-prefix match can catch a +; helper the app is mid-way through running. +; * Re-checking with no delay counts processes that are already exiting. +; +; So: kill whole process TREES by pid, wait between rounds, give the app several +; seconds to go before asking the user for anything, and make Retry actually +; retry the kill rather than just the question. +; +; The app side of the same race is in packages/shell-main/src/updater.ts -- it +; tears the sidecar down and force-exits rather than trusting Electron's +; graceful quit to win. Both halves are needed: this macro cannot reach a +; sidecar that lives outside $INSTDIR, and the app cannot clean up after a +; crash it did not survive. + +Var spydeSetupPid + +!macro customCheckAppRunning + Push $0 + Push $R0 + Push $R1 + + ; Our own pid, so a tree-kill can never take down the installer itself. (The + ; stock macro's $pid is not declared when this override is in play.) + System::Call 'kernel32::GetCurrentProcessId() i .r0' + StrCpy $spydeSetupPid $0 + + StrCpy $R1 0 ; rounds spent asking the user + + spydeCloseRetry: + DetailPrint "Closing ${PRODUCT_NAME}..." + + ; One PowerShell pass: up to 12 rounds of "find everything running out of + ; the install directory, kill each one's whole tree, wait". Exits 0 the + ; moment nothing is left, 1 if the directory is still busy after ~6 s. + ; The trailing backslash on the directory keeps a sibling install (...\spyde + ; vs ...\spyde-old) from matching. `$$` is NSIS's escape for a literal `$`, + ; so `$$_` reaches PowerShell as `$_`. + nsExec::Exec `"$SYSDIR\WindowsPowerShell\v1.0\powershell.exe" -NoProfile -NonInteractive -Command "$$dir='$INSTDIR\'; $$self=$spydeSetupPid; for($$round=0; $$round -lt 12; $$round++){ $$busy=@(Get-CimInstance Win32_Process -ErrorAction SilentlyContinue | Where-Object { $$_.Path -and $$_.Path.StartsWith($$dir,'CurrentCultureIgnoreCase') -and $$_.ProcessId -ne $$self }); if($$busy.Count -eq 0){ exit 0 }; foreach($$victim in $$busy){ $$null = & taskkill.exe /PID $$victim.ProcessId /T /F 2>&1 }; Start-Sleep -Milliseconds 500 }; exit 1"` + Pop $R0 + + ; nsExec pushes "error" when the program could not be started at all -- a + ; machine with no PowerShell, or one where it is blocked by policy. Fall + ; back to killing the app's own image tree, which needs nothing but + ; taskkill. It exits 128 ("no tasks") when there was nothing to kill, so + ; both 0 and 128 mean the directory is clear afterwards. + ${if} $R0 == "error" + nsExec::Exec `"$SYSDIR\cmd.exe" /C taskkill /IM "${APP_EXECUTABLE_FILENAME}" /T /F /FI "USERNAME eq %USERNAME%"` + Pop $R0 + Sleep 1000 + ${if} $R0 == 128 + StrCpy $R0 0 + ${endif} + ${endif} + + ${if} $R0 == 0 + Goto spydeClosed + ${endif} + + ; Still busy. An update handoff is allowed one silent second round: the app + ; that spawned this installer may simply be slow to die. + IntOp $R1 $R1 + 1 + ${if} $R1 < 2 + Sleep 1000 + Goto spydeCloseRetry + ${endif} + + ; Genuinely stuck -- something is holding the install directory that we are + ; not allowed to end. Now the user's help is worth asking for, and Retry + ; runs the whole kill pass again rather than re-asking the same question. + MessageBox MB_RETRYCANCEL|MB_ICONEXCLAMATION "$(appCannotBeClosed)" /SD IDCANCEL IDRETRY spydeCloseRetry + Quit + + spydeClosed: + Pop $R1 + Pop $R0 + Pop $0 +!macroend diff --git a/electron/electron-builder.yml b/electron/electron-builder.yml index 4649736e..a5410bfb 100644 --- a/electron/electron-builder.yml +++ b/electron/electron-builder.yml @@ -74,6 +74,12 @@ win: # Per-user NSIS installer: no admin, Start-Menu + desktop shortcuts, uninstaller, # user-chosen install dir. +# +# build/installer.nsh is picked up automatically (it is the `nsis.include` +# default) and replaces the stock "is the app still running?" check, whose +# single-process kill and instant re-check dead-ended a Windows auto-update in +# "SpyDE cannot be closed. Please close it manually and click Retry". Read that +# file before changing anything about how the app shuts down. nsis: oneClick: false perMachine: false diff --git a/electron/electron.vite.config.ts b/electron/electron.vite.config.ts index 4c4f3f0e..678a9b0b 100644 --- a/electron/electron.vite.config.ts +++ b/electron/electron.vite.config.ts @@ -47,10 +47,19 @@ const shellPreload = resolve( const shellRenderer = resolve( __dirname, '..', 'packages', 'shell-renderer', 'src', 'index.ts') +// Where Help -> Report a Problem sends reports. A Sentry DSN is a write-only +// public key, so baking it into the build is how Sentry is meant to be used — +// but it comes from CI's environment rather than the repo so it can be rotated +// without a code change, and so a fork builds with reporting simply switched +// off. Absent, reports are written to the user's data directory instead of +// being sent (packages/shell-main/src/errorReport.ts). +const sentryDsn = process.env.SPYDE_SENTRY_DSN ?? '' + export default defineConfig({ main: { build: { outDir: 'out/main', rollupOptions: { input: 'src/main/index.ts' } }, resolve: { alias: { '@de/shell-main': shellMain } }, + define: { SENTRY_DSN: JSON.stringify(sentryDsn) }, }, preload: { build: { outDir: 'out/preload', rollupOptions: { input: 'src/preload/index.ts' } }, diff --git a/electron/src/main/index.ts b/electron/src/main/index.ts index e52238fd..46b82589 100644 --- a/electron/src/main/index.ts +++ b/electron/src/main/index.ts @@ -17,6 +17,8 @@ import { parseUvLine, initUpdater, checkForUpdates, downloadUpdate, quitAndInstall, readUpdateChannel, setUpdateChannel, getLastUpdateStatus, updatesSupported, + initErrorReporting, reportingConfigured, recordProblem, submitReport, + collectDiagnostics, } from '@de/shell-main' // Tell the shell who we are. MUST run before any other @de/shell-main call: @@ -31,6 +33,9 @@ configureShell({ pythonDist: 'spyde', }) +/** Baked in at build time by electron.vite.config.ts — '' when unconfigured. */ +declare const SENTRY_DSN: string + let win: BrowserWindow | null = null // ── Global crash backstop ───────────────────────────────────────────────────── @@ -47,6 +52,10 @@ let win: BrowserWindow | null = null function surfaceMainProcessCrash(kind: string, err: unknown): void { const detail = (err as Error)?.stack ?? (err as Error)?.message ?? String(err) process.stderr.write(`[spyde main] ${kind}: ${detail}\n`) + // Keep it for a problem report the user may write later. Recording is local + // and unconditional; nothing is sent unless they open Help -> Report a + // Problem and press Send. + recordProblem(kind, detail) try { if (win && !win.isDestroyed() && !win.webContents.isDestroyed()) { win.webContents.send('spyde:message', { @@ -426,6 +435,25 @@ app.whenReady().then(async () => { // doesn't compete with the Python sidecar coming up. initUpdater(win!, app.getPath('userData')) + // Problem reporting. The DSN is baked in at build time from the CI + // environment (see electron.vite.config.ts) so it can be rotated without a + // code change; SPYDE_SENTRY_DSN overrides it for a local test. With neither, + // reports are still written to /reports and the dialog says so. + initErrorReporting({ + dsn: process.env.SPYDE_SENTRY_DSN ?? SENTRY_DSN, + collectHostDiagnostics: async () => { + const env = managedEnvPaths(process.resourcesPath, app.getPath('userData')) + return { + nvidia: await probeNvidiaSmi(), + pythonEnv: { + bundled: env.bundled, + envExists: env.envExists, + lockedTorch: env.bundled ? readLockedTorchVersion(env.projectDir) : null, + }, + } + }, + }) + // Resolve (and on first packaged run, create via `uv sync`) the Python // sidecar env, then start the backend. // __dirname is electron/out/main → three levels up is the repo root (dev), @@ -535,6 +563,13 @@ app.whenReady().then(async () => { if (msg.type === 'ready' || msg.type === 'dask_ready' || msg.type === 'error') { console.log(`[spyde backend] ${msg.type}: ${msg.text ?? msg.dashboard ?? ''}`) } + // The two things worth having in a problem report written after the fact: + // the backend dying, and whatever it said before it did. + if (msg.type === 'backend_exited') { + recordProblem('backend', `Analysis backend exited (code ${String(msg.code)})`) + } else if (msg.type === 'error' && msg.text) { + recordProblem('backend', String(msg.text)) + } sendToRenderer(msg) }, onBinary: (header, payload) => { @@ -733,6 +768,11 @@ function buildMenu(): void { label: 'GPU Status…', click: () => win?.webContents.send('spyde:open_gpu_status_dialog'), }, + { type: 'separator' }, + { + label: 'Report a Problem…', + click: () => win?.webContents.send('spyde:open_report_dialog'), + }, ], }, ]) @@ -1053,6 +1093,23 @@ ipcMain.handle('spyde:get-update-info', () => ({ appVersion: app.getVersion(), })) +// ── Problem reporting (Help → Report a Problem) ─────────────────────────────── + +/** What the dialog shows before the user writes anything: whether a report can + * be sent at all, and the machine facts they are about to include. Reading + * them is free of side effects — nothing is sent by opening the dialog. */ +ipcMain.handle('spyde:report-diagnostics', async () => ({ + canSend: reportingConfigured(), + diagnostics: await collectDiagnostics(), +})) + +/** The user pressed Send. Writes the report locally either way, and sends it + * when a reporting service is configured and reachable. */ +ipcMain.handle( + 'spyde:submit-report', + (_, input: { message: string; contact?: string }) => submitReport(input), +) + /** Channel radio in the update dialog — persisted Electron-side (updater.ts) * AND mirrored into ~/.spyde/settings.json via the Python action so it's * visible/debuggable from that side too. */ diff --git a/electron/src/preload/index.ts b/electron/src/preload/index.ts index bb9a2375..0939a986 100644 --- a/electron/src/preload/index.ts +++ b/electron/src/preload/index.ts @@ -41,6 +41,9 @@ contextBridge.exposeInMainWorld('electron', { /** Open the "GPU & CUDA" help dialog (from the Help menu). Returns an unsubscribe fn. */ onOpenGpuHelpDialog: (cb: () => void) => onEvent('spyde:open_gpu_help_dialog', cb), + /** Open the "Report a Problem" dialog (from the Help menu). Returns an unsubscribe fn. */ + onOpenReportDialog: (cb: () => void) => onEvent('spyde:open_report_dialog', cb), + /** electron-updater's check/download/install progress. Returns an unsubscribe fn. */ onUpdateStatus: (cb: (v: Record) => void) => onEvent<[Record]>('spyde:update-status', cb), @@ -122,6 +125,25 @@ contextBridge.exposeInMainWorld('electron', { /** Flip the update channel (stable/beta). */ setUpdateChannel: (channel: 'stable' | 'beta') => ipcRenderer.send('spyde:set-update-channel', channel), + // ── Problem reporting ───────────────────────────────────────────────────── + + /** What the Report a Problem dialog shows before anything is written: whether + * a report can be sent at all, and the machine facts it would include. + * Reading this sends nothing. */ + reportDiagnostics: (): Promise<{ + canSend: boolean + diagnostics: Record + }> => ipcRenderer.invoke('spyde:report-diagnostics'), + + /** Send the report. Always saves a copy locally; `sent` says whether it also + * reached the maintainers. */ + submitReport: (input: { message: string; contact?: string }): Promise<{ + sent: boolean + eventId?: string + bundlePath?: string + error?: string + }> => ipcRenderer.invoke('spyde:submit-report', input), + /** GPU triage probe (Help → GPU & CUDA): nvidia-smi result + managed-env * facts. torch-side facts come from the backend's get_gpu_status. */ gpuTriage: (): Promise<{ diff --git a/electron/src/renderer/src/App.tsx b/electron/src/renderer/src/App.tsx index 9644ec69..b489eaa3 100644 --- a/electron/src/renderer/src/App.tsx +++ b/electron/src/renderer/src/App.tsx @@ -12,6 +12,7 @@ import { StackGate } from './components/StackGate' import { UpdateGate } from './components/UpdateGate' import { GpuStatusGate } from './components/GpuStatusGate' import { GpuHelpGate } from './components/GpuHelpGate' +import { ReportProblemGate } from './components/ReportProblemGate' import { UpdateCard } from './components/UpdateCard' import { MenuBar } from './components/MenuBar' import { DownloadToasts } from './components/DownloadToasts' @@ -105,6 +106,7 @@ export function App() { + {/* First-run welcome walkthrough (docs overhaul Phase 4): auto-opens the "First Steps" tour exactly once, tracked by the tutorial_seen settings flag. Always re-launchable afterwards from Help → First Steps. */} diff --git a/electron/src/renderer/src/components/MenuBar.tsx b/electron/src/renderer/src/components/MenuBar.tsx index a0654ff8..06519563 100644 --- a/electron/src/renderer/src/components/MenuBar.tsx +++ b/electron/src/renderer/src/components/MenuBar.tsx @@ -98,7 +98,7 @@ export function MenuBar({ onStartGuide, onShowInfo }: { /** Help → → Info… — opens GuideInfoDialog for that technique. */ onShowInfo: (g: Guide) => void }) { - const { sendAction, openStackDialog, openUpdateDialog, openGpuStatusDialog, openGpuHelpDialog, state } = useSpyDE() + const { sendAction, openStackDialog, openUpdateDialog, openGpuStatusDialog, openGpuHelpDialog, openReportDialog, state } = useSpyDE() const [open, setOpen] = useState(null) const barRef = useRef(null) const [exampleGroups, setExampleGroups] = useState([]) @@ -273,6 +273,8 @@ export function MenuBar({ onStartGuide, onShowInfo }: { { label: 'Check for Updates…', onClick: () => openUpdateDialog() }, { label: 'GPU & CUDA', onClick: () => openGpuHelpDialog() }, { label: 'GPU Status…', onClick: () => openGpuStatusDialog() }, + { separator: true }, + { label: 'Report a Problem…', onClick: () => openReportDialog() }, ], } diff --git a/electron/src/renderer/src/components/ReportProblemDialog.tsx b/electron/src/renderer/src/components/ReportProblemDialog.tsx new file mode 100644 index 00000000..3fe7f6ff --- /dev/null +++ b/electron/src/renderer/src/components/ReportProblemDialog.tsx @@ -0,0 +1,248 @@ +/** + * ReportProblemDialog.tsx — Help -> Report a Problem… + * + * What the user writes is the smaller half of a report. The rest — OS, app and + * runtime versions, GPU, the state of the managed Python environment, the last + * thing the updater said, and the tail of the backend's output — is collected + * by the main process (packages/shell-main/src/errorReport.ts) and shown here + * BEFORE anything is sent, because a report that quietly ships a machine + * description is a report people learn not to send. + * + * Nothing leaves the machine until Send is pressed, and a copy is always + * written to disk so an offline instrument PC still produces something the user + * can attach to an email. + */ +import React, { useEffect, useState } from 'react' + +type Phase = 'writing' | 'sending' | 'done' + +interface SubmitResult { + sent: boolean + eventId?: string + bundlePath?: string + error?: string +} + +export function ReportProblemDialog({ onClose }: { onClose: () => void }) { + const [message, setMessage] = useState('') + const [contact, setContact] = useState('') + const [canSend, setCanSend] = useState(false) + const [diagnostics, setDiagnostics] = useState | null>(null) + const [showDetails, setShowDetails] = useState(false) + const [phase, setPhase] = useState('writing') + const [result, setResult] = useState(null) + + useEffect(() => { + let cancelled = false + window.electron.reportDiagnostics().then((info) => { + if (cancelled) return + setCanSend(info.canSend) + setDiagnostics(info.diagnostics) + }).catch(() => { /* the dialog still works; details just stay empty */ }) + return () => { cancelled = true } + }, []) + + const submit = async () => { + setPhase('sending') + try { + setResult(await window.electron.submitReport({ message, contact })) + } catch (err) { + setResult({ sent: false, error: String(err) }) + } + setPhase('done') + } + + const problems = countProblems(diagnostics) + + return ( +
+
e.stopPropagation()}> +

Report a Problem

+ + {phase === 'done' && result ? ( + + ) : ( + <> +

+ {canSend + ? 'This goes straight to the SpyDE maintainers, with the details below attached.' + : 'This build has no reporting service configured, so the report will be saved ' + + 'to your computer for you to send on.'} +

+ + +