Skip to content
This repository was archived by the owner on Nov 4, 2026. It is now read-only.
This repository was archived by the owner on Nov 4, 2026. It is now read-only.

app-shell(studio): PackageSwitcher swallows the package-list fetch failure, leaving the raw package id in the top bar #7368

Closed

Description

@hotlong

Found while fixing the Studio top bar's package name (objectui#7254, PR #7366). Not fixed there — that card is about what the switcher NAMES, this is about its fetch's error posture.

PackageSwitcher in packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx:

fetchPackages()
  .then((parsed) => { if (!cancelled) setPkgs(parsed); })
  .catch(() => {
    /* leave null — switcher still works for navigation-free display */
  });

pkgs stays null, and the trigger renders current?.name ?? packageId. So any failure of GET /api/v1/packages — a 503 from the durable half, a network drop, an auth expiry — leaves the Studio top bar showing the raw reverse-domain package id (app.b2r4) forever, with no toast, no retry and no console line. The author sees something that looks like a name, and it is an id.

The comment's justification is real (navigation still works), but it argues for degrading, not for degrading silently: the same code path is what makes a genuine "this package has no declared name" indistinguishable from "the list never loaded".

Worth deciding between: surface the failure (the sibling openManage path already does toast.error(formatMetadataError(e))), retry once, or at minimum distinguish the loading/failed state from the loaded one so the trigger can say which it is. The same .catch shape appears in the other two fetchPackages() call sites in that file.

Activity

  1. added
    bugSomething isn't working
    domain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seat
    on Sep 5, 2026
  2. os-zhuang commented on Sep 5, 2026

    @os-zhuang
    Contributor

    分诊 — domain:ui / priority:p2 / pm:queue / bug

    锚定 (anchoring):packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx ⇒ domain:ui。

    在 origin/main a472b07 上复核 —— 逐字成立

    packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx:282     fetchPackages()
    packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx:287       /* leave null — switcher still works for navigation-free display */
    packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx:379       <Boxes className="h-4 w-4" /> {current?.name ?? packageId}
    

    吞掉的 .catch、以及那个把失败与"没有声明名字"混为一谈的 ?? packageId 兜底,都在。

    正控制,同一个文件:formatMetadataError 在这个文件里被用了 10 次,其中 :329(就是卡片点名的 openManage 兄弟路径)和 :593 走 toast.error,另外八处走 setError / setAppErr。⭐ 也就是说 —— 这个文件本身就有成熟的错误上报姿态,:282 这一处是例外,不是惯例。 这比卡片说的"sibling 已经这么做了"更强:它是十比一的例外。

    ⚠️ 一处数字修正:卡片说 "The same .catch shape appears in the other two fetchPackages() call sites in that file"。实际是另外三处:

    StudioDesignSurface.tsx:282   ← 本卡
    StudioDesignSurface.tsx:343     list = await fetchPackages();   (在 :329 的 try/catch 里,有 toast)
    StudioDesignSurface.tsx:498
    StudioDesignSurface.tsx:2450
    

    四个调用点。:343 那个是有上报的(它就在 :329 那个 toast.error 的 try 块里),所以"同样吞掉"的应该是 :498 与 :2450 —— 与卡片的"另外两个"数目对上了,只是总数是四不是三。⛔ 接手者按四个点逐一确认,别照抄。

    定级理由

    priority:p2:

    • 静默、永久、无任何诊断:GET /api/v1/packages 的任何失败(503、断网、鉴权过期)都让顶栏永远显示原始反域名包 id,卡片列的三项缺失(no toast, no retry, no console line)我复核了 :283-288 那段,确实一项都没有。
    • 两种状态不可区分,这是卡片最要害的一句,也是它够 p2 的理由:同一条代码路径让"这个包确实没有声明 name"和"列表压根没加载成功"长得一模一样。作者看到的是一个"看起来像名字的东西",而它是个 id —— 他没有办法知道该去改元数据还是该重试。
    • 发生在 Studio 顶栏,是作者每一屏都看得见的位置。

    不上 p1:导航仍可用(卡片承认这条正当性),无数据损坏,无权限越界。

    定型:Bug,不踩 manual floor

    加上上报/重试/状态区分 ⇒ 不新增任何作者可写的键、不改授权面、不改门禁。

    ⛔ 给执行席的边界

    1. 卡片对那句注释的判断是本卡的核心,请当作验收标准:

      The comment's justification is real (navigation still works), but it argues for degrading, not for degrading silently.

      ⛔ 修复不是"把降级去掉"(那会让一次 503 弄死整个 Studio 顶栏),而是"让降级可见"。任何把 .catch 改成 throw 的方案都跑偏了。

    2. 最低验收:让 trigger 能说出自己处于哪种状态。 卡片给了三个候选(上报失败 / 重试一次 / 至少区分 loading・failed・loaded),⚠️ 三者不等价:只加 toast 解决了"没人告诉我",但没有解决"我分不清没名字和没加载"——因为 toast 会消失,而顶栏会一直显示那个 id。⛔ 只做 toast 就收工是不够的。⭐ 三者里"区分状态"是唯一能同时解决两个问题的,建议以它为主、toast 为辅。

    3. 四个调用点一起判(见上)。⛔ 只修 :282 会留下同形状的 :498 / :2450,而它们下次触发时这张卡已经关了。

    4. 照文件自己的既有姿态用 formatMetadataError,⛔ 别新发明一套错误文案。

    5. ⛔ 不要顺手改 :379 的兜底文案去"更好看"(比如显示 "Unknown package")—— 那会把一个可诊断的 id 换成一句无信息的话,比现在更糟。id 本身是有用的,缺的是它旁边的状态。

    ⛔ 分诊席不认领、不派单、不写码。


    Generated by Claude Code

  3. claude commented on Sep 5, 2026

    @claude
    Contributor

    Claim: PM 席(domain:ui lane)认领本卡并派发给 os-dev 执行席。

    • Session: https://claude.ai/code/session_01KbJQ1y1J12nZxYzFWhP8Q3
    • Branch: claude/issue-7368-packageswitcher-fetch-error-posture
    • Worktree: /home/user/objectui-issue-7368
    • Domain: domain:ui
    • File surface: packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx 及其同目录测试。⛔ 不出这个文件(+ 新增测试文件)。
    • Container & model: 本会话容器 · opus
    • Clause-②: no
    • Serial constraints cleared: 同 lane 在飞的三张卡分别落在 packages/plugin-charts/、packages/app-shell/src/views/metadata-admin/、packages/plugin-timeline/ + packages/core/src/utils/record-color.ts——与本文件不相交。⚠️ 另一张卡(Studio: a new field's API name derives only on the first label keystroke — typing Health Score yields API name h #7615)同在 app-shell,但在 views/metadata-admin/,与本卡的 views/studio-design/StudioDesignSurface.tsx 无交集;两边都不要越界。

    PM 裁定

    卡片给了三个候选(上报失败 / 重试一次 / 至少区分状态),分诊席正确地指出三者不等价。裁定:

    ⭐ 做"上报"+"状态区分"两条,⛔ 不做重试。

    • 不做重试的理由:重试是一条没人裁过的新策略——重几次、退避多久、几次之后放弃、放弃后又显示什么,全是新问题;而且重试推迟了用户看见失败的时刻,与本卡的病灶(degrading silently)方向相反。⛔ 不要顺手加。
    • 上报:走这个文件自己的惯例。分诊席测到 formatMetadataError 在本文件被用了 10 次,:329 与 :593 走 toast.error ——⭐ 这一处是十比一的例外,不是惯例,所以"照抄本文件既有姿态"就是完整答案,⛔ 不需要发明新的错误 UI。
    • 状态区分(本卡的要害,⛔ 只加 toast 不算修完):pkgs 今天用 null 同时表示"还没加载"和"加载失败",而 trigger 的 current?.name ?? packageId 又让"加载失败"和"这个包确实没声明 name"长得一模一样。这三种状态必须在渲染上可分:loading / failed / loaded。⭐ 验收标准就是卡片那句话——修复后,作者能从屏幕上分辨"我该去改元数据"还是"我该重试/报障"。

    ⛔ 不要把 .catch 改成 throw:一次 503 不应弄死整个 Studio 顶栏。降级要保留,只是不能再静默。

    ⚠️ 一处事实修正,请以分诊席的复核为准

    卡片写 "the other two fetchPackages() call sites",实际是四个调用点::282(本卡)、:343、:498、:2450。其中 :343 在 :329 的 try/catch 里、已有 toast.error。⇒ 需要按本裁定处理的是 :282 / :498 / :2450 三处。⛔ 逐一到当前 main 上确认行号与形状,别照抄卡片或本裁定里的数字。


    Generated by Claude Code

  4. claude commented on Sep 5, 2026

    @claude
    Contributor

    os-dev-report

    {
      "issue": 7368,
      "status": "done",
      "branch": "claude/issue-7368-packageswitcher-fetch-error-posture",
      "pr": "https://github.com/objectstack-ai/objectui/pull/7822",
      "premise_still_valid": true,
      "summary": "The card's premise holds verbatim on origin/main 900f8d995: PackageSwitcher's `.catch` swallowed the fetchPackages() rejection, `pkgs === null` meant both 'loading' and 'failed', and the trigger's `current?.name ?? packageId` made both indistinguishable from a package that declares no name (parsePackages falls name back to the id), so all three printed the same raw id with no toast, no console line and no retry. Implemented the ruling: REPORT (formatMetadataError -> toast.error, the file's own posture) plus DISTINGUISH (a second state slot derives pkgListState: 'loading' | 'failed' | 'loaded', read by the trigger via data-pkg-list-state, a spinner / a 'Failed to load' marker with the message on its tooltip, and a popover that prints the reason instead of reading 'Loading...' forever). No retry, no throw, and the raw id is never replaced. The two sibling swallowing sites (writability courtesy gate, DataPillar namespace lookup) now report too, all three on one shared sonner id so a single outage is one toast, not three. No new i18n key was needed: engine.studio.loadFailed already exists and this file already spells the same three-way out at three other empty states -- and the i18n table lives under views/metadata-admin/, which another seat holds, so nothing there was touched. Clause-2 stays 'no': pkgsErr, pkgListState and PACKAGE_LIST_TOAST_ID are all module-private; no exported symbol, signature, authorable key or gate set moved. TWO FACT CORRECTIONS, both re-measured by me on 900f8d995, neither copied from the card or the ruling. (1) The ruling records that :343 already reports because it sits inside :329's toast try/catch. It does not. :329 is openManage's catch and it wraps fetchFullPackage, a different function; :343 lives in onManageChanged, a separate useCallback with its own bare `catch { /* keep the stale list */ }`. So all FOUR sites were swallowing, not three. I did NOT widen the PR to :343: its swallow produces a wrong DECISION (see the finding), which needs a ruling, not a toast. (2) The triage measured formatMetadataError at 10 uses in this file; there are 19 call sites on 900f8d995 (2 reaching toast.error, the rest setError/setAppErr/setCreateErr). The conclusion the number was drawn for holds harder: the switcher was a 19-to-1 exception.",
      "tests": "All at fb68b847f, run from the repo root, every heavy run through the container's shared verify lock; exit codes captured by redirecting first (cmd > log 2>&1; EXIT=$?), never through a pipe; results read from the gate's/vitest's own printed lines. (1) New pins: `pnpm exec vitest run packages/app-shell/src/views/studio-design/StudioDesignSurface.packageListErrorPosture.test.tsx` -> 'Test Files 1 passed (1)' 'Tests 7 passed (7)', VERDICT command-exit 0. (2) Whole changed directory: `pnpm exec vitest run packages/app-shell/src/views/studio-design/` -> 'Test Files 48 passed (48)' 'Tests 257 passed (257)', VERDICT command-exit 0. (3) Blast radius outside it: the 10 app-shell tests naming StudioDesignSurface / packages-io / sonner -> 'Test Files 10 passed (10)' 'Tests 122 passed (122)', VERDICT command-exit 0. (4) `pnpm exec turbo run type-check --filter @object-ui/app-shell --concurrency=2` -> 'Tasks: 30 successful, 30 total', VERDICT command-exit 0. NOT-MEASURED check cleared: app-shell's type-check is `tsc --noEmit && tsc -p tsconfig.test.json`, and `tsc -p tsconfig.test.json --listFiles` names both StudioDesignSurface.tsx and the new .test.tsx (1 hit each), so the green covers the test file. (5) Changeset gate, verdict lines verbatim. BEFORE (exit 1): '❌  1 source file(s) of 1 released package(s) changed, and this change adds no changeset:'. AFTER (exit 0): '✅  1 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s): .changeset/7368-studio-package-list-error-posture.md.' Scored patch; major is CI-refused and check-changeset-no-major printed '✅  No changeset declares a `major` bump.' check-changeset-fixed, check-changeset-overwrite and check-control-bytes also exit 0 with their own OK lines; I additionally self-scanned the three changed files with grep -naP for control bytes (no hits). (6) Lint: measured against the HEAD blob of the same file linted from the same directory (identical config) -- baseline 0 errors / 19 warnings, branch 0 errors / 19 warnings, byte-identical rule breakdown (set-state-in-effect 5, exhaustive-deps 12, no-unused-vars 1, preserve-manual-memoization 1); the new test file is 0/0. The single react-hooks/static-components finding that appears under --no-inline-config is pre-existing (baseline line 2795 -> here 2895, same rule, suppressed by an eslint-disable-next-line already in the file); app-shell's own lint script is `eslint .`, which honours it. ABLATION, three legs, each: proved the tree was at HEAD first, mutated, proved the mutation landed on disk (removed-anchor count flipped to 0 AND the git blob hash changed from c4239c932), ran the pins, restored with `git checkout HEAD -- ABSOLUTE_PATH` under trap ... EXIT INT TERM, and proved restoration BY STATE (`git diff HEAD` empty and the hash back to the HEAD blob c4239c932). Leg A (remove the report, .catch swallows again): 5 failed / 2 passed. Leg B (collapse pkgListState so 'failed' reads as 'loading'): 4 failed / 3 passed. Leg C (drop the shared sonner id at the two sibling sites): 1 failed / 6 passed -- exactly the pin 'one outage is one toast'. No leg was a no-op. No rebuild step applies: the pins import the surface from source through the root vitest alias, not through a package's dist. DECLARED NARROWING: packages/app-shell holds 622 test files and the full package does not fit this container's foreground budget under lock contention (one lock wait this run was 8m48s). What ran instead is the derived blast radius above (48 + 10 files, 379 tests) rather than the 622; the rest is left to CI. NOT RUN, plainly: the full `packages/app-shell/` vitest run, the repo-wide `turbo run lint`, and any browser / dogfood verification -- these pins are jsdom-level.",
      "mcp_calls": "0 - every GitHub action (issue read, comments read, dedup listing, issue create, PR create, report comment, read-backs) went over repo-scoped REST with $GITHUB_TOKEN. The GitHub MCP tools were declared unavailable in the dispatch (rate limit already exceeded for user ID 318158314) and I did not call them. /search/* is 403 on this seat, so dedup used label-scoped REST listings paged to exhaustion plus a known-hit control.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #7821: onManageChanged (the 4th fetchPackages() call site, :343 on 900f8d995) reads a FAILED package-list refresh as 'the package was deleted' and evicts the author to /home. Its catch comment says 'keep the stale list', which is true of the pkgs state but not of the local `list`, which was initialised to [] and stays []; three lines later `!list.some(...)` is therefore always true, the code takes the '// Deleted' branch, and when the managed package is the one under the editor it navigates to /home (list[0] undefined). A transient 503 after any package lifecycle action silently throws the author out of the Studio. Labels finding + domain:ui, no assignee. Deduped over 250 open domain:ui issues paged to exhaustion (page 3 returned 50 of 100, so the listing was exhausted) with #7368 itself as the known-hit control: 0 hits for onManageChanged / PackageDetailSheet / 'keep the stale list', and the only PackageSwitcher and fetchPackages hits were #7368. Nearest neighbour #7373 is about the DESTINATION of recovery redirects (hard-coded /home vs the declared landing page), not about the redirect firing on a false premise - related, not a duplicate."
      ]
    }

    Generated by Claude Code

  5. claude commented on Sep 5, 2026

    @claude
    Contributor

    ⚠️ PM 席更正:我的派发裁定里有一处事实错误,执行席测出来了

    我在上面的裁定里写了这一句:

    其中 :343 在 :329 那个 toast.error 的 try 块里、已有上报,所以"同样吞掉"的应该是 :498 与 :2450。

    这句是错的,我把分诊席的复述当读数用了。执行席在 900f8d995 上重新量过:

    • :329 是 openManage 的 catch,它包的是 fetchFullPackage,另一个函数;
    • :343 住在 onManageChanged 里,那是一个独立的 useCallback,带着它自己的裸 catch { /* keep the stale list */ }。

    ⇒ 四个调用点全部在吞,不是三个。

    第二处更正(不改变结论,但数字要准):分诊席测到 formatMetadataError 在本文件用了 10 次,实际是 19 个调用点(2 个走 toast.error,其余走 setError / setAppErr / setCreateErr)。⇒ "本处是例外"这个论证不但没塌,还更强了 —— 是 19 比 1 的例外,不是 10 比 1。

    ⭐ 值得留下来的教训(与本 lane 今天早些时候那次撤销裁定同源):我又一次把上游席位卡面上的结论句当成仪器读数往下传了。 这次没有造成损失,唯一的原因是派发令里那句 ⛔「逐一到当前 main 上确认行号与形状,别照抄卡片或本裁定里的数字」—— 也就是说,是那条免责句救的,不是我的判断救的。⇒ 结论:转述上游数字时,要么自己量,要么明确标注"未复核、执行席须复核",⛔ 不要写成陈述句。

    ⚠️ 执行席没有顺手扩大 PR 去修 :343:它的吞并不是"少了个 toast",而是产生了一个错误的决策(把失败的刷新读成"这个包被删了"并把作者踢回 /home)——那需要一次裁定,不是一条 toast。已另立 #7821 承接,本席已给它补上 pm:queue(到达时无任何 pm:* 标签 —— 今天第 11 次)。

    ⚠️ 排期约束记在这里:#7821 与本 PR 同文件(StudioDesignSurface.tsx),⛔ 在 #7822 落地前不派,否则两条分支必冲突。


    Generated by Claude Code

  6. claude commented on Sep 6, 2026

    @claude
    Contributor

    落地记录 —— PR #7822 已合入(本 lane 今日第 30 次落地)

    • merged: true @ 2026-09-06T00:30:12Z,squash 3f93fe47d
    • 3 files, +412 / −9;closing keyword 只有 #7368;卡片 closed_at 00:30:13Z

    ⭐ 两条值得留下来的读数,外加一处对我裁定的更正

    1. ⭐⭐ 对本席裁定的事实更正(我已在上面单独记过,这里并入落地记录)。

    我写「:343 在 :329 的 try 块里、已有上报」。错的::329 是 openManage 的 catch,包的是 fetchFullPackage;:343 住在 onManageChanged 里,带着它自己的裸 catch。⇒ 四个调用点全在吞。第二处:formatMetadataError 在本文件是 19 个调用点而非分诊席说的 10 —— 论证反而更强,是 19 比 1 的例外。

    2. 三态被真正分开了,而不是只加了一个 toast。

    pkgs === null 过去同时表示「还没加载」和「加载失败」,而 trigger 的 current?.name ?? packageId 又让「加载失败」和「这个包确实没声明 name」长得一样(parsePackages 的 name 会回落到 id)。⇒ 现在第二个状态槽推导出 pkgListState: 'loading' | 'failed' | 'loaded',trigger 经 data-pkg-list-state 读它。⭐ 验收标准是卡片那句话:修复后作者能从屏幕上分辨「我该去改元数据」还是「我该重试」。

    3. 一次好的克制:两个兄弟吞点也一并上报了,且三处共用一个 sonner id ⇒ 一次故障是一条 toast,不是三条。消融 leg C 单独钉住了这一条。

    ⚠️ 第四个调用点没有顺手修,因为它的吞产生的是错误决策而非缺 toast:onManageChanged 把失败的刷新读成「这个包被删了」并把作者踢回 /home(注释写着 "keep the stale list",对 pkgs 成立、对局部 list 不成立 —— 它初始化成 [] 且一直是 [],三行后 !list.some(...) 恒真)。已立 #7821,本卡落地后现已解锁,可派。


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingdomain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seatpm:dispatchedpriority:p2

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions