Harden dev menu rebuild and extension startup paths

[manux81]

Pull request info

References

If applicable substitute the issue reference to the one that this PR refers

This PR resolve the N/A.

Link to Jira task

N/A

Description of the changes proposed

• guard development-only menu rebuild paths against destroyed BrowserWindow instances
• prevent duplicate context-menu listeners from accumulating across menu rebuilds
• catch and log rejected buildMenu() promises during theme updates and IPC-triggered rebuilds
• downgrade optional React DevTools installation failures to a compact warning during startup

DOD checklist

• The code is complete and according to developers’ standards.
• I have performed a self-review of my code.
• Meet the acceptance criteria.
• Unit tests are written and green.
• Test coverage: N/A %.
• Integration tests are written and green.
• Changes were communicated and updated in the ticket description.
• Reviewed and accepted by the Product Owner.
• End-to-end test are successful.

[coderabbitai]

Walkthrough

This pull request improves error handling and window lifecycle management across three files. It replaces promise chaining with explicit try/catch in extension installation, adds window liveness checks to prevent operations on destroyed windows, and ensures promise rejections are properly handled to avoid unhandled rejection warnings.

Changes

Cohort / File(s)	Summary
Error handling improvements src/main/main.ts, src/main/modules/ipc/main.ts	Replaced promise chaining with explicit try/catch blocks and .catch() handlers. Extension installation failure now derives messages from errors and logs warnings, while menu rebuilding properly logs error details instead of silently failing.
Window lifecycle guards src/main/menu.ts	Added hasLiveWindow() and getFallbackMenu() helpers to protect against operations on destroyed windows. buildMenu() returns fallback menu when mainWindow is destroyed, and context-menu and theme update operations are gated by liveness checks.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 When windows crash and errors bloom,
We catch them all before the gloom,
With defensive checks both here and there,
Our Electron app shows it cares! ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main changes: hardening dev menu rebuild and extension startup paths against teardown races and optional failures.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check	✅ Passed	The pull request description is well-structured, follows the template, and includes concrete details about the changes, problems addressed, and solutions implemented.

_{✏️ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/main/menu.ts (1)

163-170: ⚠️ Potential issue | 🟠 Major

Handle rejected buildMenu() promise in theme updates.

Line 170 calls buildMenu() without error handling. Since buildMenu() can reject if template building fails (via buildDarwinTemplate() or buildDefaultTemplate()), unhandled rejections will occur during theme toggles. The pattern for proper error handling already exists elsewhere in the codebase (see src/main/modules/ipc/main.ts:handleWindowRebuildMenu).

Suggested fix

   updateAppTheme() {
     const newTheme = nativeTheme.shouldUseDarkColors ? 'light' : 'dark'
     nativeTheme.themeSource = newTheme
     store.set('theme', newTheme)
     if (this.hasLiveWindow()) {
       this.mainWindow.webContents.send('system:update-theme')
     }
-    void this.buildMenu()
+    void this.buildMenu().catch((error) => {
+      console.error('Error rebuilding application menu:', error)
+    })
   }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/menu.ts` around lines 163 - 170, updateAppTheme toggles theme and
calls buildMenu() without handling rejections; wrap the buildMenu() call in a
promise rejection handler (e.g., use await in an async context or append
.catch(...)) so any errors from
buildMenu()/buildDarwinTemplate()/buildDefaultTemplate() are caught and handled
similarly to handleWindowRebuildMenu in src/main/modules/ipc/main.ts (log the
error and avoid unhandled rejection). Locate updateAppTheme, nativeTheme, store,
hasLiveWindow, mainWindow.webContents.send and replace the bare void
this.buildMenu() call with a safe invocation that catches and logs the error.

🧹 Nitpick comments (1)

src/main/menu.ts (1)

144-160: Remove duplicate context-menu listeners on repeated menu rebuilds.

buildMenu() is called multiple times (initial setup, IPC rebuild handler, theme updates), and each call invokes setupDevelopmentEnvironment(), which registers a new webContents.on('context-menu', ...) handler without removing the previous one. This accumulates listeners over dev sessions. Use webContents.removeAllListeners('context-menu') before registering, or switch to .once() if this listener should only fire once per rebuild.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/menu.ts` around lines 144 - 160, The context-menu handler registered
in setupDevelopmentEnvironment() (called from buildMenu()) is being added
repeatedly via this.mainWindow.webContents.on('context-menu', ...) causing
duplicate listeners; before registering the handler, remove existing handlers
with this.mainWindow.webContents.removeAllListeners('context-menu') (or use
.once(...) if it should fire only once) and then add the listener that calls
this.mainWindow.webContents.inspectElement(x, y); update
setupDevelopmentEnvironment() to perform this cleanup using the webContents API
to prevent accumulating listeners across rebuilds.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/main/menu.ts`:
- Around line 163-170: updateAppTheme toggles theme and calls buildMenu()
without handling rejections; wrap the buildMenu() call in a promise rejection
handler (e.g., use await in an async context or append .catch(...)) so any
errors from buildMenu()/buildDarwinTemplate()/buildDefaultTemplate() are caught
and handled similarly to handleWindowRebuildMenu in src/main/modules/ipc/main.ts
(log the error and avoid unhandled rejection). Locate updateAppTheme,
nativeTheme, store, hasLiveWindow, mainWindow.webContents.send and replace the
bare void this.buildMenu() call with a safe invocation that catches and logs the
error.

---

Nitpick comments:
In `@src/main/menu.ts`:
- Around line 144-160: The context-menu handler registered in
setupDevelopmentEnvironment() (called from buildMenu()) is being added
repeatedly via this.mainWindow.webContents.on('context-menu', ...) causing
duplicate listeners; before registering the handler, remove existing handlers
with this.mainWindow.webContents.removeAllListeners('context-menu') (or use
.once(...) if it should fire only once) and then add the listener that calls
this.mainWindow.webContents.inspectElement(x, y); update
setupDevelopmentEnvironment() to perform this cleanup using the webContents API
to prevent accumulating listeners across rebuilds.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 1008fc7e-70b1-40cc-bcbd-bdd9ba2e8830

📥 Commits

Reviewing files that changed from the base of the PR and between 8817969 and 0e8677d.

📒 Files selected for processing (3)

• src/main/main.ts
• src/main/menu.ts
• src/main/modules/ipc/main.ts