🐛 收紧 LimiterFileSystem 的 429 自动重试范围，避免写操作被盲目重放 (#1376)

* 收紧 `LimiterFileSystem` 的 429 自动重试范围，避免写操作被盲目重放

* update shouldRetry429 condition

* update shouldRetry429 condition

* 处理 Copilot 评论：补全 op JSDoc / opts 透传 / 重试覆盖测试

- RateLimiter.execute / executeWithRetry 的 JSDoc 补充 op 参数说明
- executeWithRetry 末尾兜底错误带上 op，便于定位（理论上不可达）
- LimiterFileSystem.create / createDir 透传 FileCreateOptions（修复预存在的元信息丢失）
- 新增测试：verify/open/openDir/getDirUrl 在 429 自动重试，create 不重试
- 新增测试：create / createDir 的 FileCreateOptions 透传

---------

Co-authored-by: 王一之 <yz@ggnb.top>

Author: cyfung1031

packages/filesystem/limiter.test.ts

@@ -0,0 +1,184 @@
+import { afterEach, describe, expect, it, vi } from "vitest";
+import type FileSystem from "./filesystem";
+import type { FileInfo, FileReader, FileWriter } from "./filesystem";
+import LimiterFileSystem from "./limiter";
+
+function createFs(): FileSystem {
+  return {
+    verify: vi.fn(async () => {}),
+    open: vi.fn(async () => {
+      const reader: FileReader = {
+        read: vi.fn(async () => "content"),
+      };
+      return reader;
+    }),
+    openDir: vi.fn(async () => createFs()),
+    create: vi.fn(async () => {
+      const writer: FileWriter = {
+        write: vi.fn(async () => {}),
+      };
+      return writer;
+    }),
+    createDir: vi.fn(async () => {}),
+    delete: vi.fn(async () => {}),
+    list: vi.fn(async () => []),
+    getDirUrl: vi.fn(async () => "url"),
+  };
+}
+
+const file: FileInfo = {
+  name: "test.user.js",
+  path: "/test.user.js",
+  size: 1,
+  digest: "digest",
+  createtime: 1,
+  updatetime: 1,
+};
+
+describe("LimiterFileSystem", () => {
+  afterEach(() => {
+    vi.useRealTimers();
+    vi.restoreAllMocks();
+  });
+
+  it("should retry list on 429", async () => {
+    vi.useFakeTimers();
+    const fs = createFs();
+    vi.mocked(fs.list).mockRejectedValueOnce(new Error("429 Too Many Requests")).mockResolvedValueOnce([]);
+    const limiter = new LimiterFileSystem(fs);
+
+    const promise = limiter.list();
+    await vi.runOnlyPendingTimersAsync();
+
+    await expect(promise).resolves.toEqual([]);
+    expect(fs.list).toHaveBeenCalledTimes(2);
+  });
+
+  it("should retry verify on 429", async () => {
+    vi.useFakeTimers();
+    const fs = createFs();
+    vi.mocked(fs.verify).mockRejectedValueOnce(new Error("429 Too Many Requests")).mockResolvedValueOnce(undefined);
+    const limiter = new LimiterFileSystem(fs);
+
+    const promise = limiter.verify();
+    await vi.runOnlyPendingTimersAsync();
+
+    await expect(promise).resolves.toBeUndefined();
+    expect(fs.verify).toHaveBeenCalledTimes(2);
+  });
+
+  it("should retry open on 429", async () => {
+    vi.useFakeTimers();
+    const fs = createFs();
+    const reader: FileReader = { read: vi.fn(async () => "content") };
+    vi.mocked(fs.open).mockRejectedValueOnce(new Error("429 Too Many Requests")).mockResolvedValueOnce(reader);
+    const limiter = new LimiterFileSystem(fs);
+
+    const promise = limiter.open(file);
+    await vi.runOnlyPendingTimersAsync();
+
+    await expect(promise).resolves.toBeDefined();
+    expect(fs.open).toHaveBeenCalledTimes(2);
+  });
+
+  it("should retry openDir on 429", async () => {
+    vi.useFakeTimers();
+    const fs = createFs();
+    const inner = createFs();
+    vi.mocked(fs.openDir).mockRejectedValueOnce(new Error("429 Too Many Requests")).mockResolvedValueOnce(inner);
+    const limiter = new LimiterFileSystem(fs);
+
+    const promise = limiter.openDir("/dir");
+    await vi.runOnlyPendingTimersAsync();
+
+    await expect(promise).resolves.toBeDefined();
+    expect(fs.openDir).toHaveBeenCalledTimes(2);
+  });
+
+  it("should retry getDirUrl on 429", async () => {
+    vi.useFakeTimers();
+    const fs = createFs();
+    vi.mocked(fs.getDirUrl).mockRejectedValueOnce(new Error("429 Too Many Requests")).mockResolvedValueOnce("url");
+    const limiter = new LimiterFileSystem(fs);
+
+    const promise = limiter.getDirUrl();
+    await vi.runOnlyPendingTimersAsync();
+
+    await expect(promise).resolves.toBe("url");
+    expect(fs.getDirUrl).toHaveBeenCalledTimes(2);
+  });
+
+  it("should not retry create on 429", async () => {
+    const fs = createFs();
+    vi.mocked(fs.create).mockRejectedValueOnce(new Error("429 Too Many Requests"));
+    const limiter = new LimiterFileSystem(fs);
+
+    await expect(limiter.create(file.path)).rejects.toThrow("429 Too Many Requests");
+    expect(fs.create).toHaveBeenCalledTimes(1);
+  });
+
+  it("should pass FileCreateOptions through create", async () => {
+    const fs = createFs();
+    const limiter = new LimiterFileSystem(fs);
+    await limiter.create(file.path, { modifiedDate: 123 });
+    expect(fs.create).toHaveBeenCalledWith(file.path, { modifiedDate: 123 });
+  });
+
+  it("should pass FileCreateOptions through createDir", async () => {
+    const fs = createFs();
+    const limiter = new LimiterFileSystem(fs);
+    await limiter.createDir("/dir", { modifiedDate: 456 });
+    expect(fs.createDir).toHaveBeenCalledWith("/dir", { modifiedDate: 456 });
+  });
+
+  it("should not retry delete on 429", async () => {
+    const fs = createFs();
+    vi.mocked(fs.delete).mockRejectedValueOnce(new Error("429 Too Many Requests"));
+    const limiter = new LimiterFileSystem(fs);
+
+    await expect(limiter.delete("/test.user.js")).rejects.toThrow("429 Too Many Requests");
+    expect(fs.delete).toHaveBeenCalledTimes(1);
+  });
+
+  it("should not retry createDir on 429", async () => {
+    const fs = createFs();
+    vi.mocked(fs.createDir).mockRejectedValueOnce(new Error("429 Too Many Requests"));
+    const limiter = new LimiterFileSystem(fs);
+
+    await expect(limiter.createDir("/dir")).rejects.toThrow("429 Too Many Requests");
+    expect(fs.createDir).toHaveBeenCalledTimes(1);
+  });
+
+  it("should not retry writer.write on 429", async () => {
+    const fs = createFs();
+    const write = vi.fn(async () => {});
+    vi.mocked(fs.create).mockResolvedValueOnce({
+      write,
+    });
+    write.mockRejectedValueOnce(new Error("429 Too Many Requests"));
+    const limiter = new LimiterFileSystem(fs);
+    const writer = await limiter.create(file.path);
+
+    await expect(writer.write("content")).rejects.toThrow("429 Too Many Requests");
+    expect(write).toHaveBeenCalledTimes(1);
+  });
+
+  it("should retry reader.read on 429", async () => {
+    vi.useFakeTimers();
+    const fs = createFs();
+    const read = vi.fn(async () => "content");
+    vi.mocked(fs.open).mockResolvedValueOnce({
+      read,
+    });
+    read.mockRejectedValueOnce(new Error("429 Too Many Requests"));
+    read.mockResolvedValueOnce("content");
+    const limiter = new LimiterFileSystem(fs);
+    const reader = await limiter.open(file);
+
+    const promise = reader.read("string");
+    await vi.runOnlyPendingTimersAsync();
+
+    await expect(promise).resolves.toBe("content");
+    expect(read).toHaveBeenCalledTimes(2);
+  });
+});

packages/filesystem/limiter.ts

@@ -1,5 +1,7 @@
 import type FileSystem from "./filesystem";
-import type { FileInfo, FileReader, FileWriter } from "./filesystem";
+import type { FileCreateOptions, FileInfo, FileReader, FileWriter } from "./filesystem";
+
+const RETRYABLE_429_OPS = new Set(["verify", "open", "read", "openDir", "list", "getDirUrl"]);
 
 /**
  * 速率限制器
@@ -19,9 +21,10 @@ export class RateLimiter {
   /**
    * 执行限速操作
    * @param fn 要执行的操作函数
+   * @param op 操作类型，用于在遇到 429 时判断是否允许自动重试。默认值 "unknown" 不在白名单内，会被视为不可重试
    * @returns 操作结果
    */
-  async execute<T>(fn: () => Promise<T>): Promise<T> {
+  async execute<T>(fn: () => Promise<T>, op = "unknown"): Promise<T> {
     // 如果当前运行的操作数已达到上限，则等待
     while (this.running >= this.maxConcurrent) {
       await new Promise<void>((resolve) => {
@@ -31,7 +34,7 @@ export class RateLimiter {
 
     this.running++;
     try {
-      return await this.executeWithRetry(fn);
+      return await this.executeWithRetry(fn, op);
     } finally {
       this.running--;
       // 执行完成后，从队列中取出下一个等待的操作
@@ -45,17 +48,18 @@ export class RateLimiter {
   /**
    * 执行操作并处理 429 错误重试
    * @param fn 要执行的操作函数
+   * @param op 操作类型，用于判定该操作在遇到 429 时是否进入指数退避重试
    * @returns 操作结果
    */
-  private async executeWithRetry<T>(fn: () => Promise<T>): Promise<T> {
+  private async executeWithRetry<T>(fn: () => Promise<T>, op: string): Promise<T> {
     // 最多重试 10 次
     for (let i = 0; i <= 10; i++) {
       try {
         return await fn();
       } catch (error) {
         // 检查错误字符串中是否包含 429
-        const errorStr = String(error);
-        if (errorStr.includes("429") && i < 10) {
+        const errorStr = String(error).toLowerCase();
+        if (this.shouldRetry429(op, ` ${errorStr} `) && i < 10) {
           // 遇到 429 错误且未达到重试上限，采用指数退避策略延迟后继续重试
           const delay = Math.min(2000 * Math.pow(2, i), 60000);
           await new Promise((resolve) => setTimeout(resolve, delay));
@@ -66,7 +70,15 @@ export class RateLimiter {
         throw error;
       }
     }
-    throw new Error("Max retries exceeded");
+    // 理论上不会到达这里：循环最后一次的 catch 已经把原始错误抛出
+    throw new Error(`Max retries exceeded (op=${op})`);
+  }
+
+  private shouldRetry429(op: string, errorStr: string): boolean {
+    return (
+      ((errorStr.includes("429") && /[^a-z\d]429[^a-z\d]/.test(errorStr)) || errorStr.includes("too many requests")) &&
+      RETRYABLE_429_OPS.has(op)
+    );
   }
 }
 
@@ -83,47 +95,47 @@ export default class LimiterFileSystem implements FileSystem {
   }
 
   verify(): Promise<void> {
-    return this.limiter.execute(() => this.fs.verify());
+    return this.limiter.execute(() => this.fs.verify(), "verify");
   }
 
   async open(file: FileInfo): Promise<FileReader> {
     return this.limiter.execute(async () => {
       const reader = await this.fs.open(file);
       return {
-        read: (type) => this.limiter.execute(() => reader.read(type)),
+        read: (type) => this.limiter.execute(() => reader.read(type), "read"),
       };
-    });
+    }, "open");
   }
 
   async openDir(path: string): Promise<FileSystem> {
     return this.limiter.execute(async () => {
       const fs = await this.fs.openDir(path);
       return new LimiterFileSystem(fs, this.limiter);
-    });
+    }, "openDir");
   }
 
-  async create(path: string): Promise<FileWriter> {
+  async create(path: string, opts?: FileCreateOptions): Promise<FileWriter> {
     return this.limiter.execute(async () => {
-      const writer = await this.fs.create(path);
+      const writer = await this.fs.create(path, opts);
       return {
-        write: (content) => this.limiter.execute(() => writer.write(content)),
+        write: (content) => this.limiter.execute(() => writer.write(content), "write"),
       };
-    });
+    }, "create");
   }
 
-  createDir(dir: string): Promise<void> {
-    return this.limiter.execute(() => this.fs.createDir(dir));
+  createDir(dir: string, opts?: FileCreateOptions): Promise<void> {
+    return this.limiter.execute(() => this.fs.createDir(dir, opts), "createDir");
   }
 
   delete(path: string): Promise<void> {
-    return this.limiter.execute(() => this.fs.delete(path));
+    return this.limiter.execute(() => this.fs.delete(path), "delete");
   }
 
   list(): Promise<FileInfo[]> {
-    return this.limiter.execute(() => this.fs.list());
+    return this.limiter.execute(() => this.fs.list(), "list");
   }
 
   getDirUrl(): Promise<string> {
-    return this.limiter.execute(() => this.fs.getDirUrl());
+    return this.limiter.execute(() => this.fs.getDirUrl(), "getDirUrl");
   }
 }