(Codex) 收紧 LimiterFileSystem 的 429 自动重试范围，避免写操作被盲目重放#1376
Conversation
There was a problem hiding this comment.
Pull request overview
该 PR 在 packages/filesystem 的限速/重试装饰器层（LimiterFileSystem + RateLimiter）中收紧了对 429 Too Many Requests 的自动重试策略，将其从“所有操作无差别重试”调整为“仅对相对安全的读类操作重试”，以降低写操作被隐式重放带来的数据风险。
Changes:
- 为
RateLimiter.execute()/executeWithRetry()增加操作类型（op）入参，并据此决定 429 是否允许自动重试 - 新增
shouldRetry429()，将“429 识别 + op 白名单”封装为统一判断逻辑 - 新增 Vitest 用例，覆盖部分读/写操作在 429 场景下的重试/不重试行为
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| packages/filesystem/limiter.ts | 引入按操作类型区分的 429 重试白名单，并在 LimiterFileSystem 各方法中传递 op |
| packages/filesystem/limiter.test.ts | 新增针对 list/read 的 429 重试、delete/createDir/write 的不重试测试 |
| @@ -68,6 +70,13 @@ | |||
| } | |||
| throw new Error("Max retries exceeded"); | |||
There was a problem hiding this comment.
executeWithRetry() 在达到重试上限后会抛出固定的 Max retries exceeded，丢失最后一次失败的错误上下文（以及当前 op）。这会显著降低定位问题的可观测性；建议把最后一次捕获的 error 附带到新错误里（例如拼接 message 或使用 cause），并包含 op 信息。
| import type FileSystem from "./filesystem"; | ||
| import type { FileInfo, FileReader, FileWriter } from "./filesystem"; | ||
|
|
||
| const RETRYABLE_429_OPS = new Set(["verify", "open", "read", "openDir", "list", "getDirUrl"]); |
There was a problem hiding this comment.
当前新增的“按操作类型决定 429 是否重试”的策略里，RETRYABLE_429_OPS 覆盖了 verify/open/openDir/getDirUrl 等操作，但测试只覆盖了 list/read 会重试以及 delete/createDir/write 不重试。建议补充至少对 verify/open/openDir/getDirUrl 的 429 重试测试，以及对 create 本身（fs.create 抛 429）不重试的测试，避免后续字符串/集合项改动导致策略悄然失效。
| /** | ||
| * 执行限速操作 | ||
| * @param fn 要执行的操作函数 | ||
| * @returns 操作结果 | ||
| */ | ||
| async execute<T>(fn: () => Promise<T>): Promise<T> { | ||
| async execute<T>(fn: () => Promise<T>, op = "unknown"): Promise<T> { | ||
| // 如果当前运行的操作数已达到上限，则等待 |
There was a problem hiding this comment.
RateLimiter.execute() 新增了 op 参数，但这里的 JSDoc 仍只描述了 fn；建议补充 op 的含义（用于 429 重试判定/操作类型）以及默认值行为，避免后续调用方误用或漏传。
| async create(path: string): Promise<FileWriter> { | ||
| return this.limiter.execute(async () => { | ||
| const writer = await this.fs.create(path); | ||
| return { | ||
| write: (content) => this.limiter.execute(() => writer.write(content)), | ||
| write: (content) => this.limiter.execute(() => writer.write(content), "write"), | ||
| }; | ||
| }); | ||
| }, "create"); | ||
| } |
There was a problem hiding this comment.
LimiterFileSystem.create() 没有透传 FileCreateOptions（例如 modifiedDate）：上层可以以第二个参数传入 opts，但这里会被忽略并导致实际创建/写入时丢失元信息。建议将方法签名改为接收 opts?: FileCreateOptions 并传递给 this.fs.create(path, opts)。
| createDir(dir: string): Promise<void> { | ||
| return this.limiter.execute(() => this.fs.createDir(dir)); | ||
| return this.limiter.execute(() => this.fs.createDir(dir), "createDir"); | ||
| } |
There was a problem hiding this comment.
LimiterFileSystem.createDir() 同样没有透传 opts?: FileCreateOptions，会导致调用方传入的目录创建元信息（如修改时间）被丢弃。建议调整签名并将 opts 传给 this.fs.createDir(dir, opts)。
- RateLimiter.execute / executeWithRetry 的 JSDoc 补充 op 参数说明 - executeWithRetry 末尾兜底错误带上 op，便于定位（理论上不可达） - LimiterFileSystem.create / createDir 透传 FileCreateOptions（修复预存在的元信息丢失） - 新增测试：verify/open/openDir/getDirUrl 在 429 自动重试，create 不重试 - 新增测试：create / createDir 的 FileCreateOptions 透传
背景
当前
packages/filesystem/limiter.ts里，RateLimiter.executeWithRetry()只要发现错误字符串包含429，就会统一进行指数退避重试。这个行为对读操作通常没问题，但对写操作风险很高。
例如在以下场景中：
429后再次自动重放同一个写操作这样会把“临时限流”放大成真正的数据风险：
本次修改
1. 给 429 自动重试增加操作类型边界
修改文件
packages/filesystem/limiter.ts修改内容
RateLimiter.execute()增加操作类型参数executeWithRetry()增加基于操作类型的判断429时自动重试：verifyopenreadopenDirlistgetDirUrl不再自动重试的操作
createwritecreateDirdelete修改意图
这次不是取消
429重试，而是把它从“无差别重试”改成“只对相对安全的读类操作重试”。核心意图：
保留对限流场景的基本恢复能力
list/read/verify仍然可以自动退避重试避免写操作被底层偷偷重放
write/createDir/delete不再由 limiter 擅自重试减少“请求已落地但响应丢失”时的数据破坏风险
为什么这样改
原实现的问题不在于“会不会重试”，而在于“它不知道自己在重试什么”。
对同步系统来说：
因此 limiter 不应该仅凭字符串里出现
429，就自动重放所有文件系统操作。这类策略如果不收紧，在以下场景中特别危险：
测试
新增测试
packages/filesystem/limiter.test.ts覆盖内容
list遇到429时会自动重试read遇到429时会自动重试delete遇到429时不会自动重试createDir遇到429时不会自动重试write遇到429时不会自动重试验证结果
vitest通过tsc --noEmit通过影响范围
本次改动只影响
LimiterFileSystem对429的自动重试策略。不影响：
预期收益
这次修改后，filesystem 层在面对限流和不稳定网络时会更保守：