Skip to content

fix(platform): add fallback for app.getPath('logs') failure#1967

Merged
piorpua merged 1 commit intomainfrom
fix/sentry-ELECTRON-GV
Mar 31, 2026
Merged

fix(platform): add fallback for app.getPath('logs') failure#1967
piorpua merged 1 commit intomainfrom
fix/sentry-ELECTRON-GV

Conversation

@kaizhou-lab
Copy link
Copy Markdown
Collaborator

Summary

  • Add try-catch around app.getPath('logs') in ElectronPlatformServices with fallback to <userData>/logs
  • Add unit tests verifying both success and fallback paths

Sentry Issue

  • ELECTRON-GV: Error: Failed to get 'logs' path — 22+ events in ~14 minutes (escalating)
  • Affects conversation creation flow (createAcpAgentbuildWorkspaceWidthFilesgetSystemDir)

Root Cause

app.getPath('logs') can throw when Electron cannot resolve the logs directory path. The call was unguarded, causing unhandled promise rejections.

Verification

  • Unit tests pass: success path returns app.getPath('logs'), fallback path returns <userData>/logs
  • Type check passes (tsc --noEmit)
  • Lint and format clean

Test Plan

  • Unit test: getLogsDir returns correct path when app.getPath('logs') succeeds
  • Unit test: getLogsDir falls back to userData/logs when app.getPath('logs') throws
  • All platform tests pass (19/19)
  • Type check passes

Closes #1966

When Electron cannot determine the logs path (e.g. transient state at
startup), getLogsDir now falls back to <userData>/logs instead of
throwing an unhandled rejection.

Fixes ELECTRON-GV
@kaizhou-lab kaizhou-lab marked this pull request as ready for review March 31, 2026 03:05
@piorpua piorpua added the bot:reviewing Review in progress (mutex) label Mar 31, 2026
@piorpua
Copy link
Copy Markdown
Contributor

piorpua commented Mar 31, 2026

Code Review:fix(platform): add fallback for app.getPath('logs') failure (#1967)

变更概述

本 PR 为 ElectronPlatformServices.paths.getLogsDir 添加了 try-catch 防护:当 app.getPath('logs') 抛出异常时,回退到 <userData>/logs。改动涉及 src/common/platform/ElectronPlatformServices.ts(+7 行)和新增测试文件 tests/unit/platform/ElectronPlatformServices.test.ts(50 行)。该修复针对 Sentry 问题 ELECTRON-GV(14 分钟内 22+ 事件)。


方案评估

结论:✅ 方案合理

方案直接在异常发生点添加防护,回退路径使用 app.getPath('userData') + logs 子目录,与 Electron 在大多数平台上的默认日志目录逻辑一致。改动最小化,没有引入不必要的抽象,与项目架构(src/common/platform/ 层封装 Electron API)完全一致。userData 路径比 logs 路径更稳定(前者若失败则整个应用已无法运行),回退方案合理。


问题清单

✅ 未发现明显问题,代码质量良好,建议批准合并。


汇总

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

结论

  • 批准合并 — 改动精准修复了 Sentry 高频异常,方案合理,测试覆盖了正常和回退两条路径,代码质量良好。

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

CONCLUSION: APPROVED
IS_CRITICAL_PATH: false
PR_NUMBER: 1967

@piorpua
Copy link
Copy Markdown
Contributor

piorpua commented Mar 31, 2026

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

@piorpua piorpua merged commit 0e57685 into main Mar 31, 2026
17 checks passed
@piorpua piorpua deleted the fix/sentry-ELECTRON-GV branch March 31, 2026 05:34
@piorpua piorpua added bot:done Auto-merged by bot and removed bot:reviewing Review in progress (mutex) labels Mar 31, 2026
kaizhou-lab added a commit that referenced this pull request Mar 31, 2026
The inline auto-registered platform services in index.ts were missing
the try-catch fallback for app.getPath('logs') that was added to
ElectronPlatformServices in PR #1967. When code runs before
registerPlatformServices() is called, the inline path throws on systems
where app.getPath('logs') fails, causing 'Failed to get logs path'
errors through the adapter bridge IPC.
piorpua pushed a commit that referenced this pull request Mar 31, 2026
…1980)

The inline auto-registered platform services in index.ts were missing
the try-catch fallback for app.getPath('logs') that was added to
ElectronPlatformServices in PR #1967. When code runs before
registerPlatformServices() is called, the inline path throws on systems
where app.getPath('logs') fails, causing 'Failed to get logs path'
errors through the adapter bridge IPC.

Co-authored-by: kaizhou-lab <[email protected]>
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.

[Bug]: app.getPath('logs') throws on startup causing unhandled rejection

2 participants