Skip to content

fix(window): guard close handler against destroyed BrowserWindow#1783

Merged
piorpua merged 1 commit intomainfrom
fix/sentry-ELECTRON-ET
Mar 29, 2026
Merged

fix(window): guard close handler against destroyed BrowserWindow#1783
piorpua merged 1 commit intomainfrom
fix/sentry-ELECTRON-ET

Conversation

@kaizhou-lab
Copy link
Copy Markdown
Collaborator

@kaizhou-lab kaizhou-lab commented Mar 27, 2026

Summary

  • Add isDestroyed() guard in main window close event handler to prevent TypeError: Object has been destroyed when close-to-tray is enabled

Changes

  • src/index.ts: Add early return when mainWindow.isDestroyed() is true in the close event handler
  • tests/unit/mainWindowCloseHandler.test.ts: Add 4 unit tests covering normal hide, disabled tray, quitting, and destroyed window scenarios

Related Issue

Closes #1782

Sentry

  • Issue: ELECTRON-ET — 7 events, fatal level
  • Error: TypeError: Object has been destroyed at src/index.ts:345 (mainWindow.hide())
  • Release: v1.8.32
  • Root cause: windowControlsBridge.close() triggers the close event, but the BrowserWindow may already be destroyed by the time the handler calls hide()

Verification

  • Unit tests pass (4/4)
  • Type check passes (tsc --noEmit)
  • Lint and format pass

Test Plan

  • Unit test: window hides when close-to-tray enabled and not quitting
  • Unit test: window does NOT hide when close-to-tray disabled
  • Unit test: window does NOT hide when app is quitting
  • Unit test: destroyed window skips hide (reproduces ELECTRON-ET)

@kaizhou-lab
fix(window): guard close handler against destroyed BrowserWindow
3f15b5d 
When close-to-tray is enabled, the close event handler calls
mainWindow.hide(). If the BrowserWindow has already been destroyed
(e.g. via windowControlsBridge.close()), this throws
"TypeError: Object has been destroyed".

Add an isDestroyed() check at the top of the close handler to bail
out early when the window is no longer valid.

Fixes ELECTRON-ET
@kaizhou-lab kaizhou-lab marked this pull request as ready for review March 27, 2026 04:40
@piorpua piorpua added the bot:reviewing Review in progress (mutex) label Mar 29, 2026
@piorpua
Copy link
Copy Markdown
Contributor

piorpua commented Mar 29, 2026

Code Review：fix(window): guard close handler against destroyed BrowserWindow (#1783)

变更概述

本 PR 在 src/index.ts 的 BrowserWindow close 事件处理器中添加了一行 isDestroyed() 防御性检查，修复了 Sentry ELECTRON-ET 中记录的 TypeError: Object has been destroyed。同时新增了 tests/unit/mainWindowCloseHandler.test.ts，包含 4 个单元测试覆盖正常隐藏、托盘关闭禁用、应用退出中以及窗口已销毁四种场景。

方案评估

结论：✅ 方案合理

isDestroyed() 是 Electron 官方推荐的 BrowserWindow 防御性调用模式，一行改动精准定位根因。fix 位置在 event.preventDefault() 之前，避免了对已销毁窗口发起任何方法调用，逻辑正确。测试采用本地模拟 handler 逻辑的方式绕开完整 Electron 环境依赖，是测试 Electron 主进程逻辑的常见折中方案。

问题清单

🔵 LOW — simulateCloseHandler 应移至 describe 外层作用域

文件：tests/unit/mainWindowCloseHandler.test.ts，第 17 行

问题代码：

describe('mainWindow close handler', () => {
  function simulateCloseHandler(
    mainWindow: { isDestroyed: () => boolean; hide: () => void },
    closeToTrayEnabled: boolean,
    isQuitting: boolean
  ): { defaultPrevented: boolean } {
    ...
  }
  ...

问题说明：oxlint consistent-function-scoping 规则警告：simulateCloseHandler 未捕获父作用域的任何变量，放在 describe 内部会在每次调用时重新创建。

修复建议：

// 移至 describe 外部
function simulateCloseHandler(
  mainWindow: { isDestroyed: () => boolean; hide: () => void },
  closeToTrayEnabled: boolean,
  isQuitting: boolean
): { defaultPrevented: boolean } {
  const event = { defaultPrevented: false, preventDefault: () => (event.defaultPrevented = true) };
  if (mainWindow.isDestroyed()) return event;
  if (closeToTrayEnabled && !isQuitting) {
    event.preventDefault();
    mainWindow.hide();
  }
  return event;
}

describe('mainWindow close handler', () => {
  ...

汇总

# 严重级别 文件 问题
1 🔵 LOW tests/unit/mainWindowCloseHandler.test.ts:17 simulateCloseHandler 应移至外层作用域（lint 警告）

结论

✅ 批准合并 — 无阻塞性问题

修复正确且极为克制，测试覆盖了关键场景。唯一的 LOW 问题是测试文件中的 lint 风格警告，不阻塞合并。

本报告由本地 pr-review skill 生成，包含完整项目上下文，无截断限制。

@piorpua
Copy link
Copy Markdown
Contributor

piorpua commented Mar 29, 2026

✅ 已自动 review，无阻塞性问题，正在触发自动合并。

@piorpua piorpua merged commit c4ea66c into main Mar 29, 2026
17 checks passed
@piorpua piorpua deleted the fix/sentry-ELECTRON-ET branch March 29, 2026 05:34
@piorpua piorpua added bot:done Auto-merged by bot and removed bot:reviewing Review in progress (mutex) labels Mar 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

bot:done Auto-merged by bot

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

fix(window): guard close handler against destroyed BrowserWindow

2 participants

@kaizhou-lab @piorpua