fix(dashboard): survive unavailable browser storage - #4159
Green-hats wants to merge 1 commit into
Conversation
Signed-off-by: Green hats <183933879+Green-hats@users.noreply.github.com>
huangruiteng
left a comment
There was a problem hiding this comment.
评审绑定 exact head:9e7599b0b5811c89495c24a963b0452384f67c5e。
动机
这个 PR 修复的是一个真实的浏览器能力边界:loadStatusSourceCatalog() 虽然会捕获 getItem 异常,但调用方在进入函数之前就要先求值 window.localStorage。在隐私限制或 sandbox 环境里,如果 property getter 本身抛错,Dashboard 会在初始化阶段失败,连内置的 Local source 都无法显示。
改动思路
改动把异常边界放在已有 owner DashboardPage 的 catalog initializer:只有成功取得 storage 后才调用原 loader,否则复用 emptyStatusSourceCatalog()。现有 persistStatusSourceCatalog 也继续负责写入失败,因而 working-storage 路径完全不变,受限环境则明确降级为当前 session 内可用、reload 后不保留新增 source。
具体改动
dashboard-page.tsx给window.localStorage的取得与 catalog 加载增加同一层保护,不引入第二套状态或 storage abstraction。- 现有 Playwright source-switch smoke 增加 property getter denied 与 storage methods denied 两组真实浏览器场景,验证 Local source 渲染、session 内添加/切换 remote source、reload 后回退,以及没有 uncaught page error。
- 同步重建 packaged Chat 资源。我扫描了 dashboard 中其余 localStorage 调用,它们各自已有异常边界;这次不需要扩成通用 storage framework。
对主干的风险
行为证据是充分的:npm run build、npm run smoke:status-sources、独立端口上的 dev 与 packaged 浏览器 smoke、git diff --check 均通过;同一 head 测试 harness 跑在 base 081ff998f 时,会在 getter-denied 场景等待 Local Goal Only 超时,而本 head 全部通过。
但当前 exact head 还不能进入主干。GitHub 报告 CONFLICTING/DIRTY,本地 git merge-tree --write-tree origin/main HEAD 也只在 loopx/web/chat/index.html 发现冲突。手写 TypeScript source 可以干净合并,冲突来自 stale generated asset hash;手工挑一边可能让 packaged /chat 引用错误 bundle。
我的整体评价
结论是 REQUEST_CHANGES,唯一阻塞项是:rebase 当前 main,从合并后的 source 重新 build packaged Chat 资源,再跑一次 build、dev/packaged status-source browser smoke 与干净 merge-tree。源码修复本身小而正确,真实 failure path 与 session-only fallback 也已经被 mutation-sensitive 地验证;不需要扩大产品范围。
English verdict: REQUEST_CHANGES — exact head 9e7599b0b5811c89495c24a963b0452384f67c5e fixes the pre-call window.localStorage getter failure and passes real dev/packaged browser coverage, but it must rebase current main and regenerate the packaged Chat index/assets because the exact head is currently conflicting.
Accessing
window.localStoragecan throw before the status-source loader's error handler runs, preventing the Dashboard from rendering. Catch that access during initialization and fall back to the existing local source catalog. Sources added while storage is unavailable remain usable for the current page session.The browser regression covers denied storage access, throwing storage methods, local startup and reload, and adding a read-only remote source. Existing persisted-source and request-ordering checks still pass. Includes the rebuilt packaged Dashboard.
Validation:
npm run buildandnpm run smoke:status-sourcespassed.smoke:status-source-switch-browserandsmoke:status-source-switch-packagedpassed.loopx canary premerge --from-git-diff --format jsonpassed: six catalog checks, public-boundary scan, and diff hygiene checks; no manual holds.