Skip to content

fix(preview): guard messageApi calls against unmounted context holder#1793

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

fix(preview): guard messageApi calls against unmounted context holder#1793
piorpua merged 1 commit intomainfrom
fix/sentry-ELECTRON-C6

Conversation

@kaizhou-lab
Copy link
Copy Markdown
Collaborator

Closes #1792

Summary

  • Wrap messageApi.error() and messageApi.success() calls in handleOpenInSystem with try-catch to prevent TypeError: Cannot read properties of null (reading 'addInstance') when the Arco Design Message context holder has been unmounted after an async IPC operation
  • Add unit tests validating the defensive pattern

Sentry Issue

ELECTRON-C6 — 3 occurrences, last seen 2026-03-25, affecting v1.9.0

Root cause: After await ipcBridge.shell.openFile.invoke() rejects, the PreviewPanel may have unmounted, leaving contextHolderRef.current as null inside Arco's useMessage hook.

Verification

  • Unit tests pass (bun run test)
  • Type check passes (bunx tsc --noEmit)
  • Lint passes (bun run lint:fix)
  • Format passes (bun run format)

…mounted context holder

Arco Design's Message.useMessage() throws TypeError when contextHolderRef.current
is null (component unmounted after async IPC call). Wrap messageApi calls in
try-catch to prevent unhandled rejection.

Fixes ELECTRON-C6
@kaizhou-lab kaizhou-lab marked this pull request as ready for review March 27, 2026 08:16
@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(preview): guard messageApi calls against unmounted context holder (#1793)

变更概述

本 PR 修复了 Sentry issue ELECTRON-C6:在 handleOpenInSystem 中,await ipcBridge.shell.openFile.invoke() 完成后 PreviewPanel 可能已卸载,导致 Arco Design 的 messageApi.error() / messageApi.success() 抛出 TypeError: Cannot read properties of null (reading 'addInstance')。修复方式是将各 messageApi 调用包裹在 try-catch 中,并新增独立测试文件验证该防御性模式。


方案评估

结论:✅ 方案合理

针对 React 组件卸载后异步回调调用 UI API 的经典场景,try-catch 防御是标准且可靠的做法——相比 isMounted ref 方案,它同样健壮且无需额外状态管理。修复粒度精准,未引入不必要的抽象。PR 描述中对根因、触发路径及验证步骤的说明也很清晰。


问题清单

未发现新引入的问题。

Lint 工具报告的 no-shadowt 变量)和 no-unused-varserr / hasBuiltInOpenButton)均属已有警告,与本次 diff 无关,不计入本 PR。


汇总

# 严重级别 文件 问题
无新增问题

结论

批准合并 — 修复准确,代码简洁,测试覆盖了核心防御场景,无阻塞性问题。


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

CONCLUSION: APPROVED
IS_CRITICAL_PATH: false
PR_NUMBER: 1793

@piorpua
Copy link
Copy Markdown
Contributor

piorpua commented Mar 29, 2026

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

@piorpua piorpua merged commit e9f4e14 into main Mar 29, 2026
17 checks passed
@piorpua piorpua deleted the fix/sentry-ELECTRON-C6 branch March 29, 2026 06:19
@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

Labels

bot:done Auto-merged by bot

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(preview): TypeError null.addInstance in PreviewPanel handleOpenInSystem

2 participants