fix(app-shell): give missing-resource retirement a bounded recovery, so one transient 404 stops killing both inbox panels for the page (#4289) - #4360
Merged
Conversation
…4289) `sharedUserFeeds.markUnavailable()` was a one-way door — `refresh`, `schedule` and `onVisibilityChange` all returned early on `unavailable`, so one missing-resource answer retired the feed until reload or an identity change. Since #4225 pointed the bell and Home's action centre at one inbox feed, that took both panels together. A retired feed now re-probes at most UNAVAILABLE_PROBE_LIMIT (3) times, no more often than UNAVAILABLE_PROBE_MS (60s), on the timer and on visibilitychange alike; a probe that answers with rows revives the feed and restores its cadence. The retired STATE is unchanged: status stays `ready`, value stays empty, no error is ever rendered (#4315). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
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
2 participants
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 #4289
The premise moved, and it closed in the wrong direction
#4289 was filed against
AppHeader's own inbox poll: anotificationsUnavailableRefthat latched on one 404 /OBJECT_NOT_FOUNDand was cleared by nothing, whileuseHomeInbox— reading the same object with the same filter — simply retried. The card was explicit that reachability of a transient 404 was unverified, and named that as step one for whoever picked it up.That poll no longer exists. #4225 / PR #4327 deleted it and pointed both surfaces at
sharedUserFeeds' inbox feed. So the asymmetry the card describes is gone — but it closed in the wrong direction: the surviving reader is the latching one, and the sibling that used to retry is now a consumer of it.Measured on
origin/main@de627792e,markUnavailable()is still a one-way door.unavailableis set, andrefresh(),schedule()andonVisibilityChange()all return early on it, so the only exits are a key change (a different user or adapter) or a page reload. A consumer remounting does not re-arm it; returning to the tab does not; a later successful read never happens because no later read is issued.The blast radius grew rather than shrank. One missing-resource answer now takes the bell and Home's action centre together — which also removes the "Home's card still works" contrast that made the #4110 / #4230 signature diagnosable the first two times.
Why a transient 404 is reachable — the predicate is STATUS-shaped
The reachability question the card left open does not need a server race to answer.
isMissingResourceisand its two status arms are already pinned true with no error code at all —
sharedUserFeeds.isMissingResource.test.ts, "is true for a plainstatus404 as well". On the producer side,@objectstack/clientstampserror.httpStatus = res.statuson every non-ok response, before it inspects the body (packages/client/src/index.ts).So the 404 population reaching this seam is not one population but two, and they are indistinguishable here:
assertObjectRegisteredthrowsOBJECT_NOT_FOUND+ 404 for a community build with noservice-messagingready, empty, "You're all caught up", no error line/api/v1/data/...before the API is mountedOnly the first is the answer #4315 ruled on. The second used to be given the first's treatment, permanently.
The change
packages/app-shell/src/hooks/sharedUserFeeds.tsonly. Retirement keeps its meaning and gains a bound:UNAVAILABLE_PROBE_LIMIT(3) times, no more often thanUNAVAILABLE_PROBE_MS(60s);visibilitychangealike, so tab-flipping cannot spend the budget faster than the clock;error, so a 404-then-500 sequence never starts telling a community build its inbox is broken;schedule()'s retired branch is not gated onpollMs > 0, so the non-polledsys_activityfeed — whose only other re-read point was a consumer attaching, and which the retirement silenced just as permanently — gets the same bounded recovery.Cost on a deployment that genuinely has no messaging pipeline: 3 extra reads across the whole page lifetime, then silence — against the ~360/hour an un-retired 10s poll would issue. The retired state is untouched: status stays
ready, the value stays empty, no error is ever rendered.Pins, and what the reverse verification measured
New:
packages/app-shell/src/hooks/__tests__/sharedInboxFeed.transient404.test.tsx(8 cases).Directions were predicted in the file header before running, and one of them is not the template's shape — the ceiling case fails downward on
main(main issues fewer reads than the bound, not more), which is why it is written as an exact count rather than a<=.Direction 1 — the fix absent (constants present, behaviour not): 6 red / 2 green, exactly the predicted split.
Direction 2 — the retirement deleted (
markUnavailablerouted tomarkFailed), to prove the CONTROL is not vacuous:So the ceiling is a two-sided pin: it can fail upward (retry storm) and downward (no probes at all), and the #4315 control turns red the moment "404 is an answer" stops holding.
Amended pins
None. The #4315 ruling is asserted, not amended:
HomeActionCenter.unansweredInbox.test.tsx's "a deployment with no inbox object at all is an ANSWER, not an error" andsharedInboxFeed.twoSurfaces.test.tsx's "still treats a MISSING inbox object as an answer, for both surfaces" both stay green with their assertions untouched, because the probes are invisible at those surfaces by design — they publish nothing. The new file's CONTROL case restates the same rule one layer down, at the store, where the probe count is observable.Verification
pnpm exec vitest run packages/app-shell/— 350 files, 3339 passed | 1 skippedAppHeader.inboxVariant, fix(app-shell): Home's action centre needs an ANSWER before it says "all caught up" (#4235) #4315HomeActionCenter.unansweredInbox, fix(app-shell): one inbox feed for the bell and Home — already-read messages stop counting as "needs your attention" #4327sharedInboxFeed.twoSurfaces, fix(app-shell): Home's action centre badges the full unread count, not its capped list #4348InboxPopover.badgeBreakdown+inboxGrouping, plussharedUserFeeds.isMissingResource— 69 passedpnpm --filter @object-ui/app-shell type-check(bothtsc --noEmitandtsconfig.typetests.json) andeslinton the changed files — cleannode scripts/check-changeset-presence.mjs,check-changeset-no-major.mjs,check-changeset-fixed.mjs,check-control-bytes.mjs— all greenRelated: #4225, #4230, #4315, #4327, #4110.
Generated by Claude Code