fix(metadata): 坏文件不再被当作 data: null 广播 —— handleFileEvent 改走 loadDiagnosed (#5228) - #5626
Merged
Merged
Conversation
`NodeMetadataManager.handleFileEvent()` guarded its re-read with a
`try/catch` that was unreachable for loader read/parse failures: `load()`
is `(await loadDiagnosed(...)).data`, and `loadDiagnosed` (ADR-0110 D3)
absorbs a loader throw into `{ data: null, degraded: true }` instead of
rethrowing. `FilesystemLoader.load()` does throw on an unparseable file,
but the throw died one frame below the handler — so the `catch` never
ran, its `logger.error` never printed once, and the event went out
carrying `data: null`: the wire shape of "this metadata legitimately
holds nothing". The miss/outage distinction ADR-0110 D3 exists to keep
was erased at the one call site using the variant that discards it.
Read through `loadDiagnosed` and split on `degraded`. An outage takes
the road the dead `catch` meant to take (log `filePath`, type, name and
`errors[]`; announce nothing). A clean miss and every `deleted` event
keep their existing semantics.
The invalidation moves ABOVE the read, so the read's verdict can never
decide whether the caches drop — #5218's contract survives the early
return, and the `api` endpoint index still rebuilds through
`invalidateListCache`, its first seam (#5089), without the broadcast.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V7WetGmnfoXNn8cLieKKmx
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 7 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
Contributor
Author
|
CI 红说明(engine-core 车道 PM):ESLint job 唯一失败点是 Generated by Claude Code |
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
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.
Fixes #5228
注:GitHub 的正文消毒器会把裸的单引号与尖括号在存储时转义,下面刻意避开了这两类字符,测试证据段落里的引号一律改用「」,箭头一律用 →,免得关键证据被存成实体后读不出来。
一、前提核验(先于实现)
issue 的前提在合并后的
origin/main(e0b2ea7,已含同日的 #5436)上 仍然成立,三处 file:line 证据:packages/metadata/src/metadata-manager.ts:2196——load()的全部实现就是return (await this.loadDiagnosed(type, name, options)).data;packages/metadata/src/metadata-manager.ts:2221-2233——loadDiagnosed()的循环体整体包在try/catch里:loader 抛出的异常被errors.push(...)记账、logger.warn一条,然后返回{ data: null, degraded: errors.length 非零, errors },不重抛。packages/metadata/src/loaders/filesystem-loader.ts:137-144——FilesystemLoader.load()对坏 JSON 确实先logger.error再throw error。串起来即 issue 所述:坏文件的异常在
loadDiagnosed一层就被吃掉,handleFileEvent的catch永不进入、return永不执行、那句logger.error(Failed to load changed file)一次都没打印过,事件带着data: null广播出去。而data: null恰是「这份元数据合法地什么都没有」的线上形状 —— ADR-0110 D3 要分开的 miss 与 outage,在这个调用点被抹平成同一个形状。反向验证也把它钉死了:把源码退回改动前的版本,本 PR 新增的三条用例立刻变红(见第四节)。
二、修法(PM 裁决的方向 A)
handleFileEvent改用loadDiagnosed(),按degraded分三路:degraded === true(有 loader 抛异常且无人答出)catch本来要走的路:logger.error带上filePath/metadataType/name/loadDiagnosed的errors[],不广播data: null且未 degraded(文件已不在,或合法为空)data: nulldata保持undefineddata的静态类型顺手从any收到unknown,与MetadataWatchEventSchema里的data: z.unknown().optional()对齐。三、失效动作与广播动作的先后(#5218 不回退)
PM 特别点名的风险:提前
return不得把缓存失效一起跳过。现状代码里invalidateForForeignWrite()排在读之后、广播之前,所以「读完发现 degraded 就 return」的朴素改法会连失效一起吞掉。因此本 PR 把
invalidateForForeignWrite(type, name)上移到读之前:失效是无条件的,广播才是有条件的,读的结论再也决定不了缓存掉不掉。这一步是安全的 ——loadDiagnosed只走 loader,既不读也不写listCache/registry。这样两件事同时成立:
listCache——handleFileEvent只通知 watcher(#5109 的本地同形缺陷) #5218 的回归用例「still invalidates when the changed file cannot be parsed」保持绿;api端点索引在没有广播的情况下依然重建 ——invalidateListCache对 api 类型的那一支是该索引的第一条失效缝(E2(#5040 执行器):端点匹配器 —— 惰性索引 + 元数据事件失效,精确路径匹配,params恒空 #5089,本来就负责覆盖{ notify: false }的写入),构造函数里subscribe注册的那条缝被跳过不产生任何代价。这条由新增用例直接钉住。四、测试与反向验证
新增
packages/metadata/src/node-metadata-manager-degraded-file-event.test.ts(6 例),并订正 #5218 用例上那段已经过时的注释(它当时写的是「事件会带data: null广播出去,这是另一个单独的问题」)。反向验证的方向是事先预判后跑的,而且跑了两个方向,因为两种错法的红集合是不相交的:
方向一 —— 源码退回改动前(预判:两条「不广播」用例 + 日志可达性用例变红,实测一致):
干净 miss / 可读文件 / deleted 三例在改动前后都是绿的 —— 它们本就不是判别项,是「没被顺手改坏」的保护栏,如实记在用例注释里。
方向二 —— 朴素改法(先读、degraded 就 return,失效留在广播前),预判:#5218 自己的回归用例会红,实测一致:
也就是说:只有「失效在前、广播有条件」的实现能让 17 条全绿。注意 #5218 那条用例对本次改动本身不具判别力(改动前后都绿),这一点已写进它的注释里,免得下一个读者误以为它在守 #5228。
绿色结果:
pnpm check:type-check-coverage通过;packages/metadata的 tsc 原始报错数在改动前后都是 92(即本次净增 0,新增测试文件自身 0 报错);check-nul-bytes通过,并对四个改动文件做了超出门禁扫描面的自扫(含 DEL),无控制字节。五、消费者核验
notifyWatchers的仓内订阅者共 4 处,逐个核对「degraded 不广播」是否让它们的失效/重载逻辑失效:event.data吗metadata-manager.ts:391(api →endpointMatcher.invalidate())invalidateListCache这条缝覆盖,新增用例直接钉住objectql/src/plugin.ts:724(object)get()重读)plugin-email/src/email-plugin.ts:919(email_template)event?.data ?? await get(...)null是 nullish,原本就落到get(),同样拿到 null 后 returnmetadata/src/routes/hmr-routes.ts:98→ Studio SSE相关套件全绿:
@objectstack/plugin-email20 files / 297 tests;@objectstack/objectql的plugin.integration/plugin.step2/metadata-facade/metadata-diagnostics4 files / 61 tests。需要维护者知情的那一处行为变化
hmr-routes把事件当作纯粹的「有东西变了,去重读」信号转给浏览器,它不读data;而浏览器侧的缓存不在invalidateForForeignWrite的覆盖范围内。所以:一个被改坏且一直没修好的文件,现在不再唤醒 Studio,左栏会继续显示上一次的好定义,直到下一个事件或手动刷新;改动前 Studio 会被唤醒、重读、看着这个视图消失。我判断这不构成 needs_decision,理由三条,但把它显式摆出来供否决:
added+ 坏文件这个绝大多数场景下,list()前后本来就没变,唤醒本身零信息;logger.error(带errors[]),被告知「文件读不出来」这件事本身,比看着视图无声消失更接近真相。若维护者更看重那条唤醒信号,最小改法是 degraded 时仍广播、但带
data: undefined(即applyRepoEvent已经在用的那个既有形状,不是新造第三种),约五行;按 PM 裁决与 issue 方向 A,本 PR 没有自行这么做。