Codex/frontend code review fixes#285
Open
ClozyA wants to merge 2 commits into
Open
Conversation
Reviewer's Guide此 PR 在前端进一步收紧 Electron API 的类型及其使用方式，引入测试来强制这些约定，统一 Electron 调用，移除对 通过 useScriptApi 进行脚本配置导入流程的序列图sequenceDiagram
actor User
participant MaaEndUserEdit
participant useScriptApi
participant Service
User->>MaaEndUserEdit: click handleImportMaaEndConfig
MaaEndUserEdit->>useScriptApi: importScriptConfigFile(scriptId, userId)
useScriptApi->>Service: importScriptConfigFileApiScriptsConfigImportPost({ scriptId, userId })
Service-->>useScriptApi: response(code, message)
useScriptApi->>useScriptApi: [response.code !== 200] throw Error
useScriptApi-->>MaaEndUserEdit: resolved response
MaaEndUserEdit->>MaaEndUserEdit: message.success('已导入...配置文件')
使用 Ant Design Vue Modal 和 message 进行存储清理的序列图sequenceDiagram
actor User
participant QuickNavPage
participant Modal
participant BrowserStorage as localStorage_sessionStorage_IndexedDB
participant message
User->>QuickNavPage: click clearStorage
QuickNavPage->>Modal: Modal.confirm({ onOk })
Modal-->>User: show confirmation dialog
User->>Modal: confirm
Modal->>QuickNavPage: invoke onOk()
QuickNavPage->>BrowserStorage: localStorage.clear()
QuickNavPage->>BrowserStorage: sessionStorage.clear()
QuickNavPage->>BrowserStorage: [window.indexedDB]
QuickNavPage->>message: message.success('本地存储已清除，建议刷新页面')
QuickNavPage->>message: message.error('清除存储失败: ...')
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your Experience访问你的 dashboard 来：
Getting HelpOriginal review guide in EnglishReviewer's GuideThis PR tightens the Electron API typing and its usage across the frontend, introduces tests to enforce those contracts, standardizes Electron calls away from Sequence diagram for script config import flow via useScriptApisequenceDiagram
actor User
participant MaaEndUserEdit
participant useScriptApi
participant Service
User->>MaaEndUserEdit: click handleImportMaaEndConfig
MaaEndUserEdit->>useScriptApi: importScriptConfigFile(scriptId, userId)
useScriptApi->>Service: importScriptConfigFileApiScriptsConfigImportPost({ scriptId, userId })
Service-->>useScriptApi: response(code, message)
useScriptApi->>useScriptApi: [response.code !== 200] throw Error
useScriptApi-->>MaaEndUserEdit: resolved response
MaaEndUserEdit->>MaaEndUserEdit: message.success('已导入...配置文件')
Sequence diagram for storage clearing with Ant Design Vue Modal and messagesequenceDiagram
actor User
participant QuickNavPage
participant Modal
participant BrowserStorage as localStorage_sessionStorage_IndexedDB
participant message
User->>QuickNavPage: click clearStorage
QuickNavPage->>Modal: Modal.confirm({ onOk })
Modal-->>User: show confirmation dialog
User->>Modal: confirm
Modal->>QuickNavPage: invoke onOk()
QuickNavPage->>BrowserStorage: localStorage.clear()
QuickNavPage->>BrowserStorage: sessionStorage.clear()
QuickNavPage->>BrowserStorage: [window.indexedDB]
QuickNavPage->>message: message.success('本地存储已清除，建议刷新页面')
QuickNavPage->>message: message.error('清除存储失败: ...')
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - 我发现了 1 个问题，并且留下了一些整体反馈：
- 在
QuickNavPage.vue的clearStorage中，onOk处理函数在记录日志并展示错误消息之后又重新抛出了错误，这可能会在 UI 中表现为未捕获异常；建议在回调中完整处理错误（日志 + 用户反馈），不要重新抛出。 - 现在有多个调用点直接使用
window.electronAPI而没有进行存在性检查（例如初始化流程和后端启动相关组件），在非 Electron 环境或配置错误的环境中可能导致运行时错误；建议在 API 可能不可用的地方使用可选链或显式守卫并提供用户反馈。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `clearStorage` on `QuickNavPage.vue`, the `onOk` handler rethrows errors after logging and showing a message, which may surface as uncaught exceptions in the UI; consider handling the error entirely in the callback (log + user feedback) without rethrowing.
- Several call sites now use `window.electronAPI` directly without existence checks (e.g., initialization flow and backend start components), which can cause runtime errors in non‑Electron or misconfigured environments; consider using optional chaining or explicit guards and user feedback where the API might be unavailable.
## Individual Comments
### Comment 1
<location path="frontend/src/components/devtools/QuickNavPage.vue" line_range="142-151" />
<code_context>
+ okText: '清除',
+ okType: 'danger',
+ cancelText: '取消',
+ onOk: () => {
+ try {
+ localStorage.clear()
+ sessionStorage.clear()
+ // 清除IndexedDB（如果有）
+ if (window.indexedDB) {
+ // 这里可以添加更复杂的IndexedDB清理逻辑
+ }
+ logger.info('本地存储已清除')
+ message.success('本地存储已清除，建议刷新页面')
+ } catch (error) {
+ const errorMsg = error instanceof Error ? error.message : String(error)
+ logger.error(`清除存储失败: ${errorMsg}`)
+ message.error(`清除存储失败: ${errorMsg}`)
+ throw error
}
- logger.info('本地存储已清除')
</code_context>
<issue_to_address>
**issue (bug_risk):** 避免在 Modal 确认回调中展示错误消息后再次重新抛出错误
在这个 `onOk` 处理函数中，错误已经通过 `message.error` 记录并展示，因此重新抛出可能会在 `Modal.confirm` 中导致未处理的 Promise 拒绝或意外的 UI 错误。相反，应将其视为已处理的错误并省略 `throw`，或者在明确需要时返回一个被拒绝的 Promise，并确保调用方适当地处理它。
</issue_to_address>帮我变得更有用！请在每条评论上点击 👍 或 👎，我会根据这些反馈改进后续的代码审查。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- In
clearStorageonQuickNavPage.vue, theonOkhandler rethrows errors after logging and showing a message, which may surface as uncaught exceptions in the UI; consider handling the error entirely in the callback (log + user feedback) without rethrowing. - Several call sites now use
window.electronAPIdirectly without existence checks (e.g., initialization flow and backend start components), which can cause runtime errors in non‑Electron or misconfigured environments; consider using optional chaining or explicit guards and user feedback where the API might be unavailable.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `clearStorage` on `QuickNavPage.vue`, the `onOk` handler rethrows errors after logging and showing a message, which may surface as uncaught exceptions in the UI; consider handling the error entirely in the callback (log + user feedback) without rethrowing.
- Several call sites now use `window.electronAPI` directly without existence checks (e.g., initialization flow and backend start components), which can cause runtime errors in non‑Electron or misconfigured environments; consider using optional chaining or explicit guards and user feedback where the API might be unavailable.
## Individual Comments
### Comment 1
<location path="frontend/src/components/devtools/QuickNavPage.vue" line_range="142-151" />
<code_context>
+ okText: '清除',
+ okType: 'danger',
+ cancelText: '取消',
+ onOk: () => {
+ try {
+ localStorage.clear()
+ sessionStorage.clear()
+ // 清除IndexedDB（如果有）
+ if (window.indexedDB) {
+ // 这里可以添加更复杂的IndexedDB清理逻辑
+ }
+ logger.info('本地存储已清除')
+ message.success('本地存储已清除，建议刷新页面')
+ } catch (error) {
+ const errorMsg = error instanceof Error ? error.message : String(error)
+ logger.error(`清除存储失败: ${errorMsg}`)
+ message.error(`清除存储失败: ${errorMsg}`)
+ throw error
}
- logger.info('本地存储已清除')
</code_context>
<issue_to_address>
**issue (bug_risk):** Avoid rethrowing after showing an error message in the Modal confirm handler
In this `onOk` handler, errors are already logged and shown via `message.error`, so rethrowing can result in an unhandled rejection or unexpected UI error in `Modal.confirm`. Instead, treat this as a handled error and omit the `throw`, or deliberately return a rejected Promise and ensure callers handle it appropriately.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+142
to
+151
| onOk: () => { | ||
| try { | ||
| localStorage.clear() | ||
| sessionStorage.clear() | ||
| // 清除IndexedDB（如果有） | ||
| if (window.indexedDB) { | ||
| // 这里可以添加更复杂的IndexedDB清理逻辑 | ||
| } | ||
| logger.info('本地存储已清除') | ||
| message.success('本地存储已清除，建议刷新页面') |
There was a problem hiding this comment.
issue (bug_risk): 避免在 Modal 确认回调中展示错误消息后再次重新抛出错误
在这个 onOk 处理函数中，错误已经通过 message.error 记录并展示，因此重新抛出可能会在 Modal.confirm 中导致未处理的 Promise 拒绝或意外的 UI 错误。相反，应将其视为已处理的错误并省略 throw，或者在明确需要时返回一个被拒绝的 Promise，并确保调用方适当地处理它。
Original comment in English
issue (bug_risk): Avoid rethrowing after showing an error message in the Modal confirm handler
In this onOk handler, errors are already logged and shown via message.error, so rethrowing can result in an unhandled rejection or unexpected UI error in Modal.confirm. Instead, treat this as a handled error and omit the throw, or deliberately return a rejected Promise and ensure callers handle it appropriately.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
在前端范围内收紧 Electron API 的类型定义和使用方式，用带类型的辅助方法替代不安全的
any类型转换和直接的 fetch 调用，并改进面向用户的存储清理和日志导出流程。New Features:
appRestartElectron API，以便在前端进行受控的应用重启。LogExportResult契约用于日志导出操作，并将其传播到所有使用方。Bug Fixes:
Enhancements:
FileFilter类型和unknown日志参数类型，而非any。useScriptApi组合式函数来路由脚本配置导入，以集中业务逻辑并保持 Vue 页面精简。window.electronAPI的使用，在初始化、后端控制、设置、历史记录和编辑器视图中依赖带类型的 API，而不是在这些场景下使用any断言。Tests:
any的 Electron API 访问模式。Original summary in English
Summary by Sourcery
Tighten Electron API typing and usage across the frontend, replace unsafe
anycasts and direct fetch calls with typed helpers, and improve user-facing storage clearing and log export flows.New Features:
appRestartElectron API for controlled application restarts from the frontend.LogExportResultcontract for log export operations and propagate it to all consumers.Bug Fixes:
Enhancements:
FileFilterandunknownlogger argument types instead ofany.useScriptApicomposable to centralize business logic and keep Vue pages lean.window.electronAPIto rely on typed APIs rather thananyassertions in initialization, backend control, settings, history, and editor views.Tests:
any-based Electron API access patterns.