Skip to content

Fix uncaught "Unknown button clicked" crash in config load-error dialog - #2798

Open
bengotow wants to merge 1 commit into
masterfrom
claude/awesome-ritchie-e67zim
Open

Fix uncaught "Unknown button clicked" crash in config load-error dialog#2798
bengotow wants to merge 1 commit into
masterfrom
claude/awesome-ritchie-e67zim

Conversation

@bengotow

Copy link
Copy Markdown
Collaborator

Summary

Fixes MAILSPRING-CLIENT-EKError: Unknown button clicked, 8 users / 39 events, 100% on Windows (win32).

What I observed

  • The stack trace pointed to ConfigPersistenceManager._showLoadErrorDialog, called from ConfigPersistenceManager.load(), called synchronously from the constructor during Application.start() (app/src/browser/application.ts).
  • _showLoadErrorDialog shows an error dialog (via dialog.showMessageBoxSync) with three buttons — Quit / Try Again / Reset Configuration — when config.json fails to load, then switches on the clicked button index. The default branch of that switch threw new Error('Unknown button clicked') for any index other than 0, 1, or 2.
  • Querying all 39 events for this issue in Sentry showed tags[platform] was win32 for 100% of them.
  • Per Electron's docs, dialog.showMessageBoxSync's cancelId option — 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).
  • Because this all happens synchronously inside the ConfigPersistenceManager constructor, with no surrounding try/catch in application.ts, the thrown error propagates out of Application.start() uncaught — breaking app startup entirely. Since the underlying corrupted config.json is never fixed, affected users hit this again on every relaunch, which matches the 39 events across only 8 users.
  • Notably, _showSaveErrorDialog right below it in the same file already avoids this exact trap by using safe array-index lookup (['ignore', 'retry'][clickedIndex], which yields undefined — not a throw — for an out-of-range index) instead of a switch with a throwing default.

Fix

Replace the throwing switch in _showLoadErrorDialog with 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:

return ['quit', 'tryagain', 'reset'][clickedIndex] || 'quit';

load()'s existing handling of action === 'quit' (set userWantsToPreserveErrors, call app.quit()) already covers this path correctly, so no other changes were needed.

Test plan

  • Manually reproduce on Windows: corrupt 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.
  • Reviewed that load()'s existing action === 'quit' branch handles this path correctly (sets userWantsToPreserveErrors, calls app.quit()).

Generated by Claude Code

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
@indent-staging

indent-staging Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Warning

Indent Zero is shutting down on August 7th. Please migrate over to Indent 2.0 to continue getting PR reviews.

PR Summary

Fixes a Windows-only startup crash in the config load-error dialog. On Windows, dialog.showMessageBoxSync ignores the cancelId option, so dismissing the "Failed to load config.json" dialog via close button, Alt+F4, or Esc returns an index outside 0-2. Because this runs synchronously in ConfigPersistenceManager's constructor during startup, the switch's throw new Error('Unknown button clicked') was aborting app launch persistently (the corrupted config.json was never repaired).

  • app/src/browser/config-persistence-manager.ts: replace the throwing switch in _showLoadErrorDialog with ['quit', 'tryagain', 'reset'][clickedIndex] || 'quit', treating any unmapped index the same as clicking "Quit" — mirroring the defensive pattern already used by _showSaveErrorDialog in the same file.

Issues

No issues found.

CI Checks

All CI checks passed for commit be34916.

Custom Rules 3 rules evaluated, 3 passed, 0 failed

Passing This is a longer title to see what happens when they are too long to fit
Passing B
Passing Ben Rule

View all rules

@indent

indent Bot commented Aug 14, 2026

Copy link
Copy Markdown
PR Summary

Fixes an uncaught Error: Unknown button clicked that crashed app startup on Windows (Sentry MAILSPRING-CLIENT-EK; 39 events across 8 users, 100% win32). The config load-error dialog's button handler previously threw on any button index outside 0-2, and because it runs synchronously inside the ConfigPersistenceManager constructor during Application.start() with no surrounding try/catch, that throw aborted launch. On Windows Electron ignores dialog.showMessageBoxSync's cancelId, so dismissing the dialog (close button, Alt+F4, Esc) returns an out-of-range index (typically -1) and triggered the crash on every relaunch.

  • Replaced the throwing switch default in _showLoadErrorDialog with ['quit', 'tryagain', 'reset'][clickedIndex] || 'quit', treating any out-of-range dismissal as Quit.
  • Added a comment documenting the Windows cancelId quirk and why throwing here is fatal at startup.
  • Behavior-preserving for the labeled buttons (0/1/2 map to the same actions) and consistent with the sibling _showSaveErrorDialog array-lookup idiom.

Issues

No issues found.

CI Checks

All CI checks passed on be34916.

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.

2 participants