Fix uncaught "Unknown button clicked" crash in config load-error dialog - #2798
Fix uncaught "Unknown button clicked" crash in config load-error dialog#2798bengotow wants to merge 1 commit into
Conversation
On Windows, Electron's dialog.showMessageBoxSync ignores the cancelId option, so dismissing the "Failed to load config.json" dialog without clicking a button (via the window's close button, Alt+F4, or Esc) returns an index outside the expected 0-2 range. The switch statement in _showLoadErrorDialog threw for any unrecognized index, and since this runs synchronously inside the ConfigPersistenceManager constructor during app startup, the uncaught error prevented the app from launching at all — repeatedly, since the corrupted config.json was never fixed. Fall back to treating an unrecognized/dismissed dialog the same as "Quit" instead of throwing, mirroring the existing safe pattern already used by _showSaveErrorDialog in the same file. Fixes MAILSPRING-CLIENT-EK Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013o2MJfxYEMRxUr92RHeFZA
|
Warning Indent Zero is shutting down on August 7th. Please migrate over to Indent 2.0 to continue getting PR reviews.
|
|
Summary
Fixes MAILSPRING-CLIENT-EK —
Error: Unknown button clicked, 8 users / 39 events, 100% on Windows (win32).What I observed
ConfigPersistenceManager._showLoadErrorDialog, called fromConfigPersistenceManager.load(), called synchronously from the constructor duringApplication.start()(app/src/browser/application.ts)._showLoadErrorDialogshows an error dialog (viadialog.showMessageBoxSync) with three buttons — Quit / Try Again / Reset Configuration — whenconfig.jsonfails to load, thenswitches on the clicked button index. Thedefaultbranch of that switch threwnew Error('Unknown button clicked')for any index other than0,1, or2.tags[platform]waswin32for 100% of them.dialog.showMessageBoxSync'scancelIdoption — which controls what index is returned if the user dismisses the dialog without clicking a labeled button (e.g. via the dialog's native close button, Alt+F4, or Esc) — is explicitly ignored on Windows. That means on Windows there's no way to force a "cancel" index into the 0-2 range, and dismissing the dialog returns an out-of-range index (typically-1).ConfigPersistenceManagerconstructor, with no surroundingtry/catchinapplication.ts, the thrown error propagates out ofApplication.start()uncaught — breaking app startup entirely. Since the underlying corruptedconfig.jsonis never fixed, affected users hit this again on every relaunch, which matches the 39 events across only 8 users._showSaveErrorDialogright below it in the same file already avoids this exact trap by using safe array-index lookup (['ignore', 'retry'][clickedIndex], which yieldsundefined— not a throw — for an out-of-range index) instead of aswitchwith a throwingdefault.Fix
Replace the throwing
switchin_showLoadErrorDialogwith the same safe array-index pattern already used by_showSaveErrorDialog, falling back to'quit'(the least destructive option — it neither deletes the user's config nor loops) when the dialog is dismissed without an explicit button click:load()'s existing handling ofaction === 'quit'(setuserWantsToPreserveErrors, callapp.quit()) already covers this path correctly, so no other changes were needed.Test plan
config.json, launch, dismiss the error dialog via the window's close button/Alt+F4 instead of clicking a button, confirm the app quits cleanly instead of crashing with an uncaught exception.load()'s existingaction === 'quit'branch handles this path correctly (setsuserWantsToPreserveErrors, callsapp.quit()).Generated by Claude Code