Skip to content

[v1.4] updateResource 相关代码调整#1193

Draft
cyfung1031 wants to merge 39 commits intoscriptscat:release/v1.4from
cyfung1031:pr-updateResourceByType-return
Draft

[v1.4] updateResource 相关代码调整#1193
cyfung1031 wants to merge 39 commits intoscriptscat:release/v1.4from
cyfung1031:pr-updateResourceByType-return

Conversation

@cyfung1031
Copy link
Copy Markdown
Collaborator

@cyfung1031 cyfung1031 commented Feb 3, 2026

概述 Descriptions

我都不知道要怎么说明了

反正就是有问题的部份改了

有些觉得奇怪的,我没有改,只加了 comment

  • 主要移除了 loadNow 那个设定。代码太难维护了。所有 fetch 资源都加一个 concurrency 控制,避免一次过爆发就好。要不要等就上层决定吧。
  • 现在 concurrency 控制 是五个 fetch 一组。这样就不会大量 fetch 冲到同一个 server (例如 GreasyFork )

变更内容 Changes

截图 Screenshots

@cyfung1031 cyfung1031 requested a review from CodFrm February 3, 2026 10:04
@cyfung1031 cyfung1031 marked this pull request as draft February 3, 2026 10:09
@cyfung1031
Copy link
Copy Markdown
Collaborator Author

cyfung1031 commented Feb 3, 2026

弹出安装页的部份要先下载好资源
而不是到按安裝才下载
所以先改成Draft PR

UI 部份另行处理
先处理好 JS代码的改动

Comment on lines 409 to 431
await this.scriptCodeDAO.save({
uuid: script.uuid,
code: param.code,
});
logger.info("install success");

// Cache更新 & 下载资源
await Promise.all([
compiledResourceUpdatePromise,
this.resourceService.updateResourceByType(script, "require"),
this.resourceService.updateResourceByType(script, "require-css"),
this.resourceService.updateResourceByType(script, "resource"),
this.resourceService.updateResourceByTypes(script, ["require", "require-css", "resource"]),
]);
// 如果资源不完整,还是要接受安装吗???

// 广播一下
// Runtime 會負責更新 CompiledResource
this.publishInstallScript(script, { update, upsertBy });

return { update };
})
.catch((e: any) => {
logger.error("install error", Logger.E(e));
throw e;
});
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CodFrm scriptDAO 先 save 了显示成功。 如果资源下载失败,忽略掉是故意的吗?

是的话我就不改这做法
否则就改成下载不了不安装不更新

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

故意的,资源失败不影响脚本保存。不过这块逻辑可能还得优化一下

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

那我这个 PR 先不处理这个,先维持

Comment on lines +192 to +195
const updateTime = oldResources?.updatetime;
// 资源最后更新是24小时内则不更新
// 这里是假设 resources 都是 static. 使用者应该加 ?d=xxxx 之类的方式提示SC要更新资源
if (updateTime && updateTime > Date.now() - 86400_000) return;
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CodFrm 原代码有这个 updateTime 检查操作。但脚本更新可以是每6小时做一次。如果脚本更新了但资源不更新,好像有点奇怪

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

之前考虑过不做这个检查,感觉也可以,资源加载下来就不再失效,除非修改url

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我先不改动。你有需要再改一下吧

@cyfung1031 cyfung1031 changed the title [v1.3] updateResource 相关代码调整 [v1.4] updateResource 相关代码调整 Feb 24, 2026
@cyfung1031 cyfung1031 changed the base branch from release/v1.3 to release/v1.4 March 15, 2026 05:03
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

该 PR 主要围绕 Service Worker 侧的资源拉取/更新(updateResource)做重构:移除原先的 loadNow 分支,改为统一的资源获取路径,并新增一个“每次最多 5 个 fetch”的并发控制机制,以降低集中爆发请求对单一站点(如 GreasyFork)的压力。

Changes:

  • 新增通用并发控制工具(Semaphore + withTimeoutNotify),并用于资源 fetch 的并发限制
  • ResourceService:合并资源获取入口、增加按类型批量处理接口(getResourceByTypes / updateResourceByTypes),并替换原有 getScriptResources 的内部实现
  • parseUrlSRI 返回结构调整(补充 originalUrl),并联动更新 runtime/script/synchronize 的资源读取逻辑

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
src/pkg/utils/concurrency-control.ts 新增信号量与超时通知工具,用于资源请求并发控制
src/app/service/service_worker/utils.ts 调整 parseUrlSRI 返回类型(新增 originalUrl,hash 类型变化)
src/app/service/service_worker/synchronize.ts 备份生成时改为一次性按多类型拉取资源
src/app/service/service_worker/script.ts 安装流程改为批量触发资源更新;运行资源构建改用新资源读取接口
src/app/service/service_worker/runtime.ts 资源读取接口替换;file 资源更新逻辑接入新的 parseUrlSRI / updateResource 签名
src/app/service/service_worker/resource.ts 核心重构:移除 loadNow 路径,新增批量接口与 fetch 并发控制实现
src/app/repo/resource.ts 补充 contentType 字段语义注释

Comment on lines +264 to +283
async createResourceByUrlFetch(u: TUrlSRIInfo, type: ResourceType): Promise<Resource> {
const url = u.url; // 无 URI Integrity Hash

let released = false;
await fetchSemaphore.acquire();
// Semaphore 锁 - 同期只有五个 fetch 一起执行
const delay = randNum(100, 150); // 100~150ms delay before starting fetch
await sleep(delay);
// 执行 fetch, 若超过 800ms, 不会中止 fetch 但会启动下一个网络连接任务
// 这只为了避免等候时间过长,同时又不会有过多网络任务同时发生,使Web伺服器返回错误
const { result, err } = await withTimeoutNotify(fetch(url), 800, ({ done, timeouted, err }) => {
if (timeouted || done || err) {
// fetch 成功 或 发生错误 或 timeout 时解锁
if (!released) {
released = true;
fetchSemaphore.release();
}
}
});
// Semaphore 锁已解锁。继续处理 fetch Response 的结果
Copy link

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

createResourceByUrlFetch 里在 800ms timeout 时提前 release Semaphore,但实际 fetch 仍在进行;在网络慢/服务端卡顿时会导致“in-flight fetch”数量不受 5 的限制,反而可能越跑越多,失去并发控制意义并增加对目标站点的压力。建议把 release 放在 fetch settle(then/catch/finally)时,或改用 AbortController 真正中止超时请求;如果想做的是限速/分批启动,请单独实现 rate limiter 而不是提前释放并发锁。

Copilot uses AI. Check for mistakes.
Comment on lines +85 to +93
if (oldResources && !resourcePath.startsWith("file:///")) {
// 读取过但失败的资源加载也会被放在缓存,避免再加载资源
// 因此 getResource 时不会再加载资源,直接返回 undefined 表示没有资源
if (!oldResources.contentType) {
freshResource = undefined;
} else {
freshResource = oldResources;
}
} else {
Copy link

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getResourceByTypes 在 oldResources 已存在且是非 file:/// 时,如果 oldResources.contentType 为空(之前下载失败的空 Resource),会直接返回 undefined 且不会再次尝试更新,导致该资源失败后“永久缺失”(除非手动清缓存/删除记录)。建议对空 Resource 也加入重试逻辑(例如按 updatetime 做退避/过期重试),或在这里直接走 updateResource(uuid, u, type, oldResources) 以便恢复。

Copilot uses AI. Check for mistakes.
Comment on lines +1 to +57
export class Semaphore {
private active = 0;
private readonly queue: Array<() => void> = [];

constructor(readonly limit: number) {
if (limit < 1) throw new Error("limit must be >= 1");
}

async acquire() {
if (this.active >= this.limit) {
await new Promise<void>((resolve) => this.queue.push(resolve));
}
this.active++;
}

release() {
if (this.active > 0) {
this.active--;
this.queue.shift()?.();
} else {
console.warn("Semaphore double release detected");
}
}
}

type TWithTimeoutNotifyResult<T> = {
timeouted: boolean;
result: T | undefined;
done: boolean;
err: undefined | Error;
};
export const withTimeoutNotify = <T>(
promise: Promise<T>,
time: number,
fn: (res: TWithTimeoutNotifyResult<T>) => any
) => {
const res: TWithTimeoutNotifyResult<T> = { timeouted: false, result: undefined, done: false, err: undefined };
const cid = setTimeout(() => {
res.timeouted = true;
fn(res);
}, time);
return promise
.then((result: T) => {
clearTimeout(cid);
res.result = result;
res.done = true;
fn(res);
return res;
})
.catch((e) => {
clearTimeout(cid);
res.err = e;
res.done = true;
fn(res);
return res;
});
};
Copy link

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

新增的 concurrency-control 工具(Semaphore/withTimeoutNotify)目前缺少单元测试。src/pkg/utils 里已有大量 Vitest 测试文件,建议补充:1) Semaphore 的 acquire/release FIFO 行为与 limit 限制;2) withTimeoutNotify 在 timeout/resolve/reject 三种情况下的回调与返回值状态,避免后续并发控制改动引入回归。

Copilot uses AI. Check for mistakes.
CodFrm and others added 2 commits March 29, 2026 16:20
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

const scriptCodes = {} as Record<string, string>;
// 更新资源使用了file协议的脚本
// 更新资源使用了file协议的脚本 ( 不能在其他地方更新吗?? 见 Issue #918 )
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

指哪里?无法监听到file的变更,也不好处理吧

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

无法监听到file的变更,也不好处理吧

对。。
当时可能头脑不太清醒吧

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

改回注释 1fa3981

CodFrm and others added 3 commits March 29, 2026 17:21
- 提取魔法数字为命名常量(MAX_CONCURRENT_FETCHES, FETCH_DELAY, FETCH_SEMAPHORE_TIMEOUT, RESOURCE_CACHE_TTL)
- 清理疑问注释,改为明确的设计决策说明
- 为 Semaphore 和 withTimeoutNotify 添加单元测试
@cyfung1031
Copy link
Copy Markdown
Collaborator Author

📋 PR 概要 / PR Summary

  • PR: [v1.4] updateResource 相关代码调整 #1193 · [v1.4] updateResource 相关代码调整
  • 作者/Author: cyfung1031
  • Commits: 39(含多次 merge + lint/refactor 迭代)
  • 变更文件: 8 files(7 .ts + 1 .test.ts
  • 类型: 重构 + Bug 修复
  • Review 模式: ⚠ FALLBACK — 基于 PR 描述、commit log、GitHub UI 可见 diff 片段及 Copilot review 内容
  • 信心度: 架构层 (PR 描述 + Copilot review 已覆盖核心设计意图) · 行级别 (diff 片段可见,非完整 raw patch)

🔍 变更分析 / Change Analysis

src/pkg/utils/concurrency-control.ts ← 新增

引入 Semaphore(计数信号量)与 withTimeoutNotify(带 timeout 回调的 Promise 包装)。目的是限制资源 fetch 的并发上限为 5,避免向同一服务器(如 GreasyFork)短时间内发出大量请求。

评估:设计思路合理,Semaphore 实现简洁,double-release 有 warn 保护,FIFO 队列语义正确。

src/app/service/service_worker/resource.ts ← 核心重构

移除原有 loadNow 分支,统一为 createResourceByUrlFetch 入口;新增 getResourceByTypes / updateResourceByTypes 按类型批量处理接口;fetch 时加入 semaphore 控制 + 100-150ms 随机延迟。

评估:统一入口减少了代码分支,可维护性显著提升。但存在两个设计风险(见下方 Issues)。

src/app/service/service_worker/utils.ts

parseUrlSRI 返回结构新增 originalUrl 字段,hash 类型有调整。联动影响 runtime / script / synchronize 的资源读取。

src/app/service/service_worker/script.ts

安装流程从三次独立 updateResourceByType 调用合并为一次 updateResourceByTypes(["require", "require-css", "resource"]);资源构建改用新读取接口。

评估:合并调用减少冗余,方向正确。

src/app/service/service_worker/synchronize.ts

备份生成时同样改为 updateResourceByTypes 一次性调用。与 script.ts 保持一致,无明显风险。

src/app/service/service_worker/runtime.ts

资源读取接口替换;file:/// 资源更新逻辑接入新的 parseUrlSRI / updateResource 签名。疑问注释已还原(改回注释 commit)。

src/app/repo/resource.ts

contentType 字段新增语义注释:下载成功时必定有值,下载失败则为空(空 Resource)。文档化设计约定,有价值。

src/pkg/utils/concurrency-control.test.ts ← 新增

由 CodFrm 在 0b1ded9 中补充,覆盖 Semaphore acquire/release 行为与 withTimeoutNotify 的 resolve/reject/timeout 三路径。


⚠️ 问题与风险 / Issues & Risks

🔴 必须修正 / Must Fix

1. Semaphore 提前释放导致并发控制失效resource.ts · createResourceByUrlFetch

800ms timeout 触发时,semaphore 被释放,但底层 fetch() 仍在 in-flight。在网络慢或服务端响应慢的场景下,实际并发 in-flight 请求数量可能远超 5 的限制——随着更多 fetch 超时释放 slot、新 fetch 进入,并发数会持续累积。这与该 PR 的核心目标(限流)背道而驰,且可能加剧对目标服务器的压力。

建议:将 semaphore.release() 移至 fetch 真正 settle(.finally())时执行;或使用 AbortController 在 timeout 时真正终止请求。若设计意图是「分批启动」而非「并发上限」,则应单独实现 rate limiter(令牌桶/漏桶),语义更准确。

2. 失败资源「永久缺失」无恢复机制resource.ts · getResourceByTypes

oldResources.contentType 为空(即之前下载失败的空 Resource 记录),当前逻辑返回 undefined 并跳过——不会再次尝试 fetch。这意味着一次下载失败会导致该资源在缓存中「冻结」为失败状态,用户除非手动清除缓存,否则资源永远无法恢复。

建议:对空 Resource 加入基于 updatetime 的退避重试逻辑(例如失败超过 N 小时后允许重试),或直接调用 updateResource() 触发重新下载。


🟡 建议改善 / Suggested Improvements

3. script.ts 中存在重复调用残留

PR 描述与 commit log 显示已将三次独立调用合并为 updateResourceByTypes,但从 Copilot review 引用的代码片段来看,Promise.all 内同时存在旧的三次 updateResourceByType 调用和新的 updateResourceByTypes 调用——若属实,这会导致资源被重复下载两次。需确认最终 HEAD 已清理干净。

4. 24小时缓存 TTL 与脚本更新频率不匹配

原有 updateTime > Date.now() - 86400_000(24小时内不重新 fetch)的逻辑,在脚本可能每 6 小时更新一次的场景下会导致资源滞后于脚本版本。PR 中双方讨论后决定暂不修改,但这是个已知的设计债,建议后续版本处理(例如:脚本版本号变更时强制刷新资源,忽略 TTL)。

5. 随机延迟(100~150ms)的必要性存疑

在 semaphore 已限制并发数量的情况下,额外的随机 jitter 的作用有限——5 个 slot 的请求会同时发出,只有第 6 个起才会排队等待。如果目的是错开请求时间,jitter 应施加在 acquire 之前(排队阶段),而非 acquire 之后。


🟢 可选优化 / Optional Enhancements

6. withTimeoutNotifydone 字段语义冗余

.then().catch()done 均被设置为 true,而回调函数内已可通过 timeoutederr 推断状态。done 字段可简化,或改名为 settled(语义更精准)。

7. 并发上限 5 硬编码

0b1ded9 中已将魔法数字提取为 MAX_CONCURRENT_FETCHES 常量,但常量目前仍写死在 concurrency-control.ts 内部。如果未来不同类型的资源需要不同的并发策略,建议通过构造参数传入,而非全局单例。


✅ 优点 / Strengths

  • 统一入口:移除 loadNow 分支后,资源获取路径单一,代码可维护性明显提升,reviewer 无需追踪两套执行路径。
  • 类型批量接口getResourceByTypes / updateResourceByTypes):消除了三处重复调用,语义清晰,调用方代码更简洁。
  • 并发控制独立模块化concurrency-control.ts 是通用工具,与业务代码解耦,未来可复用。
  • 测试补充及时:Copilot 指出缺测试 → CodFrm 同次 review 即补充单元测试,响应速度好。
  • 注释质量提升contentType 语义注释、常量命名(MAX_CONCURRENT_FETCHES 等)均让代码意图更自文档化。
  • 设计讨论留记:PR 内 资源失败不影响脚本保存24小时 TTL 的设计取舍均有明确的对话记录,便于后续维护者理解决策背景。

📊 总体评估 / Overall Assessment

建议: REQUEST_CHANGES

理由: 核心架构方向(统一入口 + 并发控制)是正确且值得合并的。但 Semaphore 提前释放这个问题从根本上破坏了该 PR 的主要目标——限流,且在高延迟网络下会产生比修改前更差的行为;失败资源永久缺失问题则是一个静默的可靠性回归。这两个问题解决后,PR 整体质量良好,可以批准合并。

⚠ 此 review 基于 PR 描述、commit log 及 GitHub 渲染的 diff 片段,非 raw diff 全量分析。建议提供 GitHub token 以获得完整行级别审查。

@cyfung1031 cyfung1031 marked this pull request as draft April 7, 2026 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants