Skip to content

Fix runtime menu refresh crash when application menus are not yet received (#9762)#10070

Open
dpage wants to merge 1 commit into
pgadmin-org:masterfrom
dpage:worktree-fix-issue-9762-menu-refresh-crash
Open

Fix runtime menu refresh crash when application menus are not yet received (#9762)#10070
dpage wants to merge 1 commit into
pgadmin-org:masterfrom
dpage:worktree-fix-issue-9762-menu-refresh-crash

Conversation

@dpage

@dpage dpage commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Summary

  • The desktop (Electron) runtime crashed with TypeError: Cannot read properties of undefined (reading 'map') in menu.js, surfacing as an "A JavaScript error occurred in the main process" dialog.
  • Root cause: refreshMenus() rebuilds the application menu from the module-level cachedMenus, which is only populated once the renderer sends its menu definition via the setMenus IPC. If a menu refresh is triggered before that happens — e.g. an auto-update event on macOS, or the user closing the window (which fires updateConfigAndMenus('error-close', ...)) while the UI is still loading/blank — cachedMenus is undefined and bindMenuClicks() dereferences it.
  • Fix: guard refreshMenus() so it bails out early when there are no cached menus to rebuild. This matches the existing early-return style in the same file.

This explains the reporters' symptom of the error box appearing when clicking the window's X while the UI was stuck on a blank/loading screen, and the macOS variant triggered via the auto-updater.

Test plan

  • node --check passes on the modified menu.js.
  • Manual: launch the desktop runtime and close the main window before the UI finishes loading — no uncaught-exception dialog appears.
  • Manual (macOS): trigger an auto-update check before menus are set — no crash.

Closes #9762

Summary by CodeRabbit

  • Bug Fixes
    • Fixed a desktop application crash that occurred when menu refresh was triggered before application menus finished loading. This error would interrupt the startup process. The fix adds a safety check to prevent the crash and ensures a stable and reliable application startup experience.

…rg#9762)

refreshMenus() rebuilt the application menu from the module-level
cachedMenus, which is only populated once the renderer sends its menu
definition via the 'setMenus' IPC. When a menu refresh was triggered
before that happened - e.g. an auto-update event, or the user closing
the window while the UI was still loading - cachedMenus was undefined
and bindMenuClicks() crashed with 'Cannot read properties of undefined
(reading map)', surfacing as an uncaught-exception dialog.

Guard refreshMenus() so it bails out when there are no cached menus to
rebuild.

Closes pgadmin-org#9762
@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 51cc25f3-2533-484a-8388-9e2a5d2d7252

📥 Commits

Reviewing files that changed from the base of the PR and between 04fa05c and f8ecfd3.

📒 Files selected for processing (2)
  • docs/en_US/release_notes_9_16.rst
  • runtime/src/js/menu.js

Walkthrough

Added a safety check in refreshMenus to prevent crashes when menu refresh is triggered before application menus are received via IPC. The fix exits early if cached menus are unavailable, preventing undefined property access. Release notes document the crash resolution for Issue #9762.

Changes

Menu Refresh Safety Check

Layer / File(s) Summary
Early return guard and documentation
runtime/src/js/menu.js, docs/en_US/release_notes_9_16.rst
refreshMenus exits early when cachedMenus is falsy, preventing undefined dereference during refresh events before IPC initialization. Release notes document the fix for the desktop runtime crash (Issue #9762).

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main fix: preventing a runtime menu refresh crash when application menus have not yet been received.
Linked Issues check ✅ Passed The pull request fully addresses the requirements from issue #9762 by implementing the guard in refreshMenus() to prevent crashes when cachedMenus is undefined.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the menu refresh crash described in issue #9762; no extraneous modifications are present.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Prevents the Electron runtime main-process crash that could occur when refreshMenus() is triggered before the renderer has sent the initial menu definition via setMenus IPC.

Changes:

  • Add an early return in refreshMenus() when cachedMenus is not yet populated, avoiding a map dereference crash.
  • Document the fix in the v9.16 release notes (Issue #9762).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
runtime/src/js/menu.js Guard refreshMenus() to avoid rebuilding menus when cachedMenus is still unset.
docs/en_US/release_notes_9_16.rst Add a v9.16 bug-fix note referencing Issue #9762.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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.

JavaScript Error Prevents App UI from Loading

2 participants