Persist runtime logs and improve log cleanup UX#179
Open
Eldon27232 wants to merge 8 commits intoMistEO:mainfrom
Open
Persist runtime logs and improve log cleanup UX#179Eldon27232 wants to merge 8 commits intoMistEO:mainfrom
Eldon27232 wants to merge 8 commits intoMistEO:mainfrom
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - 我发现了 3 个问题，并留下了一些高层次的反馈：
- 在
LogsPanel中，清除按钮现在只调用clear_log_files，而不再清除当前实例内存中的instanceLogs，从用户体验角度来看可能会有些令人意外——建议同时调用 store 的clearLogs（或等效方法），这样在清理文件的同时也能立即清空面板内容。 LogsPanel中新增的formatLogTime辅助函数把区域设置硬编码为zh-CN，这会导致非中文用户的时间格式不一致；建议改为使用当前的 i18n 语言（或浏览器默认语言）。logger.ts中的LOG_DIR_SIZE_KEY/bootLogDirSize逻辑目前只会被清除、从未被写入，因此getBootLogDirSize始终返回null；可以考虑在启动时写入这个值（例如在启动阶段计算并保存），或者删除这个未使用的 key 和状态以简化实现。
给 AI Agent 的提示
请根据以下代码评审评论进行修改：
## 总体评论
- 在 `LogsPanel` 中，清除按钮现在只调用 `clear_log_files`，而不再清除当前实例内存中的 `instanceLogs`，从用户体验角度来看可能会有些令人意外——建议同时调用 store 的 `clearLogs`（或等效方法），这样在清理文件的同时也能立即清空面板内容。
- `LogsPanel` 中新增的 `formatLogTime` 辅助函数把区域设置硬编码为 `zh-CN`，这会导致非中文用户的时间格式不一致；建议改为使用当前的 i18n 语言（或浏览器默认语言）。
- `logger.ts` 中的 `LOG_DIR_SIZE_KEY`/`bootLogDirSize` 逻辑目前只会被清除、从未被写入，因此 `getBootLogDirSize` 始终返回 `null`；可以考虑在启动时写入这个值（例如在启动阶段计算并保存），或者删除这个未使用的 key 和状态以简化实现。
## 逐条评论
### Comment 1
<location path="src/components/LogsPanel.tsx" line_range="28-34" />
<code_context>
+ return `${value.toFixed(value >= 100 || idx === 0 ? 0 : value >= 10 ? 1 : 2)} ${units[idx]}`;
+ }
+
+ function formatLogTime(date: Date) {
+ return date.toLocaleTimeString('zh-CN', {
+ hour12: false,
+ hour: '2-digit',
+ minute: '2-digit',
+ second: '2-digit',
+ });
+ }
+
</code_context>
<issue_to_address>
**suggestion:** 为日志时间格式硬编码 'zh-CN' 区域设置会忽略当前 UI 语言环境
`formatLogTime` 始终使用 `zh-CN`，因此日志时间的显示不会与用户的 UI 语言保持一致。建议：
- 传入 `undefined` 作为 locale，以使用浏览器/操作系统的默认区域设置，或
- 传入当前应用的语言（例如来自 `i18n.language`），并带上回退逻辑。
这样可以让日志时间戳与整个本地化 UI 的其他部分保持一致。
建议实现：
```typescript
function formatBytes(bytes: number) {
if (!Number.isFinite(bytes) || bytes <= 0) return '0 B';
const units = ['B', 'KB', 'MB', 'GB', 'TB'];
const idx = Math.min(Math.floor(Math.log(bytes) / Math.log(1024)), units.length - 1);
const value = bytes / Math.pow(1024, idx);
return `${value.toFixed(value >= 100 || idx === 0 ? 0 : value >= 10 ? 1 : 2)} ${units[idx]}`;
}
function formatLogTime(date: Date, locale?: string) {
return date.toLocaleTimeString(locale || undefined, {
hour12: false,
hour: '2-digit',
minute: '2-digit',
second: '2-digit',
});
}
```
1. 在 `LogsPanel` 内部调用 `formatLogTime` 的地方，传入当前 UI 语言，例如：
- 将 `const { t } = useTranslation();` 改为 `const { t, i18n } = useTranslation();`
- 将 `formatLogTime(log.timestamp)` 改为 `formatLogTime(log.timestamp, i18n.language)`。
2. 如果在 React/i18n 上下文之外使用 `formatLogTime`，可以不传第二个参数，以回退到浏览器/操作系统的默认区域设置。
</issue_to_address>
### Comment 2
<location path="src/utils/logger.ts" line_range="19-21" />
<code_context>
// 文件日志配置
let logsDir: string | null = null;
+let logFileName: string | null = null;
+export const LOG_RESET_KEY = 'mxu.log-reset-date';
+const LOG_DIR_SIZE_KEY = 'mxu.log-dir-size-on-boot';
+let bootLogDirSize: number | null = null;
/**
</code_context>
<issue_to_address>
**suggestion (bug_risk):** 启动时日志目录大小和 LOG_DIR_SIZE_KEY 从未被写入
`LOG_DIR_SIZE_KEY` 和 `bootLogDirSize` 被引入后从未被写入，因此 `getBootLogDirSize()` 将始终返回 `null`，并且该 key 也从未被保存。建议要么暂时移除这个状态和 key，直到确实需要这个缓存；要么补充写入逻辑（在计算目录大小的地方写入），并相应更新 `bootLogDirSize`，以让缓存按预期工作。
建议实现：
```typescript
// 文件日志配置
let logsDir: string | null = null;
let logFileName: string | null = null;
export const LOG_RESET_KEY = 'mxu.log-reset-date';
```
1. 移除在 `src/utils/logger.ts` 中对 `LOG_DIR_SIZE_KEY`、`bootLogDirSize` 和 `getBootLogDirSize()`（如果存在）的所有使用和导出。
2. 如果 `getBootLogDirSize()` 是公共 API 的一部分（已导出），要么移除该导出，要么更新调用方停止使用它，因为在真正需要之前我们先移除了启动大小缓存。
</issue_to_address>
### Comment 3
<location path="src/utils/runtimeLogPersistence.ts" line_range="188-58" />
<code_context>
+): void {
+ if (!canUseLocalStorage()) return;
+
+ if (!instanceId) {
+ clearPendingPersistedLogsWrite();
+ pendingPersistedLogsPayload = null;
+ window.localStorage.removeItem(RUNTIME_LOG_STORAGE_KEY);
+ return;
+ }
</code_context>
<issue_to_address>
**suggestion (bug_risk):** clearPersistedRuntimeLogs 在删除 localStorage 时缺少错误处理
`!instanceId` 分支直接调用了 `window.localStorage.removeItem`，而其他辅助函数都会用 try/catch 封装对 `localStorage` 的访问。由于 `removeItem` 也可能抛出异常（例如在配额限制或受限环境中），这可能会把意料之外的错误暴露给调用方。请用 try/catch 包裹这些 `removeItem` 调用，在失败时直接返回，以保持一致性和“尽力而为”的行为。
建议实现：
```typescript
export function clearPersistedRuntimeLogs(
instanceId?: string,
maxLogsPerInstance: number = DEFAULT_MAX_LOGS_PER_INSTANCE,
): void {
if (!canUseLocalStorage()) return;
if (!instanceId) {
clearPendingPersistedLogsWrite();
pendingPersistedLogsPayload = null;
try {
window.localStorage.removeItem(RUNTIME_LOG_STORAGE_KEY);
} catch {
// Best-effort cleanup; ignore storage removal failures for consistency
return;
}
return;
}
```
请在该文件中搜索所有直接调用 `window.localStorage.removeItem` 的地方，并以相同方式用 `try/catch` 封装，这样在受限环境中所有 localStorage 删除操作都能统一防护运行时错误。帮我变得更有用！请在每条评论上点击 👍 或 👎，我会根据你的反馈改进后续评审。
Original comment in English
Hey - I've found 3 issues, and left some high level feedback:
- In
LogsPanel, the Clear button now only callsclear_log_filesand no longer clears the in-memoryinstanceLogsfor the active instance, which may be surprising from a UX standpoint—consider also calling the store’sclearLogs(or an equivalent) so the panel content is cleared immediately in addition to the file cleanup. - The new
formatLogTimehelper inLogsPanelhardcodes thezh-CNlocale, which will produce inconsistent time formatting for non-Chinese users; consider using the current i18n locale (or the browser default) instead. - The
LOG_DIR_SIZE_KEY/bootLogDirSizelogic inlogger.tsis currently only ever cleared and never written, sogetBootLogDirSizealways returnsnull; either wire up setting this value (e.g., once at startup) or remove the unused key/state to simplify the implementation.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `LogsPanel`, the Clear button now only calls `clear_log_files` and no longer clears the in-memory `instanceLogs` for the active instance, which may be surprising from a UX standpoint—consider also calling the store’s `clearLogs` (or an equivalent) so the panel content is cleared immediately in addition to the file cleanup.
- The new `formatLogTime` helper in `LogsPanel` hardcodes the `zh-CN` locale, which will produce inconsistent time formatting for non-Chinese users; consider using the current i18n locale (or the browser default) instead.
- The `LOG_DIR_SIZE_KEY`/`bootLogDirSize` logic in `logger.ts` is currently only ever cleared and never written, so `getBootLogDirSize` always returns `null`; either wire up setting this value (e.g., once at startup) or remove the unused key/state to simplify the implementation.
## Individual Comments
### Comment 1
<location path="src/components/LogsPanel.tsx" line_range="28-34" />
<code_context>
+ return `${value.toFixed(value >= 100 || idx === 0 ? 0 : value >= 10 ? 1 : 2)} ${units[idx]}`;
+ }
+
+ function formatLogTime(date: Date) {
+ return date.toLocaleTimeString('zh-CN', {
+ hour12: false,
+ hour: '2-digit',
+ minute: '2-digit',
+ second: '2-digit',
+ });
+ }
+
</code_context>
<issue_to_address>
**suggestion:** Hard‑coded 'zh-CN' locale for log time formatting ignores current UI locale
`formatLogTime` always uses `zh-CN`, so log times won’t match the user’s UI language. Consider either:
- Passing `undefined` as the locale to use the browser/OS default, or
- Passing the active app locale (e.g. from `i18n.language`) with a fallback.
This keeps log timestamps consistent with the rest of the localized UI.
Suggested implementation:
```typescript
function formatBytes(bytes: number) {
if (!Number.isFinite(bytes) || bytes <= 0) return '0 B';
const units = ['B', 'KB', 'MB', 'GB', 'TB'];
const idx = Math.min(Math.floor(Math.log(bytes) / Math.log(1024)), units.length - 1);
const value = bytes / Math.pow(1024, idx);
return `${value.toFixed(value >= 100 || idx === 0 ? 0 : value >= 10 ? 1 : 2)} ${units[idx]}`;
}
function formatLogTime(date: Date, locale?: string) {
return date.toLocaleTimeString(locale || undefined, {
hour12: false,
hour: '2-digit',
minute: '2-digit',
second: '2-digit',
});
}
```
1. Where `formatLogTime` is called inside `LogsPanel`, pass the active UI locale, for example:
- Change `const { t } = useTranslation();` to `const { t, i18n } = useTranslation();`
- Change usages like `formatLogTime(log.timestamp)` to `formatLogTime(log.timestamp, i18n.language)`.
2. If `formatLogTime` is used outside of React/i18n context, you can call it without the second argument to fall back to the browser/OS default locale.
</issue_to_address>
### Comment 2
<location path="src/utils/logger.ts" line_range="19-21" />
<code_context>
// 文件日志配置
let logsDir: string | null = null;
+let logFileName: string | null = null;
+export const LOG_RESET_KEY = 'mxu.log-reset-date';
+const LOG_DIR_SIZE_KEY = 'mxu.log-dir-size-on-boot';
+let bootLogDirSize: number | null = null;
/**
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Boot log directory size and LOG_DIR_SIZE_KEY are never populated
`LOG_DIR_SIZE_KEY` and `bootLogDirSize` are introduced but never written to, so `getBootLogDirSize()` will always return `null` and the key is never populated. Either remove this state/key until you actually need the cache, or add the write path (where the directory size is computed) and update `bootLogDirSize` accordingly so the cache behaves as intended.
Suggested implementation:
```typescript
// 文件日志配置
let logsDir: string | null = null;
let logFileName: string | null = null;
export const LOG_RESET_KEY = 'mxu.log-reset-date';
```
1. Remove any usages/exports of `LOG_DIR_SIZE_KEY`, `bootLogDirSize`, and `getBootLogDirSize()` (if present) elsewhere in `src/utils/logger.ts`.
2. If `getBootLogDirSize()` is part of the public API (exported), either remove the export or adjust callers to stop using it, since the boot-size cache is being removed until it’s actually needed.
</issue_to_address>
### Comment 3
<location path="src/utils/runtimeLogPersistence.ts" line_range="188-58" />
<code_context>
+): void {
+ if (!canUseLocalStorage()) return;
+
+ if (!instanceId) {
+ clearPendingPersistedLogsWrite();
+ pendingPersistedLogsPayload = null;
+ window.localStorage.removeItem(RUNTIME_LOG_STORAGE_KEY);
+ return;
+ }
</code_context>
<issue_to_address>
**suggestion (bug_risk):** clearPersistedRuntimeLogs lacks error handling for localStorage removal
The `!instanceId` branch calls `window.localStorage.removeItem` directly, unlike other helpers that wrap `localStorage` in try/catch. Since `removeItem` can throw (e.g. quota/restricted environments), this could surface unexpected errors to consumers. Please wrap these `removeItem` calls in try/catch and return on failure for consistency and best‑effort behavior.
Suggested implementation:
```typescript
export function clearPersistedRuntimeLogs(
instanceId?: string,
maxLogsPerInstance: number = DEFAULT_MAX_LOGS_PER_INSTANCE,
): void {
if (!canUseLocalStorage()) return;
if (!instanceId) {
clearPendingPersistedLogsWrite();
pendingPersistedLogsPayload = null;
try {
window.localStorage.removeItem(RUNTIME_LOG_STORAGE_KEY);
} catch {
// Best-effort cleanup; ignore storage removal failures for consistency
return;
}
return;
}
```
Search this file for any other direct calls to `window.localStorage.removeItem` and wrap them in `try/catch` in the same way, so all localStorage removal is consistently guarded against runtime errors in restricted environments.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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
Why
This continues the work for MaaEnd issue #2138.
The previous implementation solved only part of the problem and still felt awkward in a few places:
Summary by Sourcery
在应用重启间持久化运行时日志，新增由后端支持的日志文件管理，并改进日志面板的用户体验和可配置性。
New Features:
Bug Fixes:
Enhancements:
Original summary in English
Summary by Sourcery
Persist runtime logs across restarts, add backend-backed log file management, and improve the logs panel UX and configurability.
New Features:
Bug Fixes:
Enhancements: