From 5883cedbf5d30d025f9900422a474e6472100e76 Mon Sep 17 00:00:00 2001 From: Kalvin Chau Date: Tue, 27 Oct 2026 22:05:20 -0700 Subject: [PATCH 1/3] fix: retry transient finite reads before recovery actions bound reads to four attempts with jitter and caller cancellation. retain existing recovery owners, deadlines, and explicit mutation retries. Co-authored-by: am <6e30cd56c30e030cd31bb0939b94a7c257c9a09d5ba2d92cf2735da45629f248@buzz.block.builderlab.xyz> Signed-off-by: Kalvin Chau --- docs/relay-queries.md | 53 ++++ docs/retry-surfaces.md | 85 ++++++ src/app/App.profile.test.tsx | 1 + src/app/ProfileSettings.avatar.test.tsx | 12 +- src/app/ProfileSettings.test.tsx | 12 +- src/app/ProfileSettings.tsx | 36 ++- src/app/shell/SearchResults.test.tsx | 16 +- .../builderlab/adapter-type/Create.test.tsx | 6 +- .../adapter-type/Instructions.test.tsx | 6 +- .../adapter-type/ModelSettings.test.tsx | 14 +- .../builderlab/agents/RemoteAgents.test.tsx | 4 +- src/bundled/builderlab/agents/client.test.ts | 76 +++++- src/bundled/builderlab/agents/client.ts | 31 ++- .../channels/ChannelMembersDialog.test.tsx | 6 +- src/bundled/claude-code/setup.ts | 19 +- src/bundled/codex/index.test.ts | 105 ++++++++ src/bundled/codex/index.ts | 2 +- src/bundled/codex/rpc.test.ts | 22 +- src/bundled/codex/rpc.ts | 4 +- src/bundled/codex/tabs.test.tsx | 95 ++++++- src/bundled/codex/tabs.tsx | 58 +++- src/bundled/github/GitHubBody.test.tsx | 4 +- src/bundled/github/GitHubPanel.test.tsx | 2 +- src/bundled/github/data.test.tsx | 34 ++- src/bundled/github/data.ts | 27 +- src/bundled/github/object.ts | 17 +- src/bundled/goose/tabs.tsx | 5 +- src/bundled/pi/tabs.tsx | 9 +- src/bundled/relay-staff/Feedback.tsx | 17 +- src/bundled/relay-staff/read-retry.test.ts | 74 ++++++ src/bundled/relay-staff/session.tsx | 100 ++++++- src/bundled/voice-notes/VoiceNoteCard.tsx | 18 +- src/features/agent-harness/acp/process.ts | 7 +- src/features/agents/agent-config-history.ts | 30 ++- src/features/agents/library.ts | 3 +- src/features/agents/memory.ts | 5 +- .../channel-members/administration.ts | 3 +- .../ChannelInventory.test.tsx | 79 ++++++ .../channel-navigation/ChannelInventory.tsx | 31 ++- .../channels/usePublicChannelSearch.ts | 35 +-- src/features/communities/CommunityDialog.tsx | 56 ++-- src/features/communities/api.ts | 22 +- src/features/communities/native-api.test.ts | 6 +- .../communities/profile-read-retry.test.ts | 77 ++++++ src/features/messages/MediaAttachment.tsx | 9 +- src/features/messages/VideoPlayer.tsx | 9 +- .../messages/use-media-read-recovery.test.tsx | 105 ++++++++ .../messages/use-media-read-recovery.ts | 91 +++++++ src/features/relay/channel-details.ts | 6 +- src/features/relay/channel-lifecycle.test.ts | 250 +++++++++++++++++- src/features/relay/channel-lifecycle.ts | 106 +++++--- src/features/relay/directory-session.test.ts | 3 +- src/features/relay/emoji-directory.ts | 11 +- src/features/relay/errors.ts | 10 + src/features/relay/gifs.ts | 34 ++- src/features/relay/global-search.test.ts | 8 +- src/features/relay/identity-archives.ts | 15 +- src/features/relay/inbox-feed.ts | 50 ++-- src/features/relay/profile-directory.ts | 70 +++-- src/features/relay/read-retry.test.ts | 156 +++++++++++ src/features/relay/read-retry.ts | 64 +++++ src/features/relay/reminders.ts | 17 +- src/features/relay/session-read-retry.test.ts | 146 ++++++++++ src/features/relay/session.ts | 129 +++++++-- src/features/relay/store.ts | 16 +- src/features/relay/thread-window.test.ts | 6 +- src/features/relay/threads.ts | 20 +- src/features/repos/git-read-retry.test.ts | 92 +++++++ src/features/repos/git.ts | 138 ++++++---- .../hidden-conversation-retry.spec.mjs | 86 ++++++ 70 files changed, 2635 insertions(+), 336 deletions(-) create mode 100644 docs/retry-surfaces.md create mode 100644 src/bundled/relay-staff/read-retry.test.ts create mode 100644 src/features/communities/profile-read-retry.test.ts create mode 100644 src/features/messages/use-media-read-recovery.test.tsx create mode 100644 src/features/messages/use-media-read-recovery.ts create mode 100644 src/features/relay/read-retry.test.ts create mode 100644 src/features/relay/read-retry.ts create mode 100644 src/features/relay/session-read-retry.test.ts create mode 100644 src/features/repos/git-read-retry.test.ts create mode 100644 tests/browser/hidden-conversation-retry.spec.mjs diff --git a/docs/relay-queries.md b/docs/relay-queries.md index 91b01b697e..f518c53293 100644 --- a/docs/relay-queries.md +++ b/docs/relay-queries.md @@ -365,6 +365,59 @@ roster, applied through shared discovery. That makes the channel a sidebar member and enables the composer, which then receives focus. An accepted but unconfirmed join reports that it may have taken effect and stays retryable. +### Finite read recovery + +Finite public reads and retained-view refreshes use one initial attempt and up to +three quiet retries through `read-retry.ts`. This includes search, directory and +profile/catalog consumers, thread and older-history pages, config history, +reminders, repository/Git reads, read-only staff routes and provider discovery. +Callers retain their existing cancellation, access, validation and deadline +guards. Typed transient failures follow the transport's error kinds; caller +cancellation, denied/invalid responses and deterministic local failures do not +retry. Sessions constructed without a transport cannot recover by retrying. + +The first three recovery ceilings are 1/2/4 seconds with equal jitter. Long relay +admission pauses surface to the existing cooldown/action owner rather than +holding a read pending. Exhaustion preserves the original failure and stops a +nested finite caller from starting another budget. It does not change the shared +underlying error taxonomy. + +Internal roster, head, catch-up, preferences and template reads retain their +existing session recovery coordinator. The finite public boundary does not wrap +`readVerified` or `relayRead` internally. Permission reads retired by another +channel's access change reread immediately under their existing deadline, outside +the transient-failure budget. Public reads in backoff are retired on access/cache +changes; a successful response applying its own authority change retains its +original completion guards. + +Media reloads retry only the browser's explicit network-error code. Decode, +unsupported-source and playback-permission failures retain explicit controls. +Reload recovery does not call `play()`. Writes, file saves, interactive prompts, +plugin/crash recovery and ambiguous native setup failures retain their existing +explicit actions. Existing config-sync, usage, avatar, pairing, update and live-reconnect +retry owners are unchanged. + +### Hidden-conversation refresh + +Visibility refreshes retain the last confirmed hidden IDs while loading. One +refresh makes at most four reads: the initial read and three background retries +for transient network/availability failures. Retry ceilings reuse session recovery's +1, 2 and 4 second delays, with equal jitter (50–100% of each ceiling); a relay +admission hint is honored within that ceiling. A longer admission pause surfaces +immediately so the existing cooldown and Retry action remain available. Each read +retains the reader's deadline. Authorization, validation and invalid-response failures surface immediately. +Retry eligibility follows the existing shared error taxonomy, including transient +edge/proxy HTTP errors; status alone does not establish a permanent relay failure. +Exhaustion exposes the existing “Hidden conversations could not be refreshed.” / +“Retry hidden conversations” action; these terminal errors do not opt into the +session's indefinite recovery coordinator. Manual retry or a later completed roster +refresh starts a new bounded refresh. Concurrent triggers share the current refresh, +including its backoff. Sidebar unmount, superseding roster refreshes, session changes, +session-wide cancellation and cache clearing abort obsolete reads and timers; +channel-scoped access invalidation retires the current read and retries within the +same budget. Late results cannot replace current visibility. Lifecycle writes keep their separate confirmation +and explicit-retry rules. + ### Channel links A channel-only link (`buzz://channel/`) shows what the reader may see about diff --git a/docs/retry-surfaces.md b/docs/retry-surfaces.md new file mode 100644 index 0000000000..3ac0f2910d --- /dev/null +++ b/docs/retry-surfaces.md @@ -0,0 +1,85 @@ +# User-facing retry surfaces + +Transient **reads** recover quietly before their existing Retry control appears. +The finite policy is one initial attempt and three retries, with equal jitter +under 1/2/4-second ceilings. Existing background recovery owners remain in place. +See [read recovery policy](relay-queries.md#finite-read-recovery). + +This inventory maps controls to operations; it does not authorize automatic +replay of every action labelled Retry. Authorization, validation, admission +cooldowns, local setup failures and ambiguous native failures remain actionable. +No UI copy or control style changes are required. + +## Finite read owners + +| Controls / consumers | Existing operation owner | Recovery | +| --- | --- | --- | +| Hidden conversations | `relay/channel-lifecycle.ts`, `ChannelInventory.tsx` | Four-attempt refresh; retain confirmed hidden IDs. Scoped invalidation keeps the same budget. | +| Retry channel permissions / Delete check | `relay/channel-lifecycle.ts` | Retry authority reads only. Unrelated shared-read retirement rereads immediately under the same deadline without spending a failure attempt. | +| Channel settings load | `relay/channel-details.ts` | Retry `load`; save and save-confirmation remain separate explicit operations. | +| Channel member/participant/role lists | `channel-members/administration.ts` | Retry `refresh`; membership mutations are single operations. Names use the profile directory. | +| Retry profile / Retry users / member and manager names | `relay/profile-directory.ts` | Retry missing-profile batches, retaining existing heads and coalescing callers. Foreground promotion during backoff changes the next attempt's priority. | +| Profile settings / community profile inspection | `communities/api.ts`, `ProfileSettings.tsx` | Session reads keep their single finite owner; direct-host reads own a bounded retry. Each profile read retains a 12-second bound and a caller signal. Save remains explicit; dispatched confirmation keeps its own bounded lifetime. | +| Retry people / community search / recipient directory | `session.ts` public search and directory boundary | Finite retries; preserve the existing single immediate access-invalidation reread, cache/session cancellation and request deadline. | +| Retry messages in search, including later pages | `session.ts` public `readPage` / `readRemote` | Finite retries at the public consumer boundary. Internal verified readers are single-attempt. | +| Retry channels / channel completion search | `channels/usePublicChannelSearch.ts` | Retry the finite name lookup under its typing/scope controller. The channel store retains the prior useful directory. | +| Agent browse, relay notes, Huddle discussions, profile observations, workflow lists/landing/runs | `session.ts` public reads and `observe().refresh()` | Finite retries; retained views keep loaded data while refreshing and coalesce active refreshes. | +| Retry inbox | `relay/inbox-feed.ts` | Retry addressed history and overlay reads; retain incomplete-target fences and existing access/purge controllers. | +| Retry reminders | `relay/reminders.ts` | Retry the history query; reconnect supersedes its read generation. Reminder create/snooze/complete/cancel remain explicit writes. | +| Retry thread / media comments | `relay/threads.ts` | Retry finite selection, root and page reads under existing controllers. Cached membership does not authorize a network retry. | +| Retry older messages | `relay/store.ts` pagination | Retry continuation reads only; initial/head reads retain session recovery. Loaded rows remain available. | +| Retry emoji / custom emoji catalog | `relay/emoji-directory.ts` | Retry catalog refresh; publishing sets and uploads remain explicit. | +| Retry GIF search | `relay/gifs.ts` | Retry typed network/408/429/5xx read failures; unavailable capability and invalid provider data remain explicit. | +| Retry agents / agent library | `agents/library.ts`, `agents/choices.ts` | Retry the read-only library source. Choices continue to project existing sources rather than introduce another read loop. | +| Retry archive information | `relay/identity-archives.ts` | Retry the visibility snapshot within its existing deadline; invalid or truncated authority is never treated as an empty archive. | +| Retry loading memories | `agents/memory.ts` mounted views | Retry the host read within its existing ten-second lifetime. Decode/permission failures remain explicit. Runtime memory reads keep their separate owner. | +| Retry older config revisions | `agents/agent-config-history.ts` | Retry the requested history page; retain prior revisions on failure. | +| Repository metadata / Retry files / commits | `session.ts` repository read boundary, `repos/git.ts` | Retry immutable metadata/Git reads. Shared browser/native validation still owns destination, signatures, bounds and snapshot parsing. Missing/invalid Git requests remain immediate, including 404 not-found. | +| GitHub panel / PR content | `bundled/github/data.ts`, `object.ts` | Retry public API network/408/5xx and typed GraphQL internal/unavailable failures. Public quota, auth, missing objects and ambiguous CLI setup remain actionable. | +| Staff lists, searches, details and read-only lookups | `bundled/relay-staff/session.tsx` | Explicit read-route allowlist; retry typed intercepted/ambiguous transport failures. Scope cleanup suppresses obsolete results and future requests. | +| Staff attachment preview | `bundled/relay-staff/Feedback.tsx` | Retry reading preview bytes. Native Save remains explicit. | +| Retry models / provider catalogs / read-only history/config tabs | Goose/Pi tab read owners; Claude setup; Codex catalog owner; Builderlab read-route owner | Retry typed transport/timeouts or stopped Pi-host reads. Preserve auth, validation and setup errors. Builderlab limits this to list/catalog endpoints. Codex shared model listing stays single-attempt. Initial turns also await catalog discovery: its retries share a 30-second deadline, and each turn can cancel its own wait without cancelling the shared check. | +| Retry voice note / video resource loading | `messages/use-media-read-recovery.ts` | Browser network-error code 2 gets three reloads after the original failed load. Closing, source changes and voice-note cancellation stop future reloads. No call to `play()` is added. | + +Paths above are relative to `src/features/` unless a bundled/app component is +named. Staff native APIs do not expose in-flight abort: an already-issued request +can finish after cleanup, but its result and further retries are discarded. + +## Existing automatic owners retained + +| Controls / consumers | Owner and disposition | +| --- | --- | +| Retry connection / Retry live updates / Retry now | `relay/service.ts`, live transport and `relay/recovery.ts`; existing indefinite background recovery stays intact. | +| Retry channels in sidebar/profile; initial conversation messages; catch-up heads | `relay/store.ts`, session live recovery; preserve transient state, cooldown and reconnect-triggered repair. | +| Retry templates / teams / sidebar preferences | Channel-kit and preferences recovery sources; no second finite wrapper around their internal reads. | +| Status editor readback | `relay/user-status.ts`; retain existing bounded readback retries and explicit status publication. | +| Retry config/workspace/team reads | `agents/config-sync.ts`; retain its existing bounded retry owner around lookup and current/head reads. Shared artifact/read helpers stay single-attempt. Save and revert remain explicit. | +| Usage refresh / Try again | `bundled/usage/UsagePage.tsx`; retain its existing single 500 ms retry rather than stack another owner. | +| Avatars | Shared Avatar recovery at 2/10/30 seconds; retain that owner. | +| Pairing checks | Existing polling owner; permission/authentication and pairing confirmation remain explicit. | +| App update checks | `updates/updates.ts`; retain scheduled checks. Download/install/relaunch remain explicit lifecycle actions. | + +## Explicit actions retained + +| Controls / consumers | Reason | +| --- | --- | +| Send, Outbox, reaction, edit, delete, report, moderation, invitations, feedback | Mutations or ambiguous delivery. Existing durable operation IDs and explicit confirmation/retry remain authoritative. | +| Channel creation/join, hide/unhide, sort/filter/move, Canvas/todos, unread updates, agent configuration/sharing/archive/stop, workspace actions | User intent or persistence/publication; no automatic replay. Read-only precursors use the owners above. | +| File/media downloads, native Save, copying links/images, avatar/upload retries | User-directed local writes, OS dialogs, clipboard or uploads. Preview/data reads are covered separately. | +| Appearance/notification/shortcut/search/developer settings, local drafts and repository-folder choices | Local persistence or device configuration; unknown native failures do not establish a recoverable network read. | +| Secure storage, identity restore/reveal/setup, OAuth sign-in, microphone/playback permission, local provider installation | Authentication, permission or setup can require interaction; do not repeat prompts or installation silently. | +| Media errors 1/3/4, unsupported source/listener, playback rejection | Cancellation, decoding, unsupported resource or browser permission; only explicit media network failures receive reload recovery. | +| Plugin import/install/update/rollback/delete, page/sidebar/plugin/module crash recovery, navigation/window-opening failure | Code/process lifecycle or interactive setup; preserve explicit recovery. Read-only navigation data uses the owners above. | + +## Validation boundaries + +Deterministic tests cover shared budgets, nested exhaustion, admission pauses, +caller/cache/access/dispose cancellation, retained views, staff outcomes, immutable +Git validation, provider read versus write replay, and media reload cleanup. +Existing manual-recovery presentation tests use terminal failures; transient API +failures have separate owner tests with controlled clocks. + +The representative app journeys exercise hidden-conversation exhaustion/manual +retry and relay recovery from edge/VPN errors in Chromium and WebKit. These browser +checks do not establish native signing/IPC, a real relay, native media playback, +or every screen/OS permission flow. diff --git a/src/app/App.profile.test.tsx b/src/app/App.profile.test.tsx index 97f74dd22e..dddeb4c97c 100644 --- a/src/app/App.profile.test.tsx +++ b/src/app/App.profile.test.tsx @@ -624,6 +624,7 @@ it("reopens profile settings from the same session cache while refreshing", asyn expect(refresh).toHaveBeenCalledWith( origin, current.relay.snapshot().session, + expect.any(AbortSignal), ); const reopened = screen.getByRole("dialog", { name: "Settings" }); expect( diff --git a/src/app/ProfileSettings.avatar.test.tsx b/src/app/ProfileSettings.avatar.test.tsx index f9867920a5..af152d0bff 100644 --- a/src/app/ProfileSettings.avatar.test.tsx +++ b/src/app/ProfileSettings.avatar.test.tsx @@ -222,7 +222,11 @@ it("a dispatched save remains bound to A and its late completion cannot clear B' late.resolve(); await late.promise; }); - expect(api.inspectProfile).toHaveBeenLastCalledWith(a); + expect(api.inspectProfile).toHaveBeenLastCalledWith( + a, + undefined, + expect.any(AbortSignal), + ); expect(fixture.saveProfile).toHaveBeenCalledWith({ name: "Changed", picture: "", @@ -413,7 +417,11 @@ it("confirms a dispatched save through its captured session after leaving Settin late.resolve(); await late.promise; }); - expect(api.inspectProfile).toHaveBeenLastCalledWith(a, owner.session); + expect(api.inspectProfile).toHaveBeenLastCalledWith( + a, + owner.session, + expect.any(AbortSignal), + ); expect(fixture.saveProfile).toHaveBeenCalledExactlyOnceWith({ name: "Changed", picture: "", diff --git a/src/app/ProfileSettings.test.tsx b/src/app/ProfileSettings.test.tsx index ea1b089eda..a65ded5fa4 100644 --- a/src/app/ProfileSettings.test.tsx +++ b/src/app/ProfileSettings.test.tsx @@ -123,7 +123,11 @@ it("loads and publishes the selected community profile before updating the local about: "Community bio", }); expect(await screen.findByText("Profile updated")).toBeVisible(); - expect(inspect).toHaveBeenCalledWith("community-id"); + expect(inspect).toHaveBeenCalledWith( + "community-id", + undefined, + expect.any(AbortSignal), + ); }); it("keeps a newer profile seed when an older community publication finishes last", async () => { @@ -657,7 +661,11 @@ it("reuses the verified same-community profile while refreshing and preserves ed "Fresh about", ); expect(screen.getByRole("button", { name: "Save" })).toBeEnabled(); - expect(inspect).toHaveBeenCalledWith("primary", owner.session); + expect(inspect).toHaveBeenCalledWith( + "primary", + owner.session, + expect.any(AbortSignal), + ); view.unmount(); } finally { refresh.resolve({ diff --git a/src/app/ProfileSettings.tsx b/src/app/ProfileSettings.tsx index fe308ad60a..5b4f1976bf 100644 --- a/src/app/ProfileSettings.tsx +++ b/src/app/ProfileSettings.tsx @@ -130,10 +130,12 @@ export function ProfileSettings({ const [dispatched, setDispatched] = useState(false); const [uploading, setUploading] = useState(false); const mounted = useRef(false); + const saveRead = useRef(undefined); useEffect(() => { mounted.current = true; return () => { mounted.current = false; + saveRead.current?.abort(); }; }, []); const [error, setError] = useState(""); @@ -143,6 +145,7 @@ export function ProfileSettings({ useLayoutEffect(() => { if (previousScope.current === profileScope) return; previousScope.current = profileScope; + saveRead.current?.abort(); setDraft(null); setLoaded(cachedProfile(selectedProfileSession(communities, communityId))); setLoadStatus(communityId ? "idle" : "ready"); @@ -158,14 +161,17 @@ export function ProfileSettings({ } if (!client.relayAvailable || !client.viewer) return; let current = true; + const controller = new AbortController(); const session = selectedProfileSession(communities, communityId); const seed = cachedProfile(session); if (seed) setLoaded(seed); setLoadStatus("loading"); setLoadError(""); - const read = session - ? communityApi.inspectProfile(communityId, session) - : communityApi.inspectProfile(communityId); + const read = communityApi.inspectProfile( + communityId, + session, + controller.signal, + ); void read.then( (result) => { if (!current) return; @@ -189,6 +195,7 @@ export function ProfileSettings({ ); return () => { current = false; + controller.abort(); }; }, [ communities, @@ -334,10 +341,10 @@ export function ProfileSettings({ communities, community?.id, ); - const inspect = (id: string) => - session - ? communityApi.inspectProfile(id, session) - : communityApi.inspectProfile(id); + const preflight = new AbortController(); + saveRead.current = preflight; + const inspect = (id: string, signal: AbortSignal) => + communityApi.inspectProfile(id, session, signal); const saveGeneration = beginProfileSave(communities); setSaving(true); setSaved(false); @@ -346,16 +353,23 @@ export function ProfileSettings({ try { if (community && loaded) { // Preserve fields outside this editor even if another client edited them. - const latest = await inspect(community.id); + const latest = await inspect( + community.id, + preflight.signal, + ); if (!mounted.current) return; setDispatched(true); + saveRead.current = undefined; await communityApi.publishProfile( community.id, next, latest.existing, ); // Once dispatched, finish on this session even if Settings closes. - const confirmed = await inspect(community.id); + const confirmed = await inspect( + community.id, + AbortSignal.timeout(12000), + ); if ( !confirmed.exists || !profilesEqual(confirmed.profile, next) @@ -376,13 +390,15 @@ export function ProfileSettings({ setDraft(null); setSaved(true); } catch (reason) { - if (mounted.current) + if (mounted.current && !preflight.signal.aborted) setError( reason instanceof Error ? reason.message : String(reason), ); } finally { + if (saveRead.current === preflight) + saveRead.current = undefined; if (mounted.current) { setSaving(false); setDispatched(false); diff --git a/src/app/shell/SearchResults.test.tsx b/src/app/shell/SearchResults.test.tsx index 9c69e3dcc5..492b67ec27 100644 --- a/src/app/shell/SearchResults.test.tsx +++ b/src/app/shell/SearchResults.test.tsx @@ -395,10 +395,14 @@ it("shows the real read failure, retains conversation choices, and retries to an }, ]); await act(async () => - request.fail(new ReadError("unavailable", "Relay read timed out")), + request.fail( + new ReadError("invalid-response", "Relay response was invalid"), + ), ); expect( - screen.getByText(/Message search couldn’t finish: Relay read timed out/), + screen.getByText( + /Message search couldn’t finish: Relay response was invalid/, + ), ).toBeVisible(); expect(screen.getByRole("option", { name: /wes-crew/ })).toBeVisible(); fireEvent.click(screen.getByRole("button", { name: "Retry messages" })); @@ -580,10 +584,14 @@ it("sorts a conversation newest first across pages and restarts paging when the { ...text, search_sort: "newest", page: 2 }, ]); await act(async () => - failed.fail(new ReadError("unavailable", "Relay read timed out")), + failed.fail( + new ReadError("invalid-response", "Relay response was invalid"), + ), ); expect( - screen.getByText("More messages couldn’t load: Relay read timed out"), + screen.getByText( + "More messages couldn’t load: Relay response was invalid", + ), ).toBeVisible(); fireEvent.click(screen.getByRole("option", { name: /Show more messages/ })); const second = await request(); diff --git a/src/bundled/builderlab/adapter-type/Create.test.tsx b/src/bundled/builderlab/adapter-type/Create.test.tsx index 84c6e909ed..06097c0c03 100644 --- a/src/bundled/builderlab/adapter-type/Create.test.tsx +++ b/src/bundled/builderlab/adapter-type/Create.test.tsx @@ -45,7 +45,7 @@ async function fixture(signedIn = true) { const pending = deferred(); const request = vi.fn(async () => { await pending.promise; - if (fail) throw new Error("offline"); + if (fail) return { status: 400, headers: {}, body: "{}" }; return { status: 200, headers: {}, @@ -133,9 +133,7 @@ it("retains instructions when catalog loading fails and is retried", async () => ); await act(async () => h.pending.resolve()); const retry = await screen.findByRole("button", { name: "Retry models" }); - expect(screen.getByRole("alert")).toHaveTextContent( - "Could not reach Builderlab", - ); + expect(screen.getByRole("alert")).toHaveTextContent("HTTP 400"); h.fail(false); await userEvent.setup().click(retry); await screen.findByRole("combobox", { name: "Model" }); diff --git a/src/bundled/builderlab/adapter-type/Instructions.test.tsx b/src/bundled/builderlab/adapter-type/Instructions.test.tsx index 1c5e5585f2..a6c7627d47 100644 --- a/src/bundled/builderlab/adapter-type/Instructions.test.tsx +++ b/src/bundled/builderlab/adapter-type/Instructions.test.tsx @@ -107,11 +107,9 @@ function fixture() { it("retries a failed load and refuses to save a draft through another account", async () => { const h = fixture(); const user = userEvent.setup(); - h.request.mockRejectedValueOnce(new Error("offline")); + h.request.mockResolvedValueOnce({ ...response(), status: 400 }); render(, { wrapper: StrictMode }); - expect(await screen.findByRole("alert")).toHaveTextContent( - "Could not reach Builderlab", - ); + expect(await screen.findByRole("alert")).toHaveTextContent("HTTP 400"); await user.click(screen.getByRole("button", { name: "Retry" })); const field = await screen.findByRole("textbox", { name: "Instructions" }); expect(field).toHaveValue("First instructions."); diff --git a/src/bundled/builderlab/adapter-type/ModelSettings.test.tsx b/src/bundled/builderlab/adapter-type/ModelSettings.test.tsx index 4686704080..796e62e24f 100644 --- a/src/bundled/builderlab/adapter-type/ModelSettings.test.tsx +++ b/src/bundled/builderlab/adapter-type/ModelSettings.test.tsx @@ -34,7 +34,7 @@ async function fixture(selection: Partial = {}) { })); await session.signIn(); const request = vi.fn(async ({ url, body }) => { - if (offline) throw new Error("offline"); + if (offline) return { status: 400, headers: {}, body: "{}" }; let result: unknown; if (url.endsWith("/get-model-catalog")) result = { @@ -192,9 +192,7 @@ it("preserves a failed draft and refuses to save it through a different account" h.offline(true); const user = userEvent.setup(); await user.click(screen.getByRole("button", { name: "Save model" })); - expect(await screen.findByRole("alert")).toHaveTextContent( - "Could not reach Builderlab", - ); + expect(await screen.findByRole("alert")).toHaveTextContent("HTTP 400"); expect( screen.queryByRole("dialog", { name: "Model preferences saved" }), ).not.toBeInTheDocument(); @@ -222,15 +220,11 @@ it("retries a failed catalog refresh without retrying a failed save", async () = await screen.findByRole("button", { name: "Refresh models" }), ); const retry = await screen.findByRole("button", { name: "Retry models" }); - expect(screen.getByRole("alert")).toHaveTextContent( - "Could not reach Builderlab", - ); + expect(screen.getByRole("alert")).toHaveTextContent("HTTP 400"); h.offline(false); await user.click(retry); await screen.findByRole("button", { name: "Refresh models" }); - expect(screen.getByRole("alert")).toHaveTextContent( - "Could not reach Builderlab", - ); + expect(screen.getByRole("alert")).toHaveTextContent("HTTP 400"); expect( screen.getByRole("combobox", { name: "Reasoning effort" }), ).toHaveTextContent("low"); diff --git a/src/bundled/builderlab/agents/RemoteAgents.test.tsx b/src/bundled/builderlab/agents/RemoteAgents.test.tsx index df86140234..ff06d5262c 100644 --- a/src/bundled/builderlab/agents/RemoteAgents.test.tsx +++ b/src/bundled/builderlab/agents/RemoteAgents.test.tsx @@ -65,8 +65,8 @@ it("shows a held loading state and retries a failed read", async () => { expect( screen.getByRole("button", { name: "Refresh agents" }), ).toHaveAttribute("aria-disabled", "true"); - await act(async () => held.resolve(response([], 503))); - expect(screen.getByRole("alert")).toHaveTextContent("HTTP 503"); + await act(async () => held.resolve(response([], 400))); + expect(screen.getByRole("alert")).toHaveTextContent("HTTP 400"); await userEvent.setup().click(screen.getByRole("button", { name: "Retry" })); expect(await screen.findByText("Helper · Active")).toBeInTheDocument(); expect(screen.queryByRole("alert")).not.toBeInTheDocument(); diff --git a/src/bundled/builderlab/agents/client.test.ts b/src/bundled/builderlab/agents/client.test.ts index 1ee27aef5b..f7ed77ca2f 100644 --- a/src/bundled/builderlab/agents/client.test.ts +++ b/src/bundled/builderlab/agents/client.test.ts @@ -140,7 +140,15 @@ it.each([401, 403, 500])( vi.mocked(h.host.request).mockResolvedValue( response({ error: "private-secret" }, status), ); - const error = await h.client.list(h.signal).catch((error: Error) => error); + vi.useFakeTimers(); + let error: unknown; + try { + const pending = h.client.list(h.signal).catch((error: Error) => error); + await vi.advanceTimersByTimeAsync(7000); + error = await pending; + } finally { + vi.useRealTimers(); + } expect(error).toBeInstanceOf(Error); expect(String(error)).not.toContain("private-secret"); expect(h.session.snapshot().status).toBe( @@ -772,3 +780,69 @@ it("shares one binding attempt between overlapping links", async () => { .mock.calls.map(([input]) => input.url.split("/").at(-1)), ).toEqual(["current", "attest-agent"]); }); + +it.each(["network", "503", "429"])( + "quietly recovers read-only Builderlab discovery after %s", + async (failure) => { + const h = await fixture(); + vi.useFakeTimers(); + try { + const request = vi.mocked(h.host.request); + if (failure === "network") + request.mockRejectedValueOnce(new Error("offline")); + else request.mockResolvedValueOnce(response({}, Number(failure))); + const pending = h.client.list(h.signal); + await vi.advanceTimersByTimeAsync(499); + expect(request).toHaveBeenCalledOnce(); + await vi.advanceTimersByTimeAsync(501); + expect(await pending).toHaveLength(1); + expect(request).toHaveBeenCalledTimes(2); + } finally { + vi.useRealTimers(); + } + }, +); + +it("does not replay a Builderlab mutation after a transport failure", async () => { + const h = await fixture(); + vi.useFakeTimers(); + try { + const request = vi + .mocked(h.host.request) + .mockRejectedValue(new Error("offline")); + await expect(h.client.delete({ id: "agent-1" }, h.signal)).rejects.toThrow( + "Could not reach Builderlab", + ); + await vi.advanceTimersByTimeAsync(7000); + expect(request).toHaveBeenCalledOnce(); + } finally { + vi.useRealTimers(); + } +}); + +it("exhausts Builderlab reads once and cancels the same backoff on account change", async () => { + const h = await fixture(); + vi.useFakeTimers(); + try { + const request = vi + .mocked(h.host.request) + .mockResolvedValue(response({}, 503)); + const rejected = expect(h.client.list(h.signal)).rejects.toThrow( + "HTTP 503", + ); + await vi.advanceTimersByTimeAsync(7000); + await rejected; + expect(request).toHaveBeenCalledTimes(4); + const pending = h.client.list(h.signal); + const cancelled = expect(pending).rejects.toMatchObject({ + name: "AbortError", + }); + await vi.advanceTimersByTimeAsync(0); + h.session.signOut(); + await vi.advanceTimersByTimeAsync(1000); + await cancelled; + expect(request).toHaveBeenCalledTimes(5); + } finally { + vi.useRealTimers(); + } +}); diff --git a/src/bundled/builderlab/agents/client.ts b/src/bundled/builderlab/agents/client.ts index e003076c59..526d9e2ed6 100644 --- a/src/bundled/builderlab/agents/client.ts +++ b/src/bundled/builderlab/agents/client.ts @@ -1,3 +1,4 @@ +import { retryRead } from "../../../features/relay/read-retry"; import { invoke } from "@tauri-apps/api/core"; import { nativeIdentityEnabled } from "../../../features/identity/service"; import type { @@ -98,6 +99,8 @@ const nativeOwner: OwnerIdentity = { invoke("identity_sign_builderlab_binding", { challenge }), }; +class BuilderlabUnavailable extends Error {} + const LINK_ERRORS: Record = { identity_already_bound: "This Builderlab account is linked to a different Buzz identity.", @@ -157,7 +160,7 @@ export function createAgentClient( }); } catch { check(credential, signal); - throw new Error("Could not reach Builderlab. Try again."); + throw new BuilderlabUnavailable("Could not reach Builderlab. Try again."); } check(credential, signal); signOutIfExpired(response); @@ -180,6 +183,28 @@ export function createAgentClient( throw new Error("Builderlab returned an invalid response."); } } + function readRequest( + path: "get-model-catalog" | "list-agents", + body: unknown, + signal: AbortSignal, + ) { + const credential = remoteCredential(); + return retryRead( + (signal) => { + check(credential, signal); + return request(path, body, signal); + }, + signal, + (error) => + error instanceof BuilderlabUnavailable || + (error instanceof Error && + "status" in error && + typeof error.status === "number" && + (error.status === 408 || + error.status === 429 || + error.status >= 500)), + ); + } /** Builderlab's identity routes answer failures with `{ error: { code } }`. */ async function identityRequest( path: string, @@ -335,7 +360,7 @@ export function createAgentClient( async modelCatalog( signal: AbortSignal, ): Promise { - const result = await request("get-model-catalog", {}, signal); + const result = await readRequest("get-model-catalog", {}, signal); const models = result.models ?? []; if ( !Array.isArray(models) || @@ -595,7 +620,7 @@ export function createAgentClient( }; }, async list(signal: AbortSignal): Promise { - const result = await request( + const result = await readRequest( "list-agents", { include_instructions: true }, signal, diff --git a/src/bundled/channels/ChannelMembersDialog.test.tsx b/src/bundled/channels/ChannelMembersDialog.test.tsx index 12e62cc372..741c4c2613 100644 --- a/src/bundled/channels/ChannelMembersDialog.test.tsx +++ b/src/bundled/channels/ChannelMembersDialog.test.tsx @@ -355,12 +355,14 @@ it("DMs are view-only and never search outside their members", async () => { }); it("Escape closes the dialog and restores focus to its header button", async () => { const t = await setup(); - expect(screen.getByRole("searchbox")).toHaveFocus(); + await vi.waitFor(() => expect(screen.getByRole("searchbox")).toHaveFocus()); await t.user.keyboard("{Escape}"); await vi.waitFor(() => expect(screen.queryByRole("dialog")).not.toBeInTheDocument(), ); - expect(screen.getByRole("button", { name: "Access" })).toHaveFocus(); + await vi.waitFor(() => + expect(screen.getByRole("button", { name: "Access" })).toHaveFocus(), + ); }); it("failed directory searches recover through the shared refresh, not a false empty result", async () => { const t = await setup(); diff --git a/src/bundled/claude-code/setup.ts b/src/bundled/claude-code/setup.ts index 1370c92857..03fad60226 100644 --- a/src/bundled/claude-code/setup.ts +++ b/src/bundled/claude-code/setup.ts @@ -3,6 +3,7 @@ // process; the copy lives apart from any `claude` the user installed, and // shares their `~/.claude` sign-in and settings. import type { AgentProviderSnapshot } from "../../features/agents/service"; +import { isRetryableRead, retryRead } from "../../features/relay/read-retry"; import type { HostProcess, HostRequest, @@ -76,7 +77,11 @@ async function queryModels(spawn: Spawn, signal: AbortSignal) { signal.addEventListener("abort", abort, { once: true }); if (signal.aborted) abort(); const timer = setTimeout(() => { - request.abort(new Error("Claude Code model discovery timed out")); + request.abort( + Object.assign(new Error("Claude Code model discovery timed out"), { + name: "TimeoutError", + }), + ); void child?.kill(); }, CHECK_TIMEOUT_MS); try { @@ -313,8 +318,9 @@ export class ClaudeSetup { /** Loads optional model copy only for surfaces that display the catalog. */ async loadModels(retry = false) { + const spawn = this.spawn; if ( - !this.spawn || + !spawn || this.state.status?.state !== "ready" || this.state.checking || this.modelRequest || @@ -325,7 +331,14 @@ export class ClaudeSetup { this.modelRequest = request; this.set({ modelsChecking: true, modelsError: "" }); try { - const models = await queryModels(this.spawn, request.signal); + const began = performance.now(); + const models = await retryRead( + (signal) => queryModels(spawn, signal), + request.signal, + (error) => + performance.now() - began < CHECK_TIMEOUT_MS && + isRetryableRead(error), + ); if (!models.length) throw new Error("Claude did not list its models"); if (this.modelRequest === request) this.set({ models }); } catch (error) { diff --git a/src/bundled/codex/index.test.ts b/src/bundled/codex/index.test.ts index 35f4c7366b..c6ec8f33b6 100644 --- a/src/bundled/codex/index.test.ts +++ b/src/bundled/codex/index.test.ts @@ -5,6 +5,7 @@ import { CodexLogoIcon } from "../../shared/design-system/icons"; import { CodexRuntime } from "./runtime"; import { CodexSetup } from "./setup"; import { CodexProvider } from "./tabs"; +import type { Spawn, Wire } from "./rpc"; import { apply } from "./index"; const desktop = vi.hoisted(() => ({ tauri: true })); @@ -285,3 +286,107 @@ it("preserves a compatible shared Codex model", () => { model: "gpt-shared", }); }); + +it.each(["run", "runOnce"] as const)( + "cancels %s during catalog checking and bounds a hung model list", + async (method) => { + vi.useFakeTimers(); + vi.spyOn(CodexSetup.prototype, "update").mockResolvedValue(); + vi.spyOn(CodexSetup.prototype, "installed").mockResolvedValue(); + vi.spyOn(CodexSetup.prototype, "check").mockResolvedValue({ + state: "ready", + version: "0.162.1", + }); + const run = vi + .spyOn(CodexRuntime.prototype, method) + .mockResolvedValue({ ok: true }); + const requested: string[] = []; + const spawn: Spawn = vi.fn(async (_id, options) => { + let exit!: (code: number | null) => void; + const exited = new Promise((resolve) => { + exit = resolve; + }); + return { + write: async (text: string) => { + const wire: Wire = JSON.parse(text); + if (wire.id == null) return; + requested.push(wire.method ?? ""); + if (wire.method === "model/list") return; + options?.onStdout?.( + `${JSON.stringify({ + id: wire.id, + result: + wire.method === "account/read" + ? { account: {}, requiresOpenaiAuth: false } + : {}, + })}\n`, + ); + }, + end: async () => { + exit(0); + }, + kill: async () => { + exit(null); + }, + exited, + }; + }); + let type!: AgentType; + let dispose = () => {}; + apply({ + host: { spawn, request: vi.fn() }, + agents: { + register: (registered: AgentType) => { + type = registered; + }, + snapshot: () => ({ status: "ready", agents: [], running: [] }), + relay: () => undefined, + subscribe: () => () => {}, + }, + effect: (setup: () => () => void) => { + dispose = setup(); + }, + } as never); + try { + const provider = type.provider; + const start = type[method]; + if (!provider || !start) throw new Error("Missing Codex provider or run"); + const check = provider.check(); + await vi.advanceTimersByTimeAsync(0); + expect(requested).toContain("model/list"); + const stopped = new AbortController(); + let outcome: unknown; + const first = Promise.resolve( + start({ signal: stopped.signal } as never), + ).then( + () => { + outcome = "started"; + }, + (error) => { + outcome = error; + }, + ); + await vi.advanceTimersByTimeAsync(1000); + stopped.abort(new Error("Turn stopped")); + await vi.advanceTimersByTimeAsync(0); + expect(outcome).toBe(stopped.signal.reason); + expect(run).not.toHaveBeenCalled(); + // Cancelling one waiter does not cancel the device-wide catalog for others. + const second = start({ + signal: new AbortController().signal, + } as never); + await vi.advanceTimersByTimeAsync(28_999); + expect(provider.snapshot().readiness).toBe("checking"); + expect(run).not.toHaveBeenCalled(); + await vi.advanceTimersByTimeAsync(1); + await Promise.all([check, first, second]); + expect(provider.snapshot().readiness).toBe("attention"); + expect(run).toHaveBeenCalledOnce(); + await vi.advanceTimersByTimeAsync(120_000); + expect(spawn).toHaveBeenCalledOnce(); + } finally { + dispose(); + vi.useRealTimers(); + } + }, +); diff --git a/src/bundled/codex/index.ts b/src/bundled/codex/index.ts index c2d2b47448..97d8237512 100644 --- a/src/bundled/codex/index.ts +++ b/src/bundled/codex/index.ts @@ -197,7 +197,7 @@ export const apply: PluginModule["apply"] = (ctx) => { ...codexRun(runtime, async (signal) => { await setup.installed(signal); if (providerController.snapshot().readiness === "checking") - await providerController.check(); + await providerController.check(signal); }), }); }; diff --git a/src/bundled/codex/rpc.test.ts b/src/bundled/codex/rpc.test.ts index 370d66ead6..510326ab54 100644 --- a/src/bundled/codex/rpc.test.ts +++ b/src/bundled/codex/rpc.test.ts @@ -1,6 +1,6 @@ import { expect, it, vi } from "vitest"; import type { HostProcessOptions } from "../../features/host/service"; -import { AppServer, type Wire } from "./rpc"; +import { AppServer, listModels, RequestTimeout, type Wire } from "./rpc"; function fixture() { let options: HostProcessOptions | undefined; let exit!: (code: number) => void; @@ -85,3 +85,23 @@ it("contains consumer exceptions and closes malformed output while rejecting pen consoleError.mockRestore(); } }); + +it("keeps turn-shared model listing single-attempt after its timeout", async () => { + vi.useFakeTimers(); + const f = fixture(); + try { + await f.open(); + const rejected = expect(listModels(f.rpc)).rejects.toBeInstanceOf( + RequestTimeout, + ); + await vi.advanceTimersByTimeAsync(30000); + await rejected; + await vi.advanceTimersByTimeAsync(7000); + expect(f.sent.filter((wire) => wire.method === "model/list")).toHaveLength( + 1, + ); + } finally { + await f.rpc.close(); + vi.useRealTimers(); + } +}); diff --git a/src/bundled/codex/rpc.ts b/src/bundled/codex/rpc.ts index efad5a4508..e9fd06da06 100644 --- a/src/bundled/codex/rpc.ts +++ b/src/bundled/codex/rpc.ts @@ -33,6 +33,8 @@ export type ToolReply = { /** Codex's answer refusing a request, which it therefore did not carry out. */ export class Refused extends Error {} +/** Typed transport timeout; only read-only catalog owners opt into retrying it. */ +export class RequestTimeout extends Error {} /** One request owner for the JSONL app-server connection. No Node sidecar. */ export class AppServer { @@ -202,7 +204,7 @@ export class AppServer { return new Promise((resolve, reject) => { const timer = setTimeout(() => { this.pending.delete(id); - reject(new Error(`Codex ${method} timed out`)); + reject(new RequestTimeout(`Codex ${method} timed out`)); }, 30_000); this.pending.set(id, { resolve: (value) => resolve(value as T), diff --git a/src/bundled/codex/tabs.test.tsx b/src/bundled/codex/tabs.test.tsx index a015050067..20434371ee 100644 --- a/src/bundled/codex/tabs.test.tsx +++ b/src/bundled/codex/tabs.test.tsx @@ -32,7 +32,7 @@ import { defaults, type Config } from "./config"; import { sessionTag } from "./prompt"; import { CodexProvider, createTabs } from "./tabs"; import { CodexRuntime } from "./runtime"; -import type { Wire } from "./rpc"; +import { RequestTimeout, type Wire } from "./rpc"; import { CodexSetup } from "./setup"; function deferred() { let resolve!: (value: T) => void; @@ -1550,3 +1550,96 @@ it("shows the recent empty state when only the current runtime session exists", ).toBeVisible(); expect(screen.queryByRole("list")).not.toBeInTheDocument(); }); + +it("retries a typed transport timeout only within the catalog owner", async () => { + const f = fixture(); + const spawn = vi + .fn(f.spawn) + .mockRejectedValueOnce(new RequestTimeout("Catalog timed out")); + const provider = new CodexProvider(spawn, localStore()); + vi.useFakeTimers(); + try { + const pending = provider.check(); + await vi.advanceTimersByTimeAsync(499); + expect(spawn).toHaveBeenCalledOnce(); + await vi.advanceTimersByTimeAsync(501); + await pending; + expect(provider.snapshot().readiness).toBe("ready"); + expect(spawn).toHaveBeenCalledTimes(2); + } finally { + provider.dispose(); + vi.useRealTimers(); + } +}); + +it.each([ + ["signed-out", "Log in to Codex", "Codex needs you to sign in."], + ["missing", "Install Codex", "Codex is not installed."], +] as const)( + "preserves a slow %s setup outcome", + async (state, label, error) => { + const f = fixture(); + vi.spyOn(f.setup, "check").mockImplementation( + () => + new Promise((resolve) => { + setTimeout(() => resolve({ state, version: "0.162.1" }), 31_000); + }), + ); + vi.useFakeTimers(); + try { + const pending = f.provider.check(); + await vi.advanceTimersByTimeAsync(31_000); + await pending; + expect(f.provider.snapshot()).toMatchObject({ + readiness: "attention", + attentionLabel: label, + error, + }); + expect(f.spawn).not.toHaveBeenCalled(); + } finally { + f.provider.dispose(); + vi.useRealTimers(); + } + }, +); + +it("gives the catalog its full lifetime after slow successful setup", async () => { + const f = fixture(); + vi.spyOn(f.setup, "check").mockImplementation( + () => + new Promise((resolve) => { + setTimeout( + () => resolve({ state: "ready", version: "0.162.1" }), + 31_000, + ); + }), + ); + const spawn = vi.fn(async (...args: Parameters) => { + const process = await f.spawn(...args); + return { + ...process, + write: async (text: string) => { + const wire: Wire = JSON.parse(text); + if (wire.method === "model/list") + await new Promise((resolve) => setTimeout(resolve, 15_000)); + await process.write(text); + }, + }; + }); + const provider = new CodexProvider(spawn, localStore(), f.setup); + vi.useFakeTimers(); + try { + const pending = provider.check(); + await vi.advanceTimersByTimeAsync(31_000); + expect(provider.snapshot().checking).toBe(true); + await vi.advanceTimersByTimeAsync(14_999); + expect(provider.snapshot().readiness).toBe("checking"); + await vi.advanceTimersByTimeAsync(1); + await pending; + expect(provider.snapshot().readiness).toBe("ready"); + expect(spawn).toHaveBeenCalledOnce(); + } finally { + provider.dispose(); + vi.useRealTimers(); + } +}); diff --git a/src/bundled/codex/tabs.tsx b/src/bundled/codex/tabs.tsx index 685b7532b2..f7882d6299 100644 --- a/src/bundled/codex/tabs.tsx +++ b/src/bundled/codex/tabs.tsx @@ -1,3 +1,4 @@ +import { isRetryableRead, retryRead } from "../../features/relay/read-retry"; import activityStyles from "../../features/agents/ActivityLayout.module.css"; import { formatItemTimestamp } from "../../shared/datetime"; import { InlineHeader } from "../../shared/design-system/ui/Header"; @@ -49,7 +50,13 @@ import { type Config, type Model, } from "./config"; -import { AppServer, listModels, needsSignIn, type Spawn } from "./rpc"; +import { + AppServer, + listModels, + needsSignIn, + RequestTimeout, + type Spawn, +} from "./rpc"; import type { CodexRuntime, SavedConversation } from "./runtime"; import type { CodexSetup } from "./setup"; import { readTranscript, TURN_LIMIT, type CodexTranscript } from "./transcript"; @@ -76,6 +83,8 @@ const STORAGE_KEY = "buzz.codex.provider-defaults.v1"; const SIGN_IN_ERROR = "Codex needs you to sign in."; const NOT_INSTALLED = "Codex is not installed."; const CONVERSATION_LIMIT = 30; +// Initial mentions await this catalog too; do not multiply RPC timeouts. +const CATALOG_TIMEOUT_MS = 30_000; /** Read-only setup/catalog discovery uses the same declared native transport. */ async function catalog(spawn: Spawn, signal: AbortSignal) { const rpc = new AppServer(); @@ -135,17 +144,39 @@ export class CodexProvider { this.state = Object.freeze({ ...this.state, ...change }); for (const listener of this.listeners) listener(); } - check() { - if (this.pending) return this.pending; - const pending = this.runCheck(); - this.pending = pending; - void pending.finally(() => { - if (this.pending === pending) this.pending = undefined; + check(signal?: AbortSignal) { + signal?.throwIfAborted(); + if (!this.pending) { + const pending = this.runCheck(); + this.pending = pending; + void pending.finally(() => { + if (this.pending === pending) this.pending = undefined; + }); + } + const pending = this.pending; + if (!signal) return pending; + // A turn stops waiting without retiring discovery shared by other callers. + return new Promise((resolve, reject) => { + const abort = () => reject(signal.reason); + signal.addEventListener("abort", abort, { once: true }); + void pending.then( + () => { + signal.removeEventListener("abort", abort); + resolve(); + }, + (error) => { + signal.removeEventListener("abort", abort); + reject(error); + }, + ); + if (signal.aborted) abort(); }); - return pending; } private async runCheck() { const abort = new AbortController(); + const deadline = new AbortController(); + const signal = AbortSignal.any([abort.signal, deadline.signal]); + let timer: ReturnType | undefined; this.abort = abort; this.set({ ...(this.state.data ? {} : { readiness: "checking" as const }), @@ -162,7 +193,15 @@ export class CodexProvider { ? SIGN_IN_ERROR : status.message, ); - const data = await catalog(this.spawn, abort.signal); + timer = setTimeout( + () => deadline.abort(new RequestTimeout("Codex catalog timed out")), + CATALOG_TIMEOUT_MS, + ); + const data = await retryRead( + (signal) => catalog(this.spawn, signal), + signal, + (error) => error instanceof RequestTimeout || isRetryableRead(error), + ); if (this.abort === abort) { this.set({ readiness: "ready", @@ -187,6 +226,7 @@ export class CodexProvider { error: reason, }); } finally { + clearTimeout(timer); if (this.abort === abort) this.abort = undefined; } } diff --git a/src/bundled/github/GitHubBody.test.tsx b/src/bundled/github/GitHubBody.test.tsx index 0f6e14b8fb..fdc62465f2 100644 --- a/src/bundled/github/GitHubBody.test.tsx +++ b/src/bundled/github/GitHubBody.test.tsx @@ -517,9 +517,9 @@ it("retires pending PR loads, retries errors, and clears media when navigating", ).not.toBeInTheDocument(); expect(screen.getByRole("status")).toHaveTextContent("Loading"); await act(async () => { - second.resolve(new Response(null, { status: 500 })); + second.resolve(new Response(null, { status: 400 })); }); - expect(screen.getByRole("alert")).toHaveTextContent("500"); + expect(screen.getByRole("alert")).toHaveTextContent("400"); fireEvent.click(screen.getByRole("button", { name: "Retry" })); expect(screen.getByRole("status")).toHaveTextContent("Loading"); await act(async () => { diff --git a/src/bundled/github/GitHubPanel.test.tsx b/src/bundled/github/GitHubPanel.test.tsx index 89ad1b4b12..6306bfda6e 100644 --- a/src/bundled/github/GitHubPanel.test.tsx +++ b/src/bundled/github/GitHubPanel.test.tsx @@ -296,7 +296,7 @@ it.each([ }, ); -it.each([500])( +it.each([400])( "keeps the PR title and prominent external recovery action usable after API failure %s", async (status) => { const target = "https://github.com/block/buzz-app/pull/629"; diff --git a/src/bundled/github/data.test.tsx b/src/bundled/github/data.test.tsx index 617c4c6a0a..ae0c6c75be 100644 --- a/src/bundled/github/data.test.tsx +++ b/src/bundled/github/data.test.tsx @@ -64,9 +64,16 @@ it.each([ "fetch", vi.fn().mockResolvedValue(new Response(null, { status: Number(status) })), ); - await expect( - loadGitHubDetails(reference, new AbortController().signal), - ).rejects.toThrow(String(message)); + vi.useFakeTimers(); + try { + const rejected = expect( + loadGitHubDetails(reference, new AbortController().signal), + ).rejects.toThrow(String(message)); + await vi.advanceTimersByTimeAsync(7000); + await rejected; + } finally { + vi.useRealTimers(); + } }); it("does not expose check head plumbing on non-PR objects", async () => { @@ -215,3 +222,24 @@ it.each([null, "bad", undefined])( expect(data.mergedByUrl).toBeUndefined(); }, ); + +it("recovers a transient GitHub API page quietly within one budget", async () => { + vi.useFakeTimers(); + try { + const request = vi + .fn() + .mockResolvedValueOnce(new Response(null, { status: 502 })) + .mockImplementation(async () => + Response.json({ title: "Recovered", body: "Kept content" }), + ); + vi.stubGlobal("fetch", request); + const pending = loadGitHubDetails(reference, new AbortController().signal); + await vi.advanceTimersByTimeAsync(499); + expect(request).toHaveBeenCalledOnce(); + await vi.advanceTimersByTimeAsync(501); + expect((await pending).title).toBe("Recovered"); + expect(request).toHaveBeenCalledTimes(2); + } finally { + vi.useRealTimers(); + } +}); diff --git a/src/bundled/github/data.ts b/src/bundled/github/data.ts index afd2618a49..b76528d23d 100644 --- a/src/bundled/github/data.ts +++ b/src/bundled/github/data.ts @@ -1,3 +1,5 @@ +import { ReadError } from "../../features/relay/errors"; +import { retryRead } from "../../features/relay/read-retry"; import type { GitHubReference } from "./references"; export type Branch = { label: string; url?: string | undefined }; @@ -87,14 +89,23 @@ export async function loadGitHubDetails( issue: `/issues/${id}`, commit: `/commits/${id}`, }[reference.kind]; - const response = await fetch( - `https://api.github.com/repos/${reference.repository}${surface}`, - { - signal, - headers: { Accept: "application/vnd.github.full+json" }, - credentials: "omit", - }, - ); + const response = await retryRead(async (signal) => { + const response = await fetch( + `https://api.github.com/repos/${reference.repository}${surface}`, + { + signal, + headers: { Accept: "application/vnd.github.full+json" }, + credentials: "omit", + }, + ); + if (response.status === 408 || response.status >= 500) + throw new ReadError( + "unavailable", + `GitHub couldn’t load this object (${response.status}).`, + response.status, + ); + return response; + }, signal); if (!response.ok) { if (response.status === 404) throw new Error( diff --git a/src/bundled/github/object.ts b/src/bundled/github/object.ts index 223391b818..e32e046503 100644 --- a/src/bundled/github/object.ts +++ b/src/bundled/github/object.ts @@ -1,3 +1,4 @@ +import { isRetryableRead, retryRead } from "../../features/relay/read-retry"; // What the panel shows for a GitHub link: read with the person's `gh` sign-in // when there is one, so private repositories work, else from the public API. import { loadGitHubDetails, type GitHubDetails } from "./data"; @@ -13,7 +14,21 @@ export type GitHubObject = Readonly<{ signedIn: boolean; }>; -export async function loadGitHubObject( +export function loadGitHubObject( + reference: GitHubReference, + signal: AbortSignal, + gh: GitHubCli | undefined, +): Promise { + return retryRead( + (signal) => readGitHubObject(reference, signal, gh), + signal, + (error) => + isRetryableRead(error) || + (error instanceof GitHubError && + (error.type === "INTERNAL" || error.type === "SERVICE_UNAVAILABLE")), + ); +} +async function readGitHubObject( reference: GitHubReference, signal: AbortSignal, gh: GitHubCli | undefined, diff --git a/src/bundled/goose/tabs.tsx b/src/bundled/goose/tabs.tsx index 3655802fab..1bc29eec6d 100644 --- a/src/bundled/goose/tabs.tsx +++ b/src/bundled/goose/tabs.tsx @@ -1,4 +1,5 @@ import { ModelDiscoveryRecovery } from "../../features/agents/ModelDiscoveryRecovery"; +import { retryRead } from "../../features/relay/read-retry"; // goose's New agent choices and agent settings. Providers, their keys and // their models are goose's own: listed from it, and saved into its config. import { @@ -90,12 +91,13 @@ function useRead(read: () => Promise, key: string, scope = key) { // biome-ignore lint/correctness/useExhaustiveDependencies: `read` follows `key`. useEffect(() => { let current = true; + const controller = new AbortController(); const kept = (prior: Read) => prior.value === undefined || prior.scope !== scope ? {} : { value: prior.value, scope }; setState((prior) => ({ status: "loading", ...kept(prior) })); - read().then( + retryRead(() => read(), controller.signal).then( (value) => current && setState({ status: "ready", value, scope }), (error: unknown) => current && @@ -107,6 +109,7 @@ function useRead(read: () => Promise, key: string, scope = key) { ); return () => { current = false; + controller.abort(); }; }, [key, round]); return [state, () => setRound((value) => value + 1)] as const; diff --git a/src/bundled/pi/tabs.tsx b/src/bundled/pi/tabs.tsx index 11ec9b2700..b8c6f8a606 100644 --- a/src/bundled/pi/tabs.tsx +++ b/src/bundled/pi/tabs.tsx @@ -8,6 +8,9 @@ import { useSyncExternalStore, } from "react"; import { errorText } from "../../buzz-mcp/rpc"; +import { retryRead } from "../../features/relay/read-retry"; +import { isTransientFailure } from "../../features/relay/errors"; +import { PiStopped } from "./pi"; import activityStyles from "../../features/agents/ActivityLayout.module.css"; import { ActivityTranscript } from "../../features/agents/ActivityTranscript"; import { AgentInstructions } from "../../features/agents/AgentInstructions"; @@ -113,7 +116,11 @@ function useRead( ? {} : { value: prior.value, scope }; setState((prior) => ({ status: "loading", ...kept(prior) })); - read(abort.signal).then( + retryRead( + read, + abort.signal, + (error) => error instanceof PiStopped || isTransientFailure(error), + ).then( (value) => { if (!abort.signal.aborted) setState({ status: "ready", value, scope }); }, diff --git a/src/bundled/relay-staff/Feedback.tsx b/src/bundled/relay-staff/Feedback.tsx index 969b4d57eb..115d859ac5 100644 --- a/src/bundled/relay-staff/Feedback.tsx +++ b/src/bundled/relay-staff/Feedback.tsx @@ -21,7 +21,13 @@ import type { StaffFailure, StaffRequest, } from "../../features/relay-staff/contract"; -import { describe, useRead, useSession, useWrite } from "./session"; +import { + describe, + retryStaffRead, + useRead, + useSession, + useWrite, +} from "./session"; import { CommunityBadge, Failure, @@ -289,8 +295,12 @@ function AttachmentView({ attachment }: { attachment: AttachmentRef }) { if (!image) return; let url: string | null = null; let current = true; - void session.attachment(attachment).then((outcome) => { - if (!current) return; + const controller = new AbortController(); + void retryStaffRead( + () => session.attachment(attachment), + controller.signal, + ).then((outcome) => { + if (!outcome || !current) return; setRetrying(false); if (!outcome.ok) return setPreview({ failure: outcome.failure }); url = URL.createObjectURL( @@ -301,6 +311,7 @@ function AttachmentView({ attachment }: { attachment: AttachmentRef }) { // The object URL is released when the attachment closes or unmounts. return () => { current = false; + controller.abort(); if (url) URL.revokeObjectURL(url); }; }, [session, attachment.feedbackId, attachment.sha256, image, attempt]); diff --git a/src/bundled/relay-staff/read-retry.test.ts b/src/bundled/relay-staff/read-retry.test.ts new file mode 100644 index 0000000000..1130b253e4 --- /dev/null +++ b/src/bundled/relay-staff/read-retry.test.ts @@ -0,0 +1,74 @@ +import { afterEach, beforeEach, expect, it, vi } from "vitest"; +import type { + StaffFailure, + StaffOutcome, +} from "../../features/relay-staff/contract"; +import { retryStaffRead } from "./session"; + +beforeEach(() => { + vi.useFakeTimers(); + vi.spyOn(Math, "random").mockReturnValue(0.5); +}); +afterEach(() => { + vi.restoreAllMocks(); + vi.useRealTimers(); +}); +function failure( + category: StaffFailure["category"], + status: number | null, +): StaffOutcome { + return { + ok: false, + failure: { + category, + status, + bodyComplete: false, + bodyEmpty: false, + code: null, + notSent: false, + authLost: status === 403, + message: "Existing failure", + }, + }; +} +it("quietly recovers intercepted staff reads using one bounded budget", async () => { + const read = vi + .fn() + .mockResolvedValueOnce(failure("intercepted", 403)) + .mockResolvedValue({ ok: true, value: "loaded" }); + const pending = retryStaffRead(read, new AbortController().signal); + await vi.advanceTimersByTimeAsync(749); + expect(read).toHaveBeenCalledOnce(); + await vi.advanceTimersByTimeAsync(1); + await expect(pending).resolves.toEqual({ ok: true, value: "loaded" }); +}); +it("returns the original staff failure after four reads", async () => { + const original = failure("ambiguous", 502); + const read = vi.fn().mockResolvedValue(original); + const pending = retryStaffRead(read, new AbortController().signal); + await vi.advanceTimersByTimeAsync(5250); + expect(await pending).toEqual(original); + expect(read).toHaveBeenCalledTimes(4); +}); +it.each(["unauthorized", "forbidden", "rejected", "notSent"] as const)( + "leaves %s failures explicit", + async (category) => { + const original = failure(category, 403); + const read = vi.fn().mockResolvedValue(original); + expect(await retryStaffRead(read, new AbortController().signal)).toBe( + original, + ); + expect(read).toHaveBeenCalledOnce(); + }, +); +it("scope retirement releases staff backoff without another request", async () => { + const read = vi.fn().mockResolvedValue(failure("ambiguous", 502)); + const controller = new AbortController(); + const pending = retryStaffRead(read, controller.signal); + await vi.advanceTimersByTimeAsync(0); + controller.abort(); + expect(await pending).toBeUndefined(); + await vi.advanceTimersByTimeAsync(7000); + expect(read).toHaveBeenCalledOnce(); + expect(vi.getTimerCount()).toBe(0); +}); diff --git a/src/bundled/relay-staff/session.tsx b/src/bundled/relay-staff/session.tsx index 1305a8aa95..0dc37c2705 100644 --- a/src/bundled/relay-staff/session.tsx +++ b/src/bundled/relay-staff/session.tsx @@ -20,6 +20,62 @@ import type { } from "../../features/relay-staff/contract"; import type { Staff } from "./staff"; import type { Held, Writes } from "./writes"; +import { retryRead } from "../../features/relay/read-retry"; +import { ReadRetriesExhausted } from "../../features/relay/errors"; + +const READ_ROUTES = new Set([ + "probe", + "listReports", + "getReport", + "listFeedback", + "getFeedback", + "listOperators", + "listRestrictions", + "listCommunities", + "searchMembers", + "getMember", + "getEvent", +]); +class StaffReadFailure extends Error { + constructor(readonly failure: StaffFailure) { + super(failure.message); + } +} +function transientRead(failure: StaffFailure) { + return ( + failure.category === "intercepted" || + (failure.category === "ambiguous" && + (failure.status === null || + failure.status === 408 || + failure.status === 429 || + failure.status >= 500)) + ); +} +/** Only read owners call this; frozen write intents keep their explicit retry. */ +export async function retryStaffRead( + read: () => Promise>, + signal: AbortSignal, +): Promise | undefined> { + try { + return await retryRead( + async () => { + const outcome = await read(); + if (!outcome.ok && transientRead(outcome.failure)) + throw new StaffReadFailure(outcome.failure); + return outcome; + }, + signal, + (error) => error instanceof StaffReadFailure, + ); + } catch (error) { + if (signal.aborted) return; + const original = + error instanceof ReadRetriesExhausted ? error.cause : error; + if (original instanceof StaffReadFailure) + return { ok: false, failure: original.failure }; + throw error; + } +} export type Session = { staff: Staff; @@ -127,13 +183,20 @@ export function useRead( setRead(({ settled }) => ({ state: "loading", settled: settled + 1 })); return; } - let current = true; - setRead(({ settled }) => ({ - state: "loading", - settled: settled + rescoped, - })); - void send(request).then((outcome) => { - if (!current) return; + const controller = new AbortController(); + setRead((previous) => + !rescoped && previous.state === "ok" + ? previous + : { + state: "loading", + settled: previous.settled + rescoped, + }, + ); + const reading = READ_ROUTES.has(request.route) + ? retryStaffRead(() => send(request), controller.signal) + : send(request); + void reading.then((outcome) => { + if (!outcome || controller.signal.aborted) return; setRead(({ settled }) => outcome.ok ? { state: "ok", value: outcome.value, settled: settled + 1 } @@ -141,7 +204,7 @@ export function useRead( ); }); return () => { - current = false; + controller.abort(); }; }, [send, attempt, ...deps]); const reload = useCallback(() => setAttempt((n) => n + 1), []); @@ -232,14 +295,23 @@ export function usePages( settled: 0, }); const generation = useRef(0); + const pending = useRef(undefined); const pageRef = useRef(page); pageRef.current = page; const load = async (cursor: string | undefined, at: number) => { const make = pageRef.current; - if (!make) return; + if (!make || pending.current) return; + const controller = new AbortController(); + pending.current = controller; setPages((current) => ({ ...current, loading: true, failure: null })); - const outcome = await send(make(cursor)); - if (at !== generation.current) return; + const outcome = await retryStaffRead( + () => send(make(cursor)), + controller.signal, + ).finally(() => { + if (pending.current === controller) pending.current = undefined; + }); + if (!outcome || controller.signal.aborted || at !== generation.current) + return; setPages((current) => outcome.ok ? { @@ -262,6 +334,8 @@ export function usePages( }; // biome-ignore lint/correctness/useExhaustiveDependencies: `deps` describe `page` useEffect(() => { + pending.current?.abort(); + pending.current = undefined; const at = ++generation.current; // A new scope (or a disabled one) ends any attempt kept from the old one. setPages(({ settled }) => ({ @@ -271,6 +345,8 @@ export function usePages( })); void load(undefined, at); return () => { + pending.current?.abort(); + pending.current = undefined; generation.current++; }; }, [send, ...deps]); @@ -278,6 +354,8 @@ export function usePages( if (pages.next && !pages.loading) void load(pages.next, generation.current); }; const restart = () => { + pending.current?.abort(); + pending.current = undefined; const at = ++generation.current; void load(undefined, at); }; diff --git a/src/bundled/voice-notes/VoiceNoteCard.tsx b/src/bundled/voice-notes/VoiceNoteCard.tsx index 2aa365995d..38b9597e7d 100644 --- a/src/bundled/voice-notes/VoiceNoteCard.tsx +++ b/src/bundled/voice-notes/VoiceNoteCard.tsx @@ -1,3 +1,4 @@ +import { useMediaReadRecovery } from "../../features/messages/use-media-read-recovery"; import { useMediaElementSource } from "../../features/messages/use-media-element-source"; import { MediaDownloadAction } from "../../features/messages/MediaDownloadAction"; import { AttachmentDownloadContext } from "../../features/messages/AttachmentDownloadContext"; @@ -80,6 +81,18 @@ function Player({ const [loading, setLoading] = useState(false); const [error, setError] = useState(false); const [rate, setRate] = useState(1); + const { recover: recoverMedia, cancel: cancelMedia } = useMediaReadRecovery( + active ? source : undefined, + () => { + setError(true); + setLoading(false); + setPlaying(false); + }, + () => { + setError(false); + setLoading(false); + }, + ); const peaks = waveform?.length ? waveformPeaks(Float32Array.from(waveform.slice(0, 100)), count) : Array.from({ length: count }, () => 0); @@ -139,11 +152,13 @@ function Player({ if (!element || !source) return; const operation = ++attempt.current; if (loading || !element.paused) { + cancelMedia(); element.pause(); setLoading(false); return; } claimAudio(element, () => { + cancelMedia(); attempt.current++; element.pause(); setLoading(false); @@ -155,7 +170,7 @@ function Player({ await element.play(); if (attempt.current === operation) setLoading(false); } catch { - if (attempt.current === operation) { + if (attempt.current === operation && !recoverMedia(element)) { setLoading(false); releaseAudio(element); setError(true); @@ -319,6 +334,7 @@ function Player({ paintProgress(0, duration); }} onError={(event) => { + if (recoverMedia(event.currentTarget)) return; setMetadataReady(false); releaseAudio(event.currentTarget); attempt.current++; diff --git a/src/features/agent-harness/acp/process.ts b/src/features/agent-harness/acp/process.ts index 67132cdb73..6fd0d25bca 100644 --- a/src/features/agent-harness/acp/process.ts +++ b/src/features/agent-harness/acp/process.ts @@ -364,8 +364,11 @@ export class AcpProcess { const timer = setTimeout(() => { this.calls.delete(id); reject( - new Error( - `${label} did not answer ${method} in ${Math.round(timeout / 1000)}s`, + Object.assign( + new Error( + `${label} did not answer ${method} in ${Math.round(timeout / 1000)}s`, + ), + { name: "TimeoutError" }, ), ); void this.kill(); diff --git a/src/features/agents/agent-config-history.ts b/src/features/agents/agent-config-history.ts index 7e3ad05365..b438b3d059 100644 --- a/src/features/agents/agent-config-history.ts +++ b/src/features/agents/agent-config-history.ts @@ -1,6 +1,7 @@ // NIP-AP config history (plan §7): offset pages of one artifact's revisions, // every chain segment they form, the cause of each gap, diffs and revert. // Display only: history never decides artifact state (§3.2). +import { retryRead } from "../relay/read-retry"; import type { EventData, RelayEvent } from "../relay/events"; import { type SettingsSchema, syncedSettings } from "./agent-config"; import { @@ -49,18 +50,23 @@ export async function loadOlder( if (history.done || history.next > HISTORY_MAX_OFFSET) return { ...history, failed: false }; try { - const page = await read( - [ - { - kinds: [ARTIFACT_KIND], - "#h": [channel], - "#d": [d], - artifact: "history", - limit: HISTORY_PAGE, - offset: history.next, - }, - ], - { signal }, + const active = signal ?? new AbortController().signal; + const page = await retryRead( + (signal) => + read( + [ + { + kinds: [ARTIFACT_KIND], + "#h": [channel], + "#d": [d], + artifact: "history", + limit: HISTORY_PAGE, + offset: history.next, + }, + ], + { signal }, + ), + active, ); const seen = new Set(history.revisions.map((event) => event.id)); return { diff --git a/src/features/agents/library.ts b/src/features/agents/library.ts index 9f8388c89b..0d1770662d 100644 --- a/src/features/agents/library.ts +++ b/src/features/agents/library.ts @@ -1,3 +1,4 @@ +import { retryRead } from "../relay/read-retry.ts"; /** A display-only projection of the local and owner-authored relay inventory, not proof of * ownership, membership, custody or running state. No prompts/configuration. */ export type AgentLibrary = Readonly<{ @@ -71,7 +72,7 @@ export function createAgentLibrary( .then(() => { if (closed || owned.signal.aborted) throw new Error("Library read cancelled"); - return read(owned.signal); + return retryRead(read, owned.signal); }) .then((library) => { if (!closed && !owned.signal.aborted) diff --git a/src/features/agents/memory.ts b/src/features/agents/memory.ts index 63046b8847..a9663a3ca5 100644 --- a/src/features/agents/memory.ts +++ b/src/features/agents/memory.ts @@ -1,3 +1,4 @@ +import { retryRead } from "../relay/read-retry.ts"; /** Owner-view NIP-AE snapshot from a purpose-bound key-owning host. */ /** Budgets of the development broker's single memory query. */ export const MEMORY_EVENT_LIMIT = 256; @@ -171,7 +172,9 @@ export function createAgentMemories( owned.signal, AbortSignal.timeout(10000), ]); - const listing = memoryListing(await source(agent, signal)); + const listing = memoryListing( + await retryRead((signal) => source(agent, signal), signal), + ); signal.throwIfAborted(); if (!owned.signal.aborted) publish( diff --git a/src/features/channel-members/administration.ts b/src/features/channel-members/administration.ts index b27c9d9261..2920aed872 100644 --- a/src/features/channel-members/administration.ts +++ b/src/features/channel-members/administration.ts @@ -1,5 +1,6 @@ import { getEventHash } from "nostr-tools"; import type { RelayReader } from "../relay/reader"; +import { retryRead } from "../relay/read-retry"; import type { RelayWriter } from "../relay/transport"; import { newer, type RelayEvent } from "../relay/events"; import { attestedOwner } from "../agents/owner-attestation"; @@ -128,7 +129,7 @@ export function createMemberAdministration({ const previous = snapshot(id); emit(id, { ...previous, status: "loading" }); try { - const result = await read(id, signal); + const result = await retryRead(() => read(id, signal), signal); const operation = previous.operation; const unresolved = operation?.status === "uncertain"; const confirmed = diff --git a/src/features/channel-navigation/ChannelInventory.test.tsx b/src/features/channel-navigation/ChannelInventory.test.tsx index ede20ade0e..af307c78a6 100644 --- a/src/features/channel-navigation/ChannelInventory.test.tsx +++ b/src/features/channel-navigation/ChannelInventory.test.tsx @@ -15,6 +15,8 @@ import { DiscoveryState } from "../relay/discovery"; import { keypair, metadata, roster, signed } from "../relay/testing"; import { huddleDescription } from "../huddle/lifecycle"; import { createRelaySession } from "../relay/session"; +import { createChannelLifecycle } from "../relay/channel-lifecycle"; +import { ReadError } from "../relay/errors"; import { createSidebarPreferencesStore } from "../relay/sidebar-preferences-store"; import type { SidebarGroups, @@ -226,6 +228,83 @@ function fixture( }; } +function visibilityFixture() { + const f = fixture(); + const read = vi + .fn() + .mockRejectedValue(new ReadError("unavailable", "offline", 503)); + const lifecycle = createChannelLifecycle({ + reader: { read }, + viewer: f.session.viewer, + relayAuthor: "b".repeat(64), + canAccess: () => true, + acceptDiscovery() {}, + removed() {}, + }); + f.session.channelLifecycle = lifecycle.capability; + return { f, read, lifecycle }; +} + +it("keeps visibility recovery quiet, then offers the existing retry after exhaustion", async () => { + vi.useFakeTimers(); + const random = vi.spyOn(Math, "random").mockReturnValue(0.5); + const { f, read, lifecycle } = visibilityFixture(); + try { + render(f.view("alpha")); + await act(() => vi.advanceTimersByTimeAsync(0)); + expect(read).toHaveBeenCalledOnce(); + expect( + screen.queryByText("Hidden conversations could not be refreshed."), + ).not.toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: "Retry hidden conversations" }), + ).not.toBeInTheDocument(); + await act(() => vi.advanceTimersByTimeAsync(5250)); + expect(read).toHaveBeenCalledTimes(4); + expect( + screen.getByText("Hidden conversations could not be refreshed."), + ).toBeVisible(); + read.mockResolvedValue([]); + fireEvent.click( + screen.getByRole("button", { name: "Retry hidden conversations" }), + ); + await act(() => vi.advanceTimersByTimeAsync(0)); + expect(read).toHaveBeenCalledTimes(5); + expect( + screen.queryByRole("button", { name: "Retry hidden conversations" }), + ).not.toBeInTheDocument(); + } finally { + cleanup(); + lifecycle.dispose(); + random.mockRestore(); + vi.useRealTimers(); + } +}); + +it("cancels visibility retries on sidebar unmount and session changes", async () => { + vi.useFakeTimers(); + const { f, read, lifecycle } = visibilityFixture(); + try { + const mounted = render(f.view("alpha")); + await act(() => vi.advanceTimersByTimeAsync(0)); + expect(read).toHaveBeenCalledOnce(); + const next = fixture(); + mounted.rerender(next.view("beta")); + await act(() => vi.advanceTimersByTimeAsync(120_000)); + expect(read).toHaveBeenCalledOnce(); + mounted.rerender(f.view("alpha")); + await act(() => vi.advanceTimersByTimeAsync(0)); + expect(read).toHaveBeenCalledTimes(2); + mounted.unmount(); + await act(() => vi.advanceTimersByTimeAsync(120_000)); + expect(read).toHaveBeenCalledTimes(2); + } finally { + cleanup(); + lifecycle.dispose(); + vi.useRealTimers(); + } +}); + it.each(["Retry", "Dismiss"] as const)( "%s restores mute recovery focus after cached-to-live session handoff without remounting", async (action) => { diff --git a/src/features/channel-navigation/ChannelInventory.tsx b/src/features/channel-navigation/ChannelInventory.tsx index 2077bf2f54..eb4ac1e4a5 100644 --- a/src/features/channel-navigation/ChannelInventory.tsx +++ b/src/features/channel-navigation/ChannelInventory.tsx @@ -145,9 +145,14 @@ export function ChannelInventory({ lifecycle.snapshot, lifecycle.snapshot, ); + const visibilityRequest = useRef(undefined); // biome-ignore lint/correctness/useExhaustiveDependencies: a completed roster refresh also refreshes per-viewer visibility. useEffect(() => { - if (list.status === "ready") void lifecycle.refreshVisibility(); + const request = new AbortController(); + visibilityRequest.current = request; + if (list.status === "ready") + void lifecycle.refreshVisibility(request.signal); + return () => request.abort(); }, [lifecycle, list.asOf, list.status]); const lifecycleFocus = useRef(undefined); const lifecycleTrigger = useRef(undefined); @@ -1030,16 +1035,20 @@ export function ChannelInventory({ ))} - {!cached && - dmVisibility.status === "error" && - !dmVisibility.transient && ( -
- Hidden conversations could not be refreshed.{" "} - -
- )} + {!cached && dmVisibility.status === "error" && ( +
+ Hidden conversations could not be refreshed.{" "} + +
+ )} ), content: ( diff --git a/src/features/channels/usePublicChannelSearch.ts b/src/features/channels/usePublicChannelSearch.ts index ecb51ea953..bff6b1fb4c 100644 --- a/src/features/channels/usePublicChannelSearch.ts +++ b/src/features/channels/usePublicChannelSearch.ts @@ -1,3 +1,4 @@ +import { retryRead } from "../relay/read-retry"; import { useCallback, useEffect, useMemo, useState } from "react"; import type { ChannelSummary } from "../relay/contracts"; import type { ComposerSession } from "../messages/composer-session"; @@ -38,21 +39,25 @@ export function usePublicChannelSearch( let kept = none; // Typeahead waits for a brief typing pause; cancellation also owns the delay. const timer = setTimeout(() => { - search(query, { - signal: controller.signal, - priority: "foreground", - exact, - // An exact lookup names one channel, so it waits for every page. - progress: exact - ? undefined - : ({ channels }) => { - const ids = channels.map(({ id }) => id).join(); - if (controller.signal.aborted || ids === shown) return; - shown = ids; - kept = channels; - setResult({ owner, channels, partial: false, pending: true }); - }, - }).then( + retryRead( + (signal) => + search(query, { + signal, + priority: "foreground", + exact, + // An exact lookup names one channel, so it waits for every page. + progress: exact + ? undefined + : ({ channels }) => { + const ids = channels.map(({ id }) => id).join(); + if (controller.signal.aborted || ids === shown) return; + shown = ids; + kept = channels; + setResult({ owner, channels, partial: false, pending: true }); + }, + }), + controller.signal, + ).then( ({ channels, partial }) => { if (!controller.signal.aborted) setResult({ owner, channels, partial }); diff --git a/src/features/communities/CommunityDialog.tsx b/src/features/communities/CommunityDialog.tsx index d1080b2886..9b78778fc5 100644 --- a/src/features/communities/CommunityDialog.tsx +++ b/src/features/communities/CommunityDialog.tsx @@ -93,22 +93,34 @@ export function CommunityDialog({ const [busy, setBusy] = useState(false); const [error, setError] = useState(recovery.error); const mounted = useRef(true); + const pendingRead = useRef(undefined); + const readScope = JSON.stringify([client.viewer, mode, connectionFrom]); useEffect(() => { + void readScope; // A different identity/destination retires pending reads. mounted.current = true; + setBusy(false); return () => { mounted.current = false; + pendingRead.current?.abort(); + pendingRead.current = undefined; }; - }, []); - async function work(action: () => Promise) { + }, [readScope]); + async function work(action: (signal: AbortSignal) => Promise) { + pendingRead.current?.abort(); + const controller = new AbortController(); + pendingRead.current = controller; setBusy(true); setError(""); try { - await action(); + await action(controller.signal); } catch (reason) { - if (mounted.current) + if (mounted.current && pendingRead.current === controller) setError(reason instanceof Error ? reason.message : String(reason)); } finally { - if (mounted.current) setBusy(false); + if (pendingRead.current === controller) { + pendingRead.current = undefined; + if (mounted.current) setBusy(false); + } } } const policy = info?.policy; @@ -130,17 +142,19 @@ export function CommunityDialog({ async function submit() { if (uploading || unavailable) return; if (step === "destination") { - await work(async () => { + await work(async (signal) => { const next = communityDestination(relayOrigin(url)); setDestination(next); const value = await communityRequest(next.id, "info"); if (!mounted.current) return; const pending = journal?.get(next.id); // Restoring an existing admitted profile is not a new join or policy acceptance. - const found = await inspectProfile(next.id).catch((reason: unknown) => { - if (pending && readErrorKind(reason) !== "denied") throw reason; - return undefined; - }); + const found = await inspectProfile(next.id, undefined, signal).catch( + (reason: unknown) => { + if (pending && readErrorKind(reason) !== "denied") throw reason; + return undefined; + }, + ); if (current(pending)) { setInfo(value); if (found && (found.exists || pending)) showProfile(found, pending); @@ -149,15 +163,17 @@ export function CommunityDialog({ }); } else if (step === "access") { if (!allowed) return; - await work(async () => { + await work(async (signal) => { const pending = journal?.get(id); if (pending) { // A lost claim response may already have admitted this identity, even // when its invite has since expired. Read before attempting another claim. - const found = await inspectProfile(id).catch((reason: unknown) => { - if (readErrorKind(reason) !== "denied") throw reason; - return undefined; - }); + const found = await inspectProfile(id, undefined, signal).catch( + (reason: unknown) => { + if (readErrorKind(reason) !== "denied") throw reason; + return undefined; + }, + ); if (!current(pending)) return; if (found) { showProfile(found, pending); @@ -196,19 +212,21 @@ export function CommunityDialog({ throw new Error("Membership was not confirmed"); } if (!current(transaction)) return; - const found = await inspectProfile(id); + const found = await inspectProfile(id, undefined, signal); if (!current(transaction)) return; showProfile(found, transaction); }); } else { if (!profile.name.trim()) return; - await work(async () => { + await work(async (signal) => { if (mode === "profile") communities.saveProfile({ ...profile, name: profile.name.trim() }); else { if (!destination) throw new Error("Choose a community first"); const transaction = journal?.begin(id, profile); - const found = journal ? await inspectProfile(id) : original; + const found = journal + ? await inspectProfile(id, undefined, signal) + : original; if (!current(transaction)) return; const next = found?.exists && profilesEqual(profile, found.profile) @@ -222,7 +240,7 @@ export function CommunityDialog({ await publishProfile(id, next, found?.existing ?? {}); if (!current(transaction)) return; // An accepted replaceable event may already be superseded. - const confirmed = await inspectProfile(id); + const confirmed = await inspectProfile(id, undefined, signal); if (!current(transaction)) return; if (!confirmed.exists || !profilesEqual(confirmed.profile, next)) throw new Error( diff --git a/src/features/communities/api.ts b/src/features/communities/api.ts index 17a45753ec..527d2bc00d 100644 --- a/src/features/communities/api.ts +++ b/src/features/communities/api.ts @@ -1,4 +1,5 @@ import { nativeIdentityEnabled } from "../identity/service"; +import { retryRead } from "../relay/read-retry"; import { nativeCommunityRequest } from "./native-api"; import { connectCommunityTransport, registerCommunity } from "./connection"; import { settledRefusal, type SettledRefusal } from "./leave-protocol"; @@ -50,12 +51,21 @@ export async function communityRequest( ); return result as T; } -export async function inspectProfile(id: string, session?: RelaySession) { +export async function inspectProfile( + id: string, + session: RelaySession | undefined, + caller: AbortSignal, +) { + caller.throwIfAborted(); // Existing-community editors use the captured session so confirmed reads also // update its shared profile directory. Joining uses the selected host adapter. const transport = session ?? - (await connectCommunityTransport(id, AbortSignal.timeout(12000))); + (await retryRead( + (signal) => connectCommunityTransport(id, signal), + AbortSignal.any([caller, AbortSignal.timeout(12000)]), + )); + const signal = AbortSignal.any([caller, AbortSignal.timeout(12000)]); if (!transport.viewer) throw new Error("Profile identity is unavailable"); const filters = [ { @@ -67,11 +77,9 @@ export async function inspectProfile(id: string, session?: RelaySession) { ]; const events = "read" in transport - ? await transport.read(filters, { - signal: AbortSignal.timeout(12000), - fresh: true, - }) - : await transport.query(filters, AbortSignal.timeout(12000)); + ? await transport.read(filters, { signal, fresh: true }) + : await retryRead((signal) => transport.query(filters, signal), signal); + signal.throwIfAborted(); const event = events .filter((e) => e.kind === 0 && e.pubkey === transport.viewer) .sort((a, b) => b.created_at - a.created_at || a.id.localeCompare(b.id))[0]; diff --git a/src/features/communities/native-api.test.ts b/src/features/communities/native-api.test.ts index b28533437f..134c5253b5 100644 --- a/src/features/communities/native-api.test.ts +++ b/src/features/communities/native-api.test.ts @@ -249,7 +249,11 @@ it("restores a signed community profile and preserves extra fields when publishi path === "/query" ? { body: [event] } : { body: { accepted: true, event_id: (body as { id: string }).id } }; - const found = await inspectProfile(community); + const found = await inspectProfile( + community, + undefined, + new AbortController().signal, + ); expect(found.profile).toEqual({ name: "Existing", picture: "", diff --git a/src/features/communities/profile-read-retry.test.ts b/src/features/communities/profile-read-retry.test.ts new file mode 100644 index 0000000000..374bea6873 --- /dev/null +++ b/src/features/communities/profile-read-retry.test.ts @@ -0,0 +1,77 @@ +import { afterEach, beforeEach, expect, it, vi } from "vitest"; +import { inspectProfile } from "./api"; +import { createRelaySession } from "../relay/session"; +import { ReadError } from "../relay/errors"; +import { keypair } from "../relay/testing"; +import type { RelayEvent } from "../relay/events"; + +const owners: ReturnType[] = []; +beforeEach(() => { + vi.useFakeTimers(); + vi.spyOn(Math, "random").mockReturnValue(0.5); + vi.spyOn(AbortSignal, "timeout").mockImplementation((ms) => { + const controller = new AbortController(); + setTimeout( + () => + controller.abort( + new DOMException("Profile read timed out", "TimeoutError"), + ), + ms, + ); + return controller.signal; + }); +}); +afterEach(() => { + for (const owner of owners.splice(0)) owner.dispose(); + vi.clearAllTimers(); + vi.restoreAllMocks(); + vi.useRealTimers(); +}); +function setup() { + const viewer = keypair(); + const query = vi.fn(async (): Promise => []); + const owner = createRelaySession({ + viewer: viewer.pubkey, + relayAuthor: keypair().pubkey, + query, + media: () => undefined, + }); + owners.push(owner); + return { owner, query }; +} + +it("keeps one session budget and the profile's twelve-second deadline", async () => { + const h = setup(); + h.query.mockImplementation(async () => { + await new Promise((resolve) => setTimeout(resolve, 4000)); + throw new ReadError("unavailable", "offline"); + }); + const pending = inspectProfile( + "community", + h.owner.session, + new AbortController().signal, + ); + const rejected = expect(pending).rejects.toMatchObject({ + name: "TimeoutError", + }); + await vi.advanceTimersByTimeAsync(12_000); + await rejected; + expect(h.query).toHaveBeenCalledTimes(3); + await vi.advanceTimersByTimeAsync(30_000); + expect(h.query).toHaveBeenCalledTimes(3); +}); + +it("cancels profile backoff with the editor's signal", async () => { + const h = setup(); + h.query.mockRejectedValue(new ReadError("unavailable", "offline")); + const caller = new AbortController(); + const pending = inspectProfile("community", h.owner.session, caller.signal); + const rejected = expect(pending).rejects.toMatchObject({ + name: "AbortError", + }); + await vi.advanceTimersByTimeAsync(0); + caller.abort(); + await rejected; + await vi.advanceTimersByTimeAsync(7000); + expect(h.query).toHaveBeenCalledOnce(); +}); diff --git a/src/features/messages/MediaAttachment.tsx b/src/features/messages/MediaAttachment.tsx index fcfc85c542..d6f9de3e4b 100644 --- a/src/features/messages/MediaAttachment.tsx +++ b/src/features/messages/MediaAttachment.tsx @@ -1,3 +1,4 @@ +import { useMediaReadRecovery } from "./use-media-read-recovery"; import { useConversationPresentation } from "../conversation/ConversationPresentation"; import { prepareReviewEntrance } from "./use-review-entrance"; import { useMediaCorners } from "./use-media-corners"; @@ -129,6 +130,10 @@ export function MediaAttachment({ attachment.kind === "video" ? source : undefined, ); const videoSource = element.src; + const { recover: recoverMedia } = useMediaReadRecovery( + active ? videoSource : undefined, + () => setFailed(true), + ); if (!source && !compact) return ( @@ -312,7 +317,9 @@ export function MediaAttachment({ } } }} - onError={() => setFailed(true)} + onError={(event) => { + if (!recoverMedia(event.currentTarget)) setFailed(true); + }} onPause={() => setPlaying(false)} onPlay={() => { setStarted(true); diff --git a/src/features/messages/VideoPlayer.tsx b/src/features/messages/VideoPlayer.tsx index 3e1954f7cc..9e97209526 100644 --- a/src/features/messages/VideoPlayer.tsx +++ b/src/features/messages/VideoPlayer.tsx @@ -1,3 +1,4 @@ +import { useMediaReadRecovery } from "./use-media-read-recovery"; import { useVideoGestures } from "./use-video-gestures"; import { useMediaControls } from "./use-media-controls"; import { useMediaCorners } from "./use-media-corners"; @@ -446,6 +447,10 @@ export function VideoPlayer({ // Callers render their unavailable state instead of a player when // `useMediaElementSource(source).unavailable`. const { src: elementSource } = useMediaElementSource(source); + const { recover: recoverMedia } = useMediaReadRecovery( + elementSource, + onError, + ); return (
onTime(event.currentTarget.currentTime)} - onError={onError} + onError={(event) => { + if (!recoverMedia(event.currentTarget)) onError(); + }} /> {feedback && (
{ + vi.useFakeTimers(); + vi.spyOn(Math, "random").mockReturnValue(0.5); + vi.spyOn(HTMLMediaElement.prototype, "load").mockImplementation(() => {}); +}); +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); + vi.useRealTimers(); +}); +function setup() { + const failed = vi.fn(), + recovered = vi.fn(); + function Probe({ source }: { source: string }) { + const { recover } = useMediaReadRecovery(source, failed, recovered); + return ( + + ); + } + const mounted = render(); + const element = mounted.container.querySelector("audio"); + if (!element) throw new Error("Audio fixture missing"); + const error = (code = 2) => { + Object.defineProperty(element, "error", { + configurable: true, + value: { code }, + }); + fireEvent.error(element); + }; + return { + ...mounted, + element, + error, + failed, + recovered, + change: () => mounted.rerender(), + }; +} +async function advance(ms: number) { + await act(() => vi.advanceTimersByTimeAsync(ms)); +} + +it("reloads a network failure quietly after backoff and preserves explicit playback", async () => { + const h = setup(); + const play = vi.spyOn(h.element, "play"); + h.error(); + await advance(749); + expect(h.element.load).not.toHaveBeenCalled(); + expect(h.failed).not.toHaveBeenCalled(); + await advance(1); + expect(h.element.load).toHaveBeenCalledOnce(); + expect(h.element.preload).toBe("metadata"); + fireEvent.loadedMetadata(h.element); + await advance(0); + expect(h.recovered).toHaveBeenCalledOnce(); + expect(h.element.preload).toBe("none"); + expect(h.failed).not.toHaveBeenCalled(); + expect(play).not.toHaveBeenCalled(); +}); + +it("counts the initial browser failure and exposes failure after three reloads", async () => { + const h = setup(); + h.error(); + for (const [index, delay] of [750, 1500, 3000].entries()) { + await advance(delay); + expect(h.element.load).toHaveBeenCalledTimes(index + 1); + h.error(); + } + await advance(0); + expect(h.failed).toHaveBeenCalledOnce(); + expect(vi.getTimerCount()).toBe(0); +}); + +it.each(["unmount", "source"])("%s cancels future reloads", async (action) => { + const h = setup(); + h.error(); + await advance(0); + if (action === "unmount") h.unmount(); + else h.change(); + await advance(7000); + expect(h.element.load).not.toHaveBeenCalled(); + expect(h.failed).not.toHaveBeenCalled(); + expect(vi.getTimerCount()).toBe(0); +}); + +it.each([1, 3, 4])("leaves media error %s explicit", async (code) => { + const h = setup(); + h.error(code); + await advance(7000); + expect(h.failed).toHaveBeenCalledOnce(); + expect(h.element.load).not.toHaveBeenCalled(); +}); diff --git a/src/features/messages/use-media-read-recovery.ts b/src/features/messages/use-media-read-recovery.ts new file mode 100644 index 0000000000..675c5dbf97 --- /dev/null +++ b/src/features/messages/use-media-read-recovery.ts @@ -0,0 +1,91 @@ +import { useEffect, useRef } from "react"; +import { retryRead } from "../relay/read-retry"; + +function mediaFailure(element: HTMLMediaElement): Error { + return element.error?.code === 2 + ? new TypeError("Media network read failed") + : new Error("Media could not be decoded"); +} + +/** Reload bytes after a network failure. Playback permission and decoding + * failures remain explicit; recovery never starts playback itself. */ +export function useMediaReadRecovery( + source: string | undefined, + failed: () => void, + recovered: () => void = () => {}, +) { + const pending = useRef(undefined); + const callbacks = useRef({ failed, recovered }); + callbacks.current = { failed, recovered }; + useEffect(() => { + void source; + return () => { + pending.current?.abort(); + pending.current = undefined; + }; + }, [source]); + const recover = (element: HTMLMediaElement): boolean => { + if (pending.current) return true; + if (!source || element.error?.code !== 2) return false; + const controller = new AbortController(); + pending.current = controller; + const initial = mediaFailure(element); + const preload = element.preload; + let first = true; + void retryRead((signal) => { + // The browser already issued the initial request. Count its failure + // before scheduling the same three quiet retries used by other reads. + if (first) { + first = false; + return Promise.reject(initial); + } + return new Promise((resolve, reject) => { + const finish = (error?: unknown) => { + clearTimeout(timer); + element.removeEventListener("loadedmetadata", ready); + element.removeEventListener("error", errorEvent); + signal.removeEventListener("abort", abort); + if (error) reject(error); + else resolve(); + }; + const ready = () => finish(); + const errorEvent = () => finish(mediaFailure(element)); + const abort = () => finish(signal.reason); + const timer = setTimeout( + () => + finish(new DOMException("Media read timed out", "TimeoutError")), + 10_000, + ); + element.addEventListener("loadedmetadata", ready, { once: true }); + element.addEventListener("error", errorEvent, { once: true }); + signal.addEventListener("abort", abort, { once: true }); + try { + if (preload === "none") element.preload = "metadata"; + element.load(); + } catch (error) { + finish(error); + } + }); + }, controller.signal) + .then( + () => { + if (!controller.signal.aborted) callbacks.current.recovered(); + }, + () => { + if (!controller.signal.aborted) callbacks.current.failed(); + }, + ) + .finally(() => { + element.preload = preload; + if (pending.current === controller) pending.current = undefined; + }); + return true; + }; + return { + recover, + cancel() { + pending.current?.abort(); + pending.current = undefined; + }, + }; +} diff --git a/src/features/relay/channel-details.ts b/src/features/relay/channel-details.ts index 8c45f1d720..8857de2095 100644 --- a/src/features/relay/channel-details.ts +++ b/src/features/relay/channel-details.ts @@ -1,5 +1,6 @@ import { getEventHash, verifyEvent } from "nostr-tools"; import type { RelayReader } from "./reader"; +import { retryRead } from "./read-retry"; import type { RelayWriter } from "./transport"; import type { RelayEvent } from "./events"; import { PublishRejected } from "./outbox"; @@ -140,7 +141,10 @@ export function createChannelDetails({ return owned( id, async (signal) => { - const { details, metadata } = await read(id, signal); + const { details, metadata } = await retryRead( + () => read(id, signal), + signal, + ); acceptDiscovery([metadata]); signal.throwIfAborted(); return details; diff --git a/src/features/relay/channel-lifecycle.test.ts b/src/features/relay/channel-lifecycle.test.ts index 76ab40c4f3..540afa735e 100644 --- a/src/features/relay/channel-lifecycle.test.ts +++ b/src/features/relay/channel-lifecycle.test.ts @@ -3,7 +3,17 @@ import type { RelaySession } from "./session"; import { createHash } from "node:crypto"; import { schnorr } from "@noble/curves/secp256k1.js"; import { bytesToHex } from "nostr-tools/utils"; -import { assert, describe, expect, it, vi } from "vitest"; +import { + afterEach, + beforeEach, + assert, + describe, + expect, + it, + vi, +} from "vitest"; +import { ReadError } from "./errors"; +import { createRecovery } from "./recovery"; import { finalizeEvent, getPublicKey, type EventTemplate } from "nostr-tools"; import { PublishRejected } from "./outbox"; import { @@ -154,6 +164,242 @@ function harness(role = "owner", type = "stream", owners = 1) { }; } +describe("bounded visibility recovery", () => { + beforeEach(() => { + vi.useFakeTimers(); + vi.spyOn(Math, "random").mockReturnValue(0.5); + }); + afterEach(() => { + vi.restoreAllMocks(); + vi.useRealTimers(); + }); + + it("keeps loaded visibility quiet through a transient failure and success", async () => { + const h = harness(); + h.setVisible([ + h.record(30622, [ + ["d", viewer], + ["p", viewer], + ["h", id], + ]), + ]); + await h.owner.capability.refreshVisibility(); + h.read.mockClear(); + h.read.mockRejectedValueOnce(new TypeError("fetch failed")); + const pending = h.owner.capability.refreshVisibility(); + await vi.advanceTimersByTimeAsync(0); + expect(h.owner.capability.snapshot()).toEqual({ + status: "loading", + hidden: [id], + }); + await vi.advanceTimersByTimeAsync(749); + expect(h.read).toHaveBeenCalledOnce(); + await vi.advanceTimersByTimeAsync(1); + await pending; + expect(h.read).toHaveBeenCalledTimes(2); + expect(h.owner.capability.snapshot()).toEqual({ + status: "ready", + hidden: [id], + }); + h.owner.dispose(); + }); + + it("exhausts exactly four reads without restarting through session recovery", async () => { + const h = harness(); + const lifecycle = h.owner.capability; + const recovery = createRecovery([ + { + subscribe: lifecycle.subscribe, + // Match the session adapter: only errors carrying `transient` opt in. + status: () => { + const state = lifecycle.snapshot(); + if ( + state.status === "error" && + "transient" in state && + state.transient + ) + return "failed"; + return state.status === "loading" ? "busy" : "settled"; + }, + retry: () => void lifecycle.refreshVisibility(), + }, + ]); + h.read.mockRejectedValue(new ReadError("unavailable", "offline", 503)); + const pending = lifecycle.refreshVisibility(); + await vi.advanceTimersByTimeAsync(0); + for (const [index, delay] of [750, 1500, 3000].entries()) { + await vi.advanceTimersByTimeAsync(delay - 1); + expect(h.read).toHaveBeenCalledTimes(index + 1); + expect(lifecycle.snapshot().status).toBe("loading"); + await vi.advanceTimersByTimeAsync(1); + } + await pending; + expect(lifecycle.snapshot()).toEqual({ + status: "error", + hidden: [], + error: "offline", + }); + await vi.advanceTimersByTimeAsync(120_000); + expect(h.read).toHaveBeenCalledTimes(4); + h.read.mockResolvedValue([]); + await lifecycle.refreshVisibility(); + expect(lifecycle.snapshot().status).toBe("ready"); + recovery.dispose(); + h.owner.dispose(); + }); + + it.each([ + new ReadError("denied", "denied", 403), + new ReadError("invalid-response", "validation", 400), + new ReadError("invalid-response", "bad response"), + new Error("Invalid DM visibility snapshot"), + ])("surfaces permanent failure immediately: %s", async (error) => { + const h = harness(); + h.read.mockRejectedValue(error); + await h.owner.capability.refreshVisibility(); + expect(h.owner.capability.snapshot()).toMatchObject({ + status: "error", + error: error.message, + }); + await vi.advanceTimersByTimeAsync(120_000); + expect(h.read).toHaveBeenCalledOnce(); + h.owner.dispose(); + }); + + it.each([400, 401, 403, 404])( + "retries transport-classified edge HTTP %s failures", + async (status) => { + const h = harness(); + h.read.mockRejectedValueOnce( + new ReadError("unavailable", "Edge unavailable", status), + ); + const pending = h.owner.capability.refreshVisibility(); + await vi.advanceTimersByTimeAsync(749); + expect(h.read).toHaveBeenCalledOnce(); + expect(h.owner.capability.snapshot().status).toBe("loading"); + await vi.advanceTimersByTimeAsync(1); + await pending; + expect(h.read).toHaveBeenCalledTimes(2); + expect(h.owner.capability.snapshot().status).toBe("ready"); + h.owner.dispose(); + }, + ); + + it("surfaces a long admission pause without holding the refresh pending", async () => { + const h = harness(); + h.read.mockRejectedValueOnce( + new ReadError("unavailable", "quota", 429, 5000), + ); + const pending = h.owner.capability.refreshVisibility(); + await pending; + expect(h.read).toHaveBeenCalledOnce(); + expect(h.owner.capability.snapshot()).toMatchObject({ + status: "error", + error: "quota", + }); + expect(vi.getTimerCount()).toBe(0); + h.owner.dispose(); + }); + + it("coalesces concurrent triggers during reads and backoff", async () => { + const h = harness(); + const gate = deferred(); + h.read.mockImplementationOnce(() => gate.promise); + const first = h.owner.capability.refreshVisibility(); + const concurrent = h.owner.capability.refreshVisibility(); + expect(concurrent).toBe(first); + await vi.advanceTimersByTimeAsync(0); + expect(h.read).toHaveBeenCalledOnce(); + gate.reject(new TypeError("offline")); + await vi.advanceTimersByTimeAsync(0); + expect(h.owner.capability.refreshVisibility()).toBe(first); + await vi.advanceTimersByTimeAsync(750); + await first; + expect(h.read).toHaveBeenCalledTimes(2); + h.owner.dispose(); + }); + + it.each(["caller", "cancel", "clear", "dispose"] as const)( + "%s cancels backoff without a terminal error or another read", + async (action) => { + const h = harness(); + const caller = new AbortController(); + h.read.mockRejectedValue(new TypeError("offline")); + const pending = h.owner.capability.refreshVisibility(caller.signal); + await vi.advanceTimersByTimeAsync(0); + if (action === "caller") caller.abort(); + else h.owner[action](); + await pending; + await vi.advanceTimersByTimeAsync(120_000); + expect(h.read).toHaveBeenCalledOnce(); + expect(h.owner.capability.snapshot()).not.toHaveProperty("error"); + h.owner.dispose(); + }, + ); + + it("aborts an obsolete read and rejects its late response after a superseding request", async () => { + const h = harness(); + const gate = deferred(); + const caller = new AbortController(); + h.read.mockImplementationOnce(() => gate.promise); + const old = h.owner.capability.refreshVisibility(caller.signal); + await vi.advanceTimersByTimeAsync(0); + const signal = h.read.mock.calls[0]?.[1]?.signal; + caller.abort(); + expect(signal?.aborted).toBe(true); + await h.owner.capability.refreshVisibility(); + gate.resolve([ + h.record(30622, [ + ["d", viewer], + ["p", viewer], + ["h", id], + ]), + ]); + await old; + expect(h.owner.capability.snapshot()).toEqual({ + status: "ready", + hidden: [], + }); + h.owner.dispose(); + }); + + it("retries a visibility read invalidated by an unrelated channel denial", async () => { + const h = harness(); + h.read.mockImplementationOnce( + (_filters, options) => + new Promise((_resolve, reject) => { + options?.signal?.addEventListener( + "abort", + () => reject(cancelled()), + { once: true }, + ); + }), + ); + const pending = h.owner.capability.refreshVisibility(); + await vi.advanceTimersByTimeAsync(0); + expect(h.read).toHaveBeenCalledOnce(); + h.owner.cancel([id]); + await vi.advanceTimersByTimeAsync(750); + await pending; + expect(h.read).toHaveBeenCalledTimes(2); + expect(h.owner.capability.snapshot().status).toBe("ready"); + h.owner.dispose(); + }); + + it("keeps the retry budget through channel-scoped invalidation during backoff", async () => { + const h = harness(); + h.read.mockRejectedValueOnce(new TypeError("offline")); + const pending = h.owner.capability.refreshVisibility(); + await vi.advanceTimersByTimeAsync(0); + h.owner.cancel([id]); + await vi.advanceTimersByTimeAsync(750); + await pending; + expect(h.read).toHaveBeenCalledTimes(2); + expect(h.owner.capability.snapshot().status).toBe("ready"); + h.owner.dispose(); + }); +}); + it.each(["archive", "delete", "leave", "hide"] as const)( "confirms %s while ordinary reads still see pre-write state", async (action) => { @@ -608,6 +854,8 @@ it("a clear cannot be repopulated by a late visibility response", async () => { const gate = deferred(); h.read.mockImplementationOnce(() => gate.promise); const pending = h.owner.capability.refreshVisibility(); + await Promise.resolve(); + expect(h.read).toHaveBeenCalledOnce(); h.owner.clear(); gate.resolve([ h.record(30622, [ diff --git a/src/features/relay/channel-lifecycle.ts b/src/features/relay/channel-lifecycle.ts index 3dde527bdd..6b07de0b2f 100644 --- a/src/features/relay/channel-lifecycle.ts +++ b/src/features/relay/channel-lifecycle.ts @@ -4,6 +4,7 @@ import type { RelayReader } from "./reader"; import type { RelayWriter } from "./transport"; import { PublishRejected } from "./outbox"; import { isTransientFailure, readErrorKind } from "./errors"; +import { retryRead } from "./read-retry"; import { hasTag, newer, type RelayEvent } from "./events"; import { openMetadata } from "./discovery"; import { @@ -31,8 +32,6 @@ export type DmVisibility = Readonly<{ status: "idle" | "loading" | "ready" | "error"; hidden: readonly string[]; error?: string; - /** Set with an error that a later retry can fix; see isTransientFailure. */ - transient?: true; }>; export interface ChannelLifecycleCapability { readonly available: boolean; @@ -47,7 +46,7 @@ export interface ChannelLifecycleCapability { ): Promise; snapshot(): DmVisibility; subscribe(listener: () => void): () => void; - refreshVisibility(): Promise; + refreshVisibility(signal?: AbortSignal): Promise; observeRemoval(listener: (channelId: string) => void): () => void; } @@ -81,6 +80,8 @@ export function createChannelLifecycle({ }); let visibility: RelayEvent | undefined; let refresh: Promise | undefined; + let refreshSignal: AbortSignal | undefined; + let refreshController: AbortController | undefined; const available = !!reader && !!writer; const emit = (next: DmVisibility) => { snapshot = Object.freeze(next); @@ -182,9 +183,19 @@ export function createChannelLifecycle({ try { for (let start = 0; start < owners.length; start += 4) { const batch = owners.slice(start, start + 4); - const profiles = await reader.read( - batch.map((pubkey) => ({ kinds: [0], authors: [pubkey], limit: 1 })), - { signal: profileSignal, fresh: true, priority: "foreground" }, + const profiles = await retryRead( + (signal) => + reader.read( + batch.map((pubkey) => ({ + kinds: [0], + authors: [pubkey], + limit: 1, + })), + { signal, fresh: true, priority: "foreground" }, + ), + profileSignal, + (error) => + readErrorKind(error) !== "cancelled" && isTransientFailure(error), ); profileSignal.throwIfAborted(); assertAccess(id); @@ -276,47 +287,73 @@ export function createChannelLifecycle({ listeners.delete(listener); }; }, - refreshVisibility() { - if (refresh) return refresh; - if (!reader || closed) return Promise.resolve(); + refreshVisibility(caller?: AbortSignal) { + if (!reader || closed || caller?.aborted) return Promise.resolve(); + if (refresh && !refreshSignal?.aborted) return refresh; const started = epoch; - emit({ status: "loading", hidden: snapshot.hidden }); - const operation = owned({}, async (signal) => { - await readVisibility(signal); - }) + const controller = new AbortController(); + refreshController = controller; + const signal = AbortSignal.any([ + controller.signal, + ...(caller ? [caller] : []), + ]); + refreshSignal = signal; + // Four reads total. Reuse session recovery's first three backoff ceilings + // (1/2/4s), with equal jitter. Exhausted errors omit `transient` so the + // session recovery coordinator cannot start another retry sequence. + const operation = Promise.resolve() + .then(() => + retryRead( + async () => { + await owned({ caller: signal }, readVisibility); + }, + signal, + isTransientFailure, + ), + ) .catch((error: unknown) => { - if (started === epoch && !closed) + if (started === epoch && !closed && refresh === operation) emit({ - status: "error", + status: signal.aborted + ? visibility + ? "ready" + : "idle" + : "error", hidden: snapshot.hidden, - error: error instanceof Error ? error.message : String(error), - ...(isTransientFailure(error) - ? { transient: true as const } + ...(!signal.aborted + ? { + error: + error instanceof Error ? error.message : String(error), + } : {}), }); }) .finally(() => { - if (refresh === operation) refresh = undefined; + if (refresh === operation) { + refresh = undefined; + refreshController = undefined; + } }); refresh = operation; + emit({ status: "loading", hidden: snapshot.hidden }); return operation; }, async load(value: string, caller?: AbortSignal) { const id = lifecycleChannelId(value); - return owned({ id, caller }, async (signal) => { - for (;;) { - try { - return await load(id, signal); - } catch (error) { - // Owner cancel/clear/dispose, the caller and the deadline all - // arrive through `signal`. Any other cancellation is the session - // retiring shared reads for another channel's access change, - // which is no evidence about this channel: read again. - if (signal.aborted || readErrorKind(error) !== "cancelled") - throw error; + return owned({ id, caller }, (signal) => + retryRead(async () => { + for (;;) { + try { + return await load(id, signal); + } catch (error) { + // Another channel's revocation retires shared reads. It is not + // a failed attempt for this channel; reread under the same deadline. + if (signal.aborted || readErrorKind(error) !== "cancelled") + throw error; + } } - } - }); + }, signal), + ); }, async run( action: ChannelLifecycleCommand, @@ -472,11 +509,15 @@ export function createChannelLifecycle({ capability, /** Abort work on `ids` and id-less visibility reads; without `ids`, everything. */ cancel(ids?: readonly string[]) { + // A channel-scoped invalidation retires a read, not the viewer's whole + // refresh. Its cancellation can retry within the existing budget. + if (!ids) refreshController?.abort(); for (const [controller, id] of controllers) if (!ids || id === undefined || ids.includes(id)) controller.abort(); }, clear() { epoch++; + refreshController?.abort(); for (const controller of controllers.keys()) controller.abort(); refresh = undefined; visibility = undefined; @@ -485,6 +526,7 @@ export function createChannelLifecycle({ dispose() { closed = true; epoch++; + refreshController?.abort(); for (const controller of controllers.keys()) controller.abort(); listeners.clear(); removalListeners.clear(); diff --git a/src/features/relay/directory-session.test.ts b/src/features/relay/directory-session.test.ts index c1c9e71436..586f1ec2a4 100644 --- a/src/features/relay/directory-session.test.ts +++ b/src/features/relay/directory-session.test.ts @@ -196,8 +196,9 @@ describe("endpoint profile search under session ownership", () => { signal: new AbortController().signal, }); const rejected = expect(pending).rejects.toMatchObject({ - kind: "unavailable", + name: "ReadRetriesExhausted", message: "Relay read timed out", + cause: { kind: "unavailable", message: "Relay read timed out" }, }); const request = h.wire.next(); await vi.advanceTimersByTimeAsync(10_000); diff --git a/src/features/relay/emoji-directory.ts b/src/features/relay/emoji-directory.ts index e8e761d00a..cf479a112a 100644 --- a/src/features/relay/emoji-directory.ts +++ b/src/features/relay/emoji-directory.ts @@ -1,3 +1,4 @@ +import { retryRead } from "./read-retry"; import type { UploadedAttachment } from "./attachments"; import { byteSize } from "./budget"; import { @@ -124,9 +125,13 @@ export function createEmojiDirectory( publish({ status: "loading", error: undefined }); pending = (async () => { try { - const events = await reader.read( - [{ kinds: [30030], "#d": [EMOJI_SET], limit: LIMIT }], - { signal: owned.signal, priority: "background" }, + const events = await retryRead( + (signal) => + reader.read([{ kinds: [30030], "#d": [EMOJI_SET], limit: LIMIT }], { + signal, + priority: "background", + }), + owned.signal, ); if (closed || owned.signal.aborted) return; accept(events); diff --git a/src/features/relay/errors.ts b/src/features/relay/errors.ts index 166d31466f..3f49e99b5d 100644 --- a/src/features/relay/errors.ts +++ b/src/features/relay/errors.ts @@ -15,6 +15,16 @@ export class ReadError extends Error { this.name = "ReadError"; } } +/** The read has used its automatic budget; a deliberate new read may retry. */ +export class ReadRetriesExhausted extends Error { + constructor(readonly cause: unknown) { + super(cause instanceof Error ? cause.message : String(cause)); + this.name = "ReadRetriesExhausted"; + } + override toString() { + return String(this.cause); + } +} export function httpReadError(status: number) { return new ReadError( status === 401 || status === 403 ? "denied" : "unavailable", diff --git a/src/features/relay/gifs.ts b/src/features/relay/gifs.ts index d622ea0b24..f87116399c 100644 --- a/src/features/relay/gifs.ts +++ b/src/features/relay/gifs.ts @@ -1,3 +1,5 @@ +import { retryRead } from "./read-retry"; +import { ReadError } from "./errors"; import { nativeIdentityEnabled } from "../identity/service"; import { relayKlipySearchPath, @@ -149,7 +151,15 @@ async function brokerRequest( }); const result = (await response.json()) as T & { error?: string }; if (!response.ok) - throw new Error(result.error ?? `GIF request failed (${response.status})`); + throw new ReadError( + response.status === 408 || + response.status === 429 || + response.status >= 500 + ? "unavailable" + : "invalid-response", + result.error ?? `GIF request failed (${response.status})`, + response.status, + ); return result; } @@ -204,15 +214,19 @@ export async function fetchKlipyGifs( if (nativeIdentityEnabled() && route !== "/gifs/search") throw new Error("GIF search is unavailable"); try { - const response = await brokerRequest( - community, - route, - { - customer_id: customerId(), - locale: navigator.language || "en-US", - query: query.trim(), - }, - signal, + const response = await retryRead( + (signal) => + brokerRequest( + community, + route, + { + customer_id: customerId(), + locale: navigator.language || "en-US", + query: query.trim(), + }, + signal, + ), + signal ?? new AbortController().signal, ); if (response.result === false) throw new Error("GIF search failed"); return normalizeKlipyGifs(response.data?.data ?? []); diff --git a/src/features/relay/global-search.test.ts b/src/features/relay/global-search.test.ts index dace5535f3..d373f48da3 100644 --- a/src/features/relay/global-search.test.ts +++ b/src/features/relay/global-search.test.ts @@ -297,6 +297,8 @@ it("rejects missing/forged/private evidence and purges prior public content on a }); it("surfaces bounded metadata failures instead of silently returning an empty page", async () => { + vi.useFakeTimers(); + vi.spyOn(Math, "random").mockReturnValue(0.5); const viewer = keypair(), relay = keypair(); const owner = createRelaySession({ @@ -310,11 +312,15 @@ it("surfaces bounded metadata failures instead of silently returning an empty pa }, }); try { - await expect(owner.session.read(search)).rejects.toThrow( + const rejected = expect(owner.session.read(search)).rejects.toThrow( "metadata unavailable", ); + await vi.advanceTimersByTimeAsync(5250); + await rejected; } finally { owner.dispose(); + vi.restoreAllMocks(); + vi.useRealTimers(); } }); diff --git a/src/features/relay/identity-archives.ts b/src/features/relay/identity-archives.ts index 4d6ba3a19c..b2b3c8b1fd 100644 --- a/src/features/relay/identity-archives.ts +++ b/src/features/relay/identity-archives.ts @@ -1,3 +1,4 @@ +import { isRetryableRead, retryRead } from "./read-retry"; import { getEventHash } from "nostr-tools"; import { authTagOwner, withinTimeBounds } from "../agents/owner-attestation"; import { byteSize } from "./budget"; @@ -79,9 +80,17 @@ export function createIdentityArchives( void (async () => { try { if (closed || owned.signal.aborted) return; - const events = await reader.read( - [{ kinds: [13535], authors: [authority], limit: 1 }], - { signal: owned.signal, priority: "background", fresh: true }, + const began = performance.now(); + const events = await retryRead( + (signal) => + reader.read([{ kinds: [13535], authors: [authority], limit: 1 }], { + signal, + priority: "background", + fresh: true, + }), + owned.signal, + (error) => + performance.now() - began < 10_000 && isRetryableRead(error), ); if (closed || owned.signal.aborted) return; // The relay publishes its first snapshot with its first archive, so diff --git a/src/features/relay/inbox-feed.ts b/src/features/relay/inbox-feed.ts index c20ae2e03c..8bfd6585a1 100644 --- a/src/features/relay/inbox-feed.ts +++ b/src/features/relay/inbox-feed.ts @@ -1,3 +1,4 @@ +import { retryRead } from "./read-retry"; import { byteSize } from "./budget"; import type { ChannelQueries } from "./contracts"; import type { RelayEvent, ReadFilter } from "./events"; @@ -98,16 +99,20 @@ export function createInboxFeed({ let cursor: ReturnType | undefined; for (;;) { const page = [ - ...(await reader.read( - [ - { - kinds, - "#e": ids, - limit: 500, - ...(cursor ?? {}), - }, - ], - { signal }, + ...(await retryRead( + (signal) => + reader.read( + [ + { + kinds, + "#e": ids, + limit: 500, + ...(cursor ?? {}), + }, + ], + { signal }, + ), + signal, )), ].sort((a, b) => b.created_at - a.created_at || a.id.localeCompare(b.id)); const previous = cursor; @@ -141,19 +146,20 @@ export function createInboxFeed({ "#p": [viewer], limit: 50, }; - const mentions = await addressedRead( - mentionFilter, + const mentions = await retryRead( + (signal) => + addressedRead(mentionFilter, signal, (events) => { + if (closed || generation !== epoch || owned.signal.aborted) + return; + const ids = project(events).map((event) => event.id); + if (ids.length) + publish({ + incomplete: Object.freeze([ + ...new Set([...snapshot.incomplete, ...ids]), + ]), + }); + }), owned.signal, - (events) => { - if (closed || generation !== epoch || owned.signal.aborted) return; - const ids = project(events).map((event) => event.id); - if (ids.length) - publish({ - incomplete: Object.freeze([ - ...new Set([...snapshot.incomplete, ...ids]), - ]), - }); - }, ); if (closed || generation !== epoch) return; // Retry older failed targets even when newer addressed rows pushed them diff --git a/src/features/relay/profile-directory.ts b/src/features/relay/profile-directory.ts index 3f3b136a7e..db50c94967 100644 --- a/src/features/relay/profile-directory.ts +++ b/src/features/relay/profile-directory.ts @@ -1,3 +1,4 @@ +import { retryRead } from "./read-retry"; import { attestedOwner } from "../agents/owner-attestation"; import { hasEventProof } from "./events"; import type { Outbox } from "./outbox"; @@ -30,7 +31,12 @@ export function createProfileDirectory( const owners = new WeakMap(); const pending = new Map< string, - { work: Promise; authors: string[]; priority: Priority } + { + work: Promise; + authors: string[]; + priority: Priority; + reading: boolean; + } >(); const listeners = new Set<() => void>(); let snapshot: ReadonlyMap = new Map(); @@ -116,12 +122,13 @@ export function createProfileDirectory( // waiting on the old promise alone would leave it behind background work. if (priority === "foreground" && batch.priority === "background") { batch.priority = priority; - void reader - .read([{ kinds: [0], authors: batch.authors, limit: 500 }], { - signal, - priority, - }) - .catch(() => {}); + if (batch.reading) + void reader + .read([{ kinds: [0], authors: batch.authors, limit: 500 }], { + signal, + priority, + }) + .catch(() => {}); } waiting.add(batch.work); } @@ -129,20 +136,47 @@ export function createProfileDirectory( }); for (let offset = 0; offset < missing.length; offset += 500) { const authors = missing.slice(offset, offset + 500); - const batch = { + let answered = false; + const batch: { + authors: string[]; + priority: Priority; + reading: boolean; + work: Promise; + } = { authors, priority, - work: reader - .read([{ kinds: [0], authors, limit: 500 }], { signal, priority }) - .then((result) => { - if (!signal.aborted) - accept(result.filter((event) => authors.includes(event.pubkey))); - }) - .finally(() => { - for (const id of authors) - if (pending.get(id) === batch) pending.delete(id); - }), + reading: false, + work: Promise.resolve(), + }; + const request = async (signal: AbortSignal) => { + batch.reading = true; + try { + const result = await reader.read( + [{ kinds: [0], authors, limit: 500 }], + { + signal, + priority: batch.priority, + }, + ); + answered = true; + return result; + } finally { + batch.reading = false; + } }; + batch.work = retryRead(request, signal) + .then((result) => { + if (!signal.aborted) + accept(result.filter((event) => authors.includes(event.pubkey))); + }) + .catch((error: unknown) => { + // The old directory contract silently discards a retired success. + if (!signal.aborted || !answered) throw error; + }) + .finally(() => { + for (const id of authors) + if (pending.get(id) === batch) pending.delete(id); + }); for (const id of authors) pending.set(id, batch); waiting.add(batch.work); } diff --git a/src/features/relay/read-retry.test.ts b/src/features/relay/read-retry.test.ts new file mode 100644 index 0000000000..3b816dba3d --- /dev/null +++ b/src/features/relay/read-retry.test.ts @@ -0,0 +1,156 @@ +import { afterEach, beforeEach, expect, it, vi } from "vitest"; +import { isTransientFailure, ReadError, ReadRetriesExhausted } from "./errors"; +import { retryRead } from "./read-retry"; + +beforeEach(() => { + vi.useFakeTimers(); + vi.spyOn(Math, "random").mockReturnValue(0.5); +}); +afterEach(() => { + vi.restoreAllMocks(); + vi.useRealTimers(); +}); + +it("retries transient reads at the backoff boundaries and returns success", async () => { + const read = vi + .fn() + .mockRejectedValueOnce(new TypeError("offline")) + .mockResolvedValue("loaded"); + const pending = retryRead(read, new AbortController().signal); + await vi.advanceTimersByTimeAsync(749); + expect(read).toHaveBeenCalledOnce(); + await vi.advanceTimersByTimeAsync(1); + expect(await pending).toBe("loaded"); + expect(read).toHaveBeenCalledTimes(2); +}); + +it("exhausts once across nested owners and preserves the original failure", async () => { + const error = new ReadError("unavailable", "offline", 503); + const read = vi.fn().mockRejectedValue(error); + const signal = new AbortController().signal; + const pending = retryRead(() => retryRead(read, signal), signal); + const rejected = expect(pending).rejects.toMatchObject({ + name: "ReadRetriesExhausted", + message: "offline", + cause: error, + }); + for (const [index, delay] of [750, 1500, 3000].entries()) { + await vi.advanceTimersByTimeAsync(delay - 1); + expect(read).toHaveBeenCalledTimes(index + 1); + await vi.advanceTimersByTimeAsync(1); + } + await rejected; + await vi.advanceTimersByTimeAsync(120_000); + expect(read).toHaveBeenCalledTimes(4); + expect(isTransientFailure(new ReadRetriesExhausted(error))).toBe(true); + expect(String(new ReadRetriesExhausted(error))).toBe(String(error)); + expect( + isTransientFailure( + new Error("context", { cause: new ReadRetriesExhausted(error) }), + ), + ).toBe(true); +}); + +it.each([ + new ReadError("denied", "denied", 403), + new ReadError("invalid-response", "invalid"), + new Error("Invalid input"), +])("surfaces permanent failures immediately: %s", async (error) => { + const read = vi.fn().mockRejectedValue(error); + await expect(retryRead(read, new AbortController().signal)).rejects.toBe( + error, + ); + await vi.advanceTimersByTimeAsync(120_000); + expect(read).toHaveBeenCalledOnce(); +}); + +it.each([5000, 60000, 86401000])( + "surfaces long admission delays immediately: %s", + async (delay) => { + const error = new ReadError("unavailable", "quota", 429, delay); + const read = vi.fn().mockRejectedValue(error); + await expect( + retryRead(read, new AbortController().signal), + ).rejects.toMatchObject({ cause: error }); + expect(read).toHaveBeenCalledOnce(); + expect(vi.getTimerCount()).toBe(0); + }, +); + +it("honors admission hints within the bounded backoff", async () => { + const read = vi + .fn() + .mockRejectedValueOnce(new ReadError("unavailable", "quota", 429, 900)) + .mockResolvedValue([]); + const pending = retryRead(read, new AbortController().signal); + await vi.advanceTimersByTimeAsync(899); + expect(read).toHaveBeenCalledOnce(); + await vi.advanceTimersByTimeAsync(1); + await pending; + expect(read).toHaveBeenCalledTimes(2); +}); + +it.each([ + new DOMException("retired", "AbortError"), + new ReadError("cancelled", "cancelled"), + new Error("context", { cause: new DOMException("retired", "AbortError") }), +])("does not retry owner cancellations: %s", async (error) => { + const read = vi.fn().mockRejectedValue(error); + await expect(retryRead(read, new AbortController().signal)).rejects.toBe( + error, + ); + expect(read).toHaveBeenCalledOnce(); + expect(vi.getTimerCount()).toBe(0); +}); + +it("preserves nested exhaustion through context wrappers", async () => { + const signal = new AbortController().signal; + const read = vi.fn().mockRejectedValue(new TypeError("offline")); + const pending = retryRead(async () => { + try { + return await retryRead(read, signal); + } catch (cause) { + throw new Error("context", { cause }); + } + }, signal); + const rejected = expect(pending).rejects.toMatchObject({ + message: "context", + }); + await vi.advanceTimersByTimeAsync(5250); + await rejected; + expect(read).toHaveBeenCalledTimes(4); +}); + +it("aborts backoff and releases the timer", async () => { + const controller = new AbortController(); + const read = vi.fn().mockRejectedValue(new TypeError("offline")); + const pending = retryRead(read, controller.signal); + const rejected = expect(pending).rejects.toMatchObject({ + name: "AbortError", + }); + await vi.advanceTimersByTimeAsync(0); + controller.abort(); + await rejected; + await vi.advanceTimersByTimeAsync(120_000); + expect(read).toHaveBeenCalledOnce(); + expect(vi.getTimerCount()).toBe(0); +}); + +it("rejects late success from a read that ignores cancellation", async () => { + const controller = new AbortController(); + let resolve!: (value: string) => void; + const read = vi.fn( + () => + new Promise((done) => { + resolve = done; + }), + ); + const pending = retryRead(read, controller.signal); + const rejected = expect(pending).rejects.toMatchObject({ + name: "AbortError", + }); + controller.abort(); + resolve("obsolete"); + await rejected; + expect(read).toHaveBeenCalledOnce(); +}); diff --git a/src/features/relay/read-retry.ts b/src/features/relay/read-retry.ts new file mode 100644 index 0000000000..c049b3de2f --- /dev/null +++ b/src/features/relay/read-retry.ts @@ -0,0 +1,64 @@ +import { + isTransientFailure, + readErrorKind, + ReadError, + ReadRetriesExhausted, +} from "./errors.ts"; +import { RECOVERY_DELAYS_MS } from "./recovery.ts"; + +export function isRetryableRead(error: unknown): boolean { + if (readErrorKind(error) === "cancelled") return false; + if (error instanceof Error && error.cause !== undefined) + return isRetryableRead(error.cause); + return isTransientFailure(error); +} + +function exhausted(error: unknown): boolean { + return ( + error instanceof ReadRetriesExhausted || + (error instanceof Error && + error.cause !== undefined && + exhausted(error.cause)) + ); +} + +/** One initial read and three quiet retries. Nested owners share exhaustion, + * rather than multiplying budgets. The caller owns the read's lifetime. */ +export async function retryRead( + read: (signal: AbortSignal) => Promise, + signal: AbortSignal, + retryable: (error: unknown) => boolean = isRetryableRead, +): Promise { + for (let attempt = 0; ; attempt++) { + signal.throwIfAborted(); + try { + const value = await read(signal); + signal.throwIfAborted(); + return value; + } catch (error) { + signal.throwIfAborted(); + if (exhausted(error) || !retryable(error)) throw error; + if (attempt === 3) throw new ReadRetriesExhausted(error); + const ceiling = RECOVERY_DELAYS_MS[attempt] ?? 4000; + // Admission has its own actionable cooldown. Never hide it behind a + // pending read longer than this retry's normal backoff ceiling. + if (error instanceof ReadError && (error.retryAfterMs ?? 0) > ceiling) + throw new ReadRetriesExhausted(error); + const delay = Math.max( + ceiling * (0.5 + Math.random() / 2), + error instanceof ReadError ? (error.retryAfterMs ?? 0) : 0, + ); + await new Promise((resolve, reject) => { + const abort = () => { + clearTimeout(timer); + reject(signal.reason); + }; + const timer = setTimeout(() => { + signal.removeEventListener("abort", abort); + resolve(); + }, delay); + signal.addEventListener("abort", abort, { once: true }); + }); + } + } +} diff --git a/src/features/relay/reminders.ts b/src/features/relay/reminders.ts index ecd2688100..2df309f39b 100644 --- a/src/features/relay/reminders.ts +++ b/src/features/relay/reminders.ts @@ -1,3 +1,4 @@ +import { retryRead } from "./read-retry"; // NIP-ER reminders (kind 30300): the viewer's self-encrypted, replaceable reminder list. // Encryption and signing stay in the host; this owner sees only decoded intent. import type { ReadFilter, RelayEvent } from "./events"; @@ -150,6 +151,7 @@ export function createReminders(options: { // started after (re-)establishment can. Offline reads still load the list. let offline = true; let generation = 0; + let refreshController: AbortController | undefined; let loadingGeneration = -1; const publishState = ( next: Omit, @@ -241,14 +243,21 @@ export function createReminders(options: { refresh() { if (loading && loadingGeneration === generation) return loading; const current = generation; + const controller = new AbortController(); + refreshController = controller; + const readSignal = AbortSignal.any([signal, controller.signal]); loadingGeneration = current; if (state.status !== "loading") publishState({ status: "loading" }); let read: Promise | undefined; read = (async () => { try { - const events = await options.query( - [{ kinds: [REMINDER_KIND], authors: [viewer], limit: 200 }], - signal, + const events = await retryRead( + (signal) => + options.query( + [{ kinds: [REMINDER_KIND], authors: [viewer], limit: 200 }], + signal, + ), + readSignal, ); // A disposed session's late reply, even an empty one, proves nothing. if (signal.aborted) return; @@ -265,6 +274,7 @@ export function createReminders(options: { }); } finally { if (loading === read) loading = undefined; + if (refreshController === controller) refreshController = undefined; } })(); loading = read; @@ -308,6 +318,7 @@ export function createReminders(options: { recover() { offline = false; generation++; + refreshController?.abort(); return capability.refresh(); }, /** Live delivery from another device; failures leave the last good list. */ diff --git a/src/features/relay/session-read-retry.test.ts b/src/features/relay/session-read-retry.test.ts new file mode 100644 index 0000000000..163819c612 --- /dev/null +++ b/src/features/relay/session-read-retry.test.ts @@ -0,0 +1,146 @@ +import { retryRead } from "./read-retry"; +import { afterEach, beforeEach, expect, it, vi } from "vitest"; +import { createRelaySession } from "./session"; +import { ReadRetriesExhausted, ReadError } from "./errors"; +import { keypair, profile, roster } from "./testing"; +import type { LiveCallbacks } from "./live"; +import type { ReadFilter, RelayEvent } from "./events"; + +const owners: ReturnType[] = []; +beforeEach(() => { + vi.useFakeTimers(); + vi.spyOn(Math, "random").mockReturnValue(0.5); +}); +afterEach(() => { + for (const owner of owners.splice(0)) owner.dispose(); + vi.restoreAllMocks(); + vi.useRealTimers(); +}); +function setup() { + const viewer = keypair(), + relay = keypair(); + const query = vi.fn( + async (_filters: readonly ReadFilter[], _signal?: AbortSignal) => + [] as RelayEvent[], + ); + let live!: LiveCallbacks; + const owner = createRelaySession({ + viewer: viewer.pubkey, + relayAuthor: relay.pubkey, + query, + media: () => undefined, + subscribe(callbacks) { + live = callbacks; + return { update() {}, retry() {}, dispose() {} }; + }, + }); + owners.push(owner); + const filter = [{ kinds: [0], authors: [viewer.pubkey], limit: 1 }]; + return { + owner, + query, + viewer, + relay, + filter, + emit: (events: RelayEvent[]) => live.receive(events), + }; +} + +it("retries a finite public read without changing transport edge classification", async () => { + const h = setup(); + h.query.mockRejectedValueOnce(new ReadError("unavailable", "edge", 403)); + const pending = h.owner.session.read(h.filter); + await vi.advanceTimersByTimeAsync(749); + expect(h.query).toHaveBeenCalledOnce(); + await vi.advanceTimersByTimeAsync(1); + await expect(pending).resolves.toEqual([]); + expect(h.query).toHaveBeenCalledTimes(2); +}); + +it("retains observed data and coalesces refreshes during finite recovery", async () => { + const h = setup(); + const row = profile(h.viewer, { name: "Kept" }); + h.query.mockResolvedValueOnce([row]); + const view = h.owner.session.observe(h.filter); + await view.refresh(); + h.query.mockRejectedValueOnce(new TypeError("offline")); + const pending = view.refresh(); + await vi.advanceTimersByTimeAsync(0); + await view.refresh(); + expect(h.query).toHaveBeenCalledTimes(2); + expect(view.snapshot().status).toBe("loading"); + expect(view.snapshot().events.map((event) => event.id)).toContain(row.id); + await vi.advanceTimersByTimeAsync(750); + await pending; + expect(view.snapshot().status).toBe("ready"); + expect(h.query).toHaveBeenCalledTimes(3); + view.dispose(); +}); + +it("leaves roster errors transient for their existing recovery owner", async () => { + const h = setup(); + h.query.mockRejectedValue(new ReadError("unavailable", "offline", 503)); + h.owner.session.channels.refreshList?.(); + await vi.advanceTimersByTimeAsync(0); + expect(h.owner.session.live.snapshot().roster).toMatchObject({ + state: "error", + transient: true, + }); + expect(h.query).toHaveBeenCalledOnce(); +}); + +it.each(["caller", "cache", "dispose", "access"] as const)( + "%s cancels a finite public read's backoff", + async (action) => { + const h = setup(); + h.emit([roster(h.relay, "channel", [h.viewer.pubkey], 10)]); + const caller = new AbortController(); + h.query.mockRejectedValue(new TypeError("offline")); + const pending = h.owner.session.read(h.filter, { signal: caller.signal }); + const rejected = expect(pending).rejects.toMatchObject({ + name: "AbortError", + }); + await vi.advanceTimersByTimeAsync(0); + const before = h.query.mock.calls.filter(([filters]) => + filters[0]?.kinds?.includes(0), + ).length; + if (action === "caller") caller.abort(); + else if (action === "cache") await h.owner.clearCache(); + else if (action === "dispose") h.owner.dispose(); + else h.emit([roster(h.relay, "channel", [], 11)]); + await rejected; + await vi.advanceTimersByTimeAsync(7000); + expect( + h.query.mock.calls.filter(([filters]) => filters[0]?.kinds?.includes(0)), + ).toHaveLength(before); + }, +); + +it("surfaces a long public admission pause without holding the read pending", async () => { + const h = setup(); + const error = new ReadError("unavailable", "quota", 429, 60_000); + h.query.mockRejectedValue(error); + await expect(h.owner.session.read(h.filter)).rejects.toMatchObject({ + cause: error, + }); + expect(h.query).toHaveBeenCalledOnce(); +}); + +it("marks a finite window as exhausted so an outer caller cannot restart it", async () => { + const h = setup(); + h.query.mockImplementation(async () => { + await new Promise((resolve) => setTimeout(resolve, 4000)); + throw new ReadError("unavailable", "offline"); + }); + const signal = new AbortController().signal; + const pending = retryRead( + () => h.owner.session.read(h.filter, { signal }), + signal, + ); + const rejected = expect(pending).rejects.toBeInstanceOf(ReadRetriesExhausted); + await vi.advanceTimersByTimeAsync(20000); + await rejected; + expect(h.query).toHaveBeenCalledTimes(3); + await vi.advanceTimersByTimeAsync(7000); + expect(h.query).toHaveBeenCalledTimes(3); +}); diff --git a/src/features/relay/session.ts b/src/features/relay/session.ts index 167b1c9ea8..04b8e0e266 100644 --- a/src/features/relay/session.ts +++ b/src/features/relay/session.ts @@ -34,7 +34,10 @@ import { import { createAgentActivity } from "../agents/activity"; import { OBSERVER_KIND } from "../agents/observer"; import { createDirectMessages } from "./direct-messages"; -import { createProfileSearch } from "./profile-search"; +import { + type ProfileSearchOptions, + createProfileSearch, +} from "./profile-search"; import { createChannelWrites } from "./channel-writes"; import { inventoryReader } from "../agents/inventory"; import { @@ -91,7 +94,12 @@ import { type EventData, } from "./events"; import { eventVisibility } from "./event-access"; -import { isTransientFailure, ReadError, readErrorKind } from "./errors"; +import { + isTransientFailure, + ReadError, + ReadRetriesExhausted, + readErrorKind, +} from "./errors"; import { PublishRejected, browserOutboxStorage, @@ -101,6 +109,7 @@ import { import { relayPartition, transportPartition } from "./partition"; import { browserThreadFollows } from "./thread-follows"; import { createMessages } from "./messages"; +import { isRetryableRead, retryRead } from "./read-retry"; import { createThreadView } from "./threads"; import { ByteLru } from "./budget"; import { createRelayProfiler } from "./profiling"; @@ -155,6 +164,66 @@ export function createRelaySession( const profiling = options.profiling ?? transport?.profiling ?? createRelayProfiler(); const requests = createRelayReader(transport, { profiling }); + // Finite public consumers have no recovery source. Internal roster, history, + // catch-up and preference reads keep their existing coordinator and cooldowns. + const finiteReads = new Map< + AbortController, + { filters: readonly ReadFilter[] | undefined; backoff: boolean } + >(); + async function finiteRead( + filters: readonly ReadFilter[] | undefined, + settings: ReadOptions | undefined, + work: (signal: AbortSignal) => Promise, + ): Promise { + const controller = new AbortController(); + const signal = AbortSignal.any([ + lifetime.signal, + controller.signal, + ...(settings?.signal ? [settings.signal] : []), + ]); + const operation = { filters, backoff: false }; + finiteReads.set(controller, operation); + const began = performance.now(); + try { + if ( + !transport || + options.cachedOnly || + filters?.some((filter) => + filter["#h"]?.some((id) => channels.queries.get?.(id)?.cached), + ) + ) + return await work(signal); + return await retryRead(async (signal) => { + operation.backoff = false; + try { + return await work(signal); + } catch (error) { + operation.backoff = isRetryableRead(error); + if (operation.backoff && performance.now() - began >= 10_000) + throw new ReadRetriesExhausted(error); + throw error; + } + }, signal); + } finally { + finiteReads.delete(controller); + } + } + function cancelFiniteReads(accessOnly = false) { + // The request scheduler already retires in-flight work. Do not abort a + // response applying its own authority change; its existing epoch guard + // decides whether that response can finish. This owner cancels backoff. + for (const [controller, { filters, backoff }] of finiteReads) + if ( + !accessOnly || + (backoff && + filters && + (!filters.length || + filters.some( + (filter) => !isOwnerInventoryFilter(filter, transport?.viewer), + ))) + ) + controller.abort(); + } let revision = 0; let accessEpoch = 0; let cacheClearEpoch = 0; @@ -304,6 +373,7 @@ export function createRelaySession( revoking++; try { accessEpoch++; + cancelFiniteReads(true); cancelUploads(); // Lifecycle work is keyed by channel and rechecks access after every // read; with known ids, other channels' permission reads need not die. @@ -385,6 +455,8 @@ export function createRelaySession( ) => void, page?: { withRelayCount: true }, ) { + // A null session can never acquire a transport; retrying it cannot recover. + if (!transport) throw new Error("Relay is disconnected"); if ( options.cachedOnly || filters.some((filter) => @@ -553,7 +625,18 @@ export function createRelaySession( const searchProfiles = createProfileSearch(verified); /** Directory reads bypass shared profile admission. Retry one access * invalidation here, where caller/cache/connection lifetimes are known. */ - async function directoryRead( + function directoryRead( + settings: ReadOptions | undefined, + run: () => Promise, + ): Promise { + const generation = liveGeneration; + return finiteRead(undefined, settings, (signal) => { + if (liveGeneration !== generation) + throw new DOMException("Stale directory read", "AbortError"); + return directoryReadOnce({ ...settings, signal }, run); + }); + } + async function directoryReadOnce( settings: ReadOptions | undefined, run: () => Promise, ): Promise { @@ -804,7 +887,9 @@ export function createRelaySession( const repositories = repoDestinations(async (filters, signal) => { const bound = AbortSignal.any([signal, lifetime.signal]); bound.throwIfAborted(); - const events = await requests.reader.read(filters, { signal: bound }); + const events = await finiteRead(filters, { signal: bound }, (signal) => + requests.reader.read(filters, { signal }), + ); bound.throwIfAborted(); // NIP-34/NIP-MP metadata is global; channel tags are associations, not ACLs. return events; @@ -2146,6 +2231,7 @@ export function createRelaySession( local: localViews, clock, canAccess: () => !closed && canAccess(channelId), + canRefresh: () => !closed && canReadRemote(channelId), visible: (events) => events.filter(visibility(events)), notify, }); @@ -2198,7 +2284,12 @@ export function createRelaySession( /** A plugin may request writes from this same interface when the host supports them. */ outbox: writes?.outbox, /** People by profile name only; see `createProfileSearch`. */ - searchProfiles, + searchProfiles: (query: string, settings: ProfileSearchOptions) => + finiteRead( + [{ kinds: [0], search: query, limit: settings.limit }], + settings, + (signal) => searchProfiles(query, { ...settings, signal }), + ), /** A fresh verified remote page under the normal access guards, without * merging live observations or local outbox intent into paging evidence. */ async readRemote(filters: readonly ReadFilter[], settings?: ReadOptions) { @@ -2207,11 +2298,11 @@ export function createRelaySession( ? AbortSignal.any([settings.signal, lifetime.signal]) : lifetime.signal; signal.throwIfAborted(); - const result = await readVerified(filters, { - ...settings, - signal, - fresh: true, - }); + const result = await finiteRead( + filters, + { ...settings, signal }, + (signal) => readVerified(filters, { ...settings, signal, fresh: true }), + ); signal.throwIfAborted(); // Applying discovery can revoke access itself. As with read(), let // authority-only reads finish; mixed/content reads must remain fenced. @@ -2236,12 +2327,13 @@ export function createRelaySession( ): Promise<{ events: readonly VisibleEvent[]; relayCount: number }> { const began = revision; const epoch = accessEpoch; - const { events: result, relayCount } = await readVerified( + const { events: result, relayCount } = await finiteRead( filters, settings, - true, - undefined, - { withRelayCount: true }, + (signal) => + readVerified(filters, { ...settings, signal }, true, undefined, { + withRelayCount: true, + }), ); // Applying discovery can itself revoke access. Let that authority-only // read finish under current visibility; content/search completions must @@ -2458,9 +2550,11 @@ export function createRelaySession( controller = owned; update({ status: "loading", error: undefined }); try { - const events = await verified.read(filters, { - signal: owned.signal, - }); + const read = (signal: AbortSignal) => + verified.read(filters, { signal }); + const events = await (!transport || options.cachedOnly + ? read(owned.signal) + : retryRead(read, owned.signal)); if (disposed || owned.signal.aborted) return; remote = retainEvents([...events, ...remote]); update({ status: "ready" }); @@ -3075,6 +3169,7 @@ export function createRelaySession( accessEpoch++; cancelUploads(); cacheClearEpoch++; + cancelFiniteReads(); dropHintConfirmations(); activity.clear(); memories.clear(); diff --git a/src/features/relay/store.ts b/src/features/relay/store.ts index 8b7e7c8fba..8f3b831721 100644 --- a/src/features/relay/store.ts +++ b/src/features/relay/store.ts @@ -1,3 +1,4 @@ +import { retryRead } from "./read-retry"; import { channelRowKind, rowProfileIds, messagePreview } from "./membership"; import type { Outbox } from "./outbox"; import { MessageProjection } from "./message-projection"; @@ -576,10 +577,17 @@ export function createChannelStore( settings: ReadOptions, ) { if (!transport) throw new Error("Relay is unavailable"); - const events = await transport.read( - [windowFilter(channelId, cursor)], - settings, - ); + const source = transport; + const read = (signal?: AbortSignal) => + source.read([windowFilter(channelId, cursor)], { + ...settings, + ...(signal ? { signal } : {}), + }); + // Head reads belong to session recovery; only manual pagination needs a + // finite retry owner here. + const events = cursor + ? await retryRead(read, settings.signal ?? new AbortController().signal) + : await read(); return { events, page: parseWindow(channelId, cursor, transport.relayAuthor, events), diff --git a/src/features/relay/thread-window.test.ts b/src/features/relay/thread-window.test.ts index 2b12e3b911..5afcaeb0c0 100644 --- a/src/features/relay/thread-window.test.ts +++ b/src/features/relay/thread-window.test.ts @@ -377,7 +377,10 @@ it.each([401, 403, 429, 500])( ? Response.json({ error: "fixture failure" }, { status }) : original(url, options); }); - await h.view.refresh(); + vi.useFakeTimers(); + const pending = h.view.refresh(); + await vi.advanceTimersByTimeAsync(7000); + await pending; if (status === 401 || status === 403) { expect(h.view.snapshot()).toMatchObject({ status: "idle", @@ -392,6 +395,7 @@ it.each([401, 403, 429, 500])( await h.view.refresh(); expect(h.view.snapshot().status).toBe("ready"); } + vi.useRealTimers(); }, ); diff --git a/src/features/relay/threads.ts b/src/features/relay/threads.ts index 05b0c7d139..be1193a2fe 100644 --- a/src/features/relay/threads.ts +++ b/src/features/relay/threads.ts @@ -1,3 +1,4 @@ +import { isRetryableRead, retryRead } from "./read-retry"; import { canonicalChannel, MissingThreadBounds, @@ -70,12 +71,13 @@ export function createThreadView({ relayAuthor, signingAuthority, agentOwner, - reader, - targetReader = reader, + reader: sourceReader, + targetReader: sourceTargetReader = sourceReader, admitTarget, seed, local, canAccess, + canRefresh = canAccess, visible, notify, exact = false, @@ -94,6 +96,8 @@ export function createThreadView({ seed: RelayEvent | undefined; local: LocalEvents | undefined; canAccess(): boolean; + /** Cached membership can display saved rows but cannot authorize a network retry. */ + canRefresh?(): boolean; visible(events: readonly RelayEvent[]): readonly RelayEvent[]; notify(listener: () => void): void; exact?: boolean; @@ -104,6 +108,18 @@ export function createThreadView({ | ((events: readonly RelayEvent[]) => readonly RelayEvent[]) | undefined; }) { + // These finite thread reads have no session recovery source. Their existing + // selection/page controllers still own cancellation and stale-result fencing. + const retrying = (source: RelayReader): RelayReader => ({ + read: (filters, settings) => + retryRead( + (signal) => source.read(filters, { ...settings, signal }), + settings?.signal ?? new AbortController().signal, + (error) => canRefresh() && isRetryableRead(error), + ), + }); + const reader = retrying(sourceReader); + const targetReader = retrying(sourceTargetReader); const acceptTarget = admitTarget ?? admit; let disposed = false; let rootId: string | undefined; diff --git a/src/features/repos/git-read-retry.test.ts b/src/features/repos/git-read-retry.test.ts new file mode 100644 index 0000000000..d96507ecde --- /dev/null +++ b/src/features/repos/git-read-retry.test.ts @@ -0,0 +1,92 @@ +import { afterEach, beforeEach, expect, it, vi } from "vitest"; +import { entityFailure } from "./destinations"; +import { repoGitHost } from "./git"; + +beforeEach(() => { + vi.useFakeTimers(); + vi.spyOn(Math, "random").mockReturnValue(0.5); +}); +afterEach(() => { + vi.restoreAllMocks(); + vi.useRealTimers(); +}); +const input = { owner: "a".repeat(64), dtag: "repo" }; +const empty = { + head: null, + commits: [], + files: [], + readme: null, + file: null, + diff: null, +}; +it("retries immutable Git reads and still validates their returned snapshot", async () => { + const request = vi + .fn() + .mockResolvedValueOnce(new Response(null, { status: 502 })) + .mockImplementation(async () => Response.json(empty)); + const pending = repoGitHost(request).read( + input, + new AbortController().signal, + ); + await vi.advanceTimersByTimeAsync(749); + expect(request).toHaveBeenCalledOnce(); + await vi.advanceTimersByTimeAsync(1); + await expect(pending).resolves.toEqual(empty); + expect(request).toHaveBeenCalledTimes(2); + for (const [body] of request.mock.calls) expect(body).toEqual(input); +}); +it.each([401, 403])( + "does not retry Git authorization status %s", + async (status) => { + const request = vi.fn().mockResolvedValue(new Response(null, { status })); + await expect( + repoGitHost(request).read(input, new AbortController().signal), + ).rejects.toMatchObject({ kind: "denied" }); + expect(request).toHaveBeenCalledOnce(); + }, +); +it("rejects a successful but invalid Git snapshot without retrying", async () => { + const request = vi + .fn() + .mockImplementation(async () => + Response.json({ ...empty, files: [{ path: "x", hash: "bad", size: 1 }] }), + ); + await expect( + repoGitHost(request).read(input, new AbortController().signal), + ).rejects.toThrow("Invalid repository response"); + expect(request).toHaveBeenCalledOnce(); +}); + +it.each(["row", "utf8"])( + "does not retry malformed Git wire data: %s", + async (kind) => { + const request = vi + .fn() + .mockImplementation(async () => + kind === "row" + ? Response.json({ ...empty, head: "a".repeat(40), files: [null] }) + : new Response(Uint8Array.of(255)), + ); + const pending = repoGitHost(request).read( + input, + new AbortController().signal, + ); + const rejected = expect(pending).rejects.toBeInstanceOf(Error); + await vi.advanceTimersByTimeAsync(7000); + await rejected; + expect(request).toHaveBeenCalledOnce(); + }, +); + +it("preserves Git not-found without consuming a retry budget", async () => { + const request = vi + .fn() + .mockImplementation(async () => new Response(null, { status: 404 })); + const error = await repoGitHost(request) + .read(input, new AbortController().signal) + .catch((error: unknown) => error); + expect(entityFailure(error)).toBe("not-found"); + expect(error).toMatchObject({ status: 404 }); + await vi.advanceTimersByTimeAsync(7000); + expect(request).toHaveBeenCalledOnce(); +}); diff --git a/src/features/repos/git.ts b/src/features/repos/git.ts index fa231732fb..9d7f7dcfe4 100644 --- a/src/features/repos/git.ts +++ b/src/features/repos/git.ts @@ -1,4 +1,5 @@ import { ReadError } from "../relay/errors.ts"; +import { isRetryableRead, retryRead } from "../relay/read-retry.ts"; import { entityDtag, entityHex, gitHash } from "./routes.ts"; export type GitRead = { @@ -91,16 +92,23 @@ export async function gitReadText(response: Response) { if (!response.body) throw new Error("Git response missing"); const reader = response.body.getReader(); const decoder = new TextDecoder("utf-8", { fatal: true }); + const decode = (chunk?: Uint8Array, stream = false) => { + try { + return decoder.decode(chunk, { stream }); + } catch { + throw new ReadError("invalid-response", "Invalid repository response"); + } + }; let bytes = 0, text = ""; try { while (true) { const { value, done } = await reader.read(); - if (done) return text + decoder.decode(); + if (done) return text + decode(); bytes += value.byteLength; if (bytes > GIT_READ_BYTES) throw new Error("Git response exceeds the size limit"); - text += decoder.decode(value, { stream: true }); + text += decode(value, true); } } finally { await reader.cancel().catch(() => {}); @@ -114,58 +122,82 @@ export function repoGitHost( return { async read(input, signal) { const body = parseGitRead(input); - const response = await request(body, signal); - if (!response.ok) - throw new ReadError( - response.status === 403 || response.status === 401 - ? "denied" - : "unavailable", - "Repository content could not be read", - response.status, - ); - const value = JSON.parse(await gitReadText(response)) as GitSnapshot; + const text = await retryRead( + async (signal) => { + const response = await request(body, signal); + if (!response.ok) + throw new ReadError( + response.status === 403 || response.status === 401 + ? "denied" + : "unavailable", + "Repository content could not be read", + response.status, + ); + return gitReadText(response); + }, + signal, + (error) => { + // Git's existing missing/invalid request classification is actionable. + // This is endpoint-specific; relay edge errors keep their kind taxonomy. + if ( + error instanceof ReadError && + error.status !== undefined && + error.status >= 400 && + error.status < 500 && + error.status !== 408 && + error.status !== 429 + ) + return false; + return isRetryableRead(error); + }, + ); signal.throwIfAborted(); - if ( - !value || - (value.head !== null && !gitHash.test(value.head)) || - (body.commit && value.head !== body.commit) || - !Array.isArray(value.commits) || - value.commits.length > 100 || - value.commits.some( - (c) => - !gitHash.test(c.hash) || - typeof c.author !== "string" || - !Number.isSafeInteger(c.date) || - typeof c.subject !== "string", - ) || - !Array.isArray(value.files) || - value.files.length > 20000 || - value.files.some( - (f) => - typeof f.path !== "string" || - !gitHash.test(f.hash) || - !Number.isSafeInteger(f.size) || - f.size < 0, - ) || - (value.head === null && - (value.commits.length > 0 || - value.files.length > 0 || - value.readme !== null || - value.file !== null || - value.diff !== null)) || - (value.readme !== null && typeof value.readme !== "string") || - (value.diff !== null && typeof value.diff !== "string") || - (body.commit && value.diff === null) || - (body.path && value.file?.path !== body.path) || - (value.file !== null && - (typeof value.file.path !== "string" || - !Number.isSafeInteger(value.file.size) || - value.file.size < 0 || - (value.file.content !== null && - typeof value.file.content !== "string"))) - ) - throw new Error("Invalid repository response"); - return value; + try { + const value = JSON.parse(text) as GitSnapshot; + if ( + !value || + (value.head !== null && !gitHash.test(value.head)) || + (body.commit && value.head !== body.commit) || + !Array.isArray(value.commits) || + value.commits.length > 100 || + value.commits.some( + (c) => + !gitHash.test(c.hash) || + typeof c.author !== "string" || + !Number.isSafeInteger(c.date) || + typeof c.subject !== "string", + ) || + !Array.isArray(value.files) || + value.files.length > 20000 || + value.files.some( + (f) => + typeof f.path !== "string" || + !gitHash.test(f.hash) || + !Number.isSafeInteger(f.size) || + f.size < 0, + ) || + (value.head === null && + (value.commits.length > 0 || + value.files.length > 0 || + value.readme !== null || + value.file !== null || + value.diff !== null)) || + (value.readme !== null && typeof value.readme !== "string") || + (value.diff !== null && typeof value.diff !== "string") || + (body.commit && value.diff === null) || + (body.path && value.file?.path !== body.path) || + (value.file !== null && + (typeof value.file.path !== "string" || + !Number.isSafeInteger(value.file.size) || + value.file.size < 0 || + (value.file.content !== null && + typeof value.file.content !== "string"))) + ) + throw new Error("Invalid repository response"); + return value; + } catch { + throw new ReadError("invalid-response", "Invalid repository response"); + } }, }; } diff --git a/tests/browser/hidden-conversation-retry.spec.mjs b/tests/browser/hidden-conversation-retry.spec.mjs new file mode 100644 index 0000000000..fbea78de35 --- /dev/null +++ b/tests/browser/hidden-conversation-retry.spec.mjs @@ -0,0 +1,86 @@ +import { openPage } from "./navigation.mjs"; +import { test, expect } from "./fixture.mjs"; + +test.use({ + productionBroker: true, + channelLifecycle: true, + pluginFixtures: true, + historyCounts: { alpha: 1, beta: 1 }, +}); + +// App/session/broker wiring must expose exhausted visibility reads on the +// sidebar and let the existing action recover. Timing/error matrices stay in +// service and mounted component tests. +test("hidden-conversation exhaustion exposes the existing action and manual retry recovers", async ({ + page, + app, +}) => { + let failing = true; + let reads = 0; + await page.goto(app.origin); + await openPage(page, "Messages"); + // Startup can supersede visibility reads as roster evidence arrives. Count + // one explicit visibility refresh after the live roster and visibility settle. + await expect + .poll(() => + page.evaluate(() => { + const session = window.fixtureRelay.snapshot().session; + return [ + session.live.snapshot().status, + session.channels.list().status, + session.channelLifecycle.snapshot().status, + ]; + }), + ) + .toEqual(["connected", "ready", "ready"]); + await page.route("**/api/relay/**/query", async (route) => { + if ( + !route + .request() + .postDataJSON() + .some((filter) => filter.kinds?.includes(30622)) + ) + return route.continue(); + reads++; + if (!failing) return route.continue(); + app.report.startupFailures ??= []; + app.report.startupFailures.push(route.request().url()); + return route.fulfill({ + status: 502, + json: { error: "Fixture visibility unavailable" }, + }); + }); + await page.evaluate(() => + window.fixtureRelay.snapshot().session.channelLifecycle.refreshVisibility(), + ); + const retry = page.getByRole("button", { + name: "Retry hidden conversations", + exact: true, + }); + await expect(retry).toBeVisible(); + expect(reads).toBe(4); + await expect( + page.getByRole("alert").filter({ + hasText: "Hidden conversations could not be refreshed.", + }), + ).toBeVisible(); + expect( + await page.evaluate( + () => + window.fixtureRelay.snapshot().session.recovery.snapshot().recovering, + ), + ).toBe(false); + failing = false; + await retry.click(); + await expect + .poll(() => + page.evaluate( + () => + window.fixtureRelay.snapshot().session.channelLifecycle.snapshot() + .status, + ), + ) + .toBe("ready"); + await expect(retry).toHaveCount(0); + expect(reads).toBe(5); +}); From f5df71575318099be81eb8ec83189bb12bd8a1f4 Mon Sep 17 00:00:00 2001 From: Kalvin Chau Date: Wed, 28 Oct 2026 09:12:58 -0700 Subject: [PATCH 2/3] refactor: consolidate finite read retries to one owner per path drop the nested retry budget marker and per-read elapsed caps; retryRead now owns a 10s quiet window. share transientStatus across gifs, github, builderlab, relay-staff and git. retire session backoffs with the same rule the scheduler uses for in-flight reads. revert provider-catalog, codex, claude-code, goose, acp and profile-settings retry plumbing and the surface inventory doc. consolidate duplicated tests. Co-authored-by: peon <9ac6794b000690b7e814eb1805ad32405d0bec7d52838de3a86cf967565dacc0@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 Signed-off-by: Kalvin Chau --- docs/relay-queries.md | 66 +++------- docs/retry-surfaces.md | 85 ------------- src/app/App.profile.test.tsx | 1 - src/app/ProfileSettings.avatar.test.tsx | 12 +- src/app/ProfileSettings.test.tsx | 12 +- src/app/ProfileSettings.tsx | 36 ++---- src/bundled/builderlab/agents/client.test.ts | 2 +- src/bundled/builderlab/agents/client.ts | 5 +- .../channels/ChannelMembersDialog.test.tsx | 6 +- src/bundled/claude-code/setup.ts | 19 +-- src/bundled/codex/index.test.ts | 105 --------------- src/bundled/codex/index.ts | 2 +- src/bundled/codex/rpc.test.ts | 22 +--- src/bundled/codex/rpc.ts | 4 +- src/bundled/codex/tabs.test.tsx | 95 +------------- src/bundled/codex/tabs.tsx | 58 ++------- src/bundled/github/data.test.tsx | 63 ++++----- src/bundled/github/data.ts | 34 ++--- src/bundled/github/object.ts | 2 +- src/bundled/goose/tabs.tsx | 5 +- src/bundled/pi/tabs.tsx | 6 +- src/bundled/relay-staff/session.tsx | 13 +- src/features/agent-harness/acp/process.ts | 7 +- src/features/agents/memory.ts | 2 +- src/features/communities/CommunityDialog.tsx | 56 +++----- src/features/communities/api.ts | 21 ++- src/features/communities/native-api.test.ts | 6 +- .../communities/profile-read-retry.test.ts | 77 ----------- src/features/relay/channel-lifecycle.test.ts | 73 +---------- src/features/relay/channel-lifecycle.ts | 27 ++-- src/features/relay/directory-session.test.ts | 3 +- src/features/relay/errors.ts | 12 +- src/features/relay/gifs.ts | 18 +-- src/features/relay/identity-archives.ts | 5 +- src/features/relay/profile-directory.ts | 18 +-- src/features/relay/read-retry.test.ts | 75 ++++++----- src/features/relay/read-retry.ts | 49 +++---- src/features/relay/reminders.ts | 2 +- src/features/relay/session-read-retry.test.ts | 58 +-------- src/features/relay/session.ts | 111 +++++++--------- src/features/repos/git-read-retry.test.ts | 31 ----- src/features/repos/git.ts | 120 ++++++++---------- tests/browser/history-loading.spec.mjs | 5 +- 43 files changed, 333 insertions(+), 1096 deletions(-) delete mode 100644 docs/retry-surfaces.md delete mode 100644 src/features/communities/profile-read-retry.test.ts diff --git a/docs/relay-queries.md b/docs/relay-queries.md index f518c53293..141779696f 100644 --- a/docs/relay-queries.md +++ b/docs/relay-queries.md @@ -367,56 +367,22 @@ unconfirmed join reports that it may have taken effect and stays retryable. ### Finite read recovery -Finite public reads and retained-view refreshes use one initial attempt and up to -three quiet retries through `read-retry.ts`. This includes search, directory and -profile/catalog consumers, thread and older-history pages, config history, -reminders, repository/Git reads, read-only staff routes and provider discovery. -Callers retain their existing cancellation, access, validation and deadline -guards. Typed transient failures follow the transport's error kinds; caller -cancellation, denied/invalid responses and deterministic local failures do not -retry. Sessions constructed without a transport cannot recover by retrying. - -The first three recovery ceilings are 1/2/4 seconds with equal jitter. Long relay -admission pauses surface to the existing cooldown/action owner rather than -holding a read pending. Exhaustion preserves the original failure and stops a -nested finite caller from starting another budget. It does not change the shared -underlying error taxonomy. - -Internal roster, head, catch-up, preferences and template reads retain their -existing session recovery coordinator. The finite public boundary does not wrap -`readVerified` or `relayRead` internally. Permission reads retired by another -channel's access change reread immediately under their existing deadline, outside -the transient-failure budget. Public reads in backoff are retired on access/cache -changes; a successful response applying its own authority change retains its -original completion guards. - -Media reloads retry only the browser's explicit network-error code. Decode, -unsupported-source and playback-permission failures retain explicit controls. -Reload recovery does not call `play()`. Writes, file saves, interactive prompts, -plugin/crash recovery and ambiguous native setup failures retain their existing -explicit actions. Existing config-sync, usage, avatar, pairing, update and live-reconnect -retry owners are unchanged. - -### Hidden-conversation refresh - -Visibility refreshes retain the last confirmed hidden IDs while loading. One -refresh makes at most four reads: the initial read and three background retries -for transient network/availability failures. Retry ceilings reuse session recovery's -1, 2 and 4 second delays, with equal jitter (50–100% of each ceiling); a relay -admission hint is honored within that ceiling. A longer admission pause surfaces -immediately so the existing cooldown and Retry action remain available. Each read -retains the reader's deadline. Authorization, validation and invalid-response failures surface immediately. -Retry eligibility follows the existing shared error taxonomy, including transient -edge/proxy HTTP errors; status alone does not establish a permanent relay failure. -Exhaustion exposes the existing “Hidden conversations could not be refreshed.” / -“Retry hidden conversations” action; these terminal errors do not opt into the -session's indefinite recovery coordinator. Manual retry or a later completed roster -refresh starts a new bounded refresh. Concurrent triggers share the current refresh, -including its backoff. Sidebar unmount, superseding roster refreshes, session changes, -session-wide cancellation and cache clearing abort obsolete reads and timers; -channel-scoped access invalidation retires the current read and retries within the -same budget. Late results cannot replace current visibility. Lifecycle writes keep their separate confirmation -and explicit-retry rules. +A finite read that has no session recovery source retries transient failures +quietly through `read-retry.ts` before its existing Retry action appears. That +covers public search, directory, thread, older-history, profile, catalog, +repository, staff and media reads. The budget is one read plus three retries, +with equal jitter under session recovery's first three ceilings (1/2/4 seconds). +An admission hint is honored within that ceiling; a longer pause surfaces +immediately to the existing cooldown. Exhaustion rethrows the last failure. +Access revocation and cache clears end a session read's backoff under the same +rule that retires its in-flight requests. + +Only one owner retries a given read path. Internal roster, head, catch-up, +preference and template reads keep the session recovery coordinator. Callers +keep their cancellation, access-epoch and deadline guards. Denied, invalid and +deterministic local failures, cancellations and writes never retry. Hidden +conversations keep the last confirmed IDs while refreshing; after exhaustion +the error does not opt into indefinite session recovery. ### Channel links diff --git a/docs/retry-surfaces.md b/docs/retry-surfaces.md deleted file mode 100644 index 3ac0f2910d..0000000000 --- a/docs/retry-surfaces.md +++ /dev/null @@ -1,85 +0,0 @@ -# User-facing retry surfaces - -Transient **reads** recover quietly before their existing Retry control appears. -The finite policy is one initial attempt and three retries, with equal jitter -under 1/2/4-second ceilings. Existing background recovery owners remain in place. -See [read recovery policy](relay-queries.md#finite-read-recovery). - -This inventory maps controls to operations; it does not authorize automatic -replay of every action labelled Retry. Authorization, validation, admission -cooldowns, local setup failures and ambiguous native failures remain actionable. -No UI copy or control style changes are required. - -## Finite read owners - -| Controls / consumers | Existing operation owner | Recovery | -| --- | --- | --- | -| Hidden conversations | `relay/channel-lifecycle.ts`, `ChannelInventory.tsx` | Four-attempt refresh; retain confirmed hidden IDs. Scoped invalidation keeps the same budget. | -| Retry channel permissions / Delete check | `relay/channel-lifecycle.ts` | Retry authority reads only. Unrelated shared-read retirement rereads immediately under the same deadline without spending a failure attempt. | -| Channel settings load | `relay/channel-details.ts` | Retry `load`; save and save-confirmation remain separate explicit operations. | -| Channel member/participant/role lists | `channel-members/administration.ts` | Retry `refresh`; membership mutations are single operations. Names use the profile directory. | -| Retry profile / Retry users / member and manager names | `relay/profile-directory.ts` | Retry missing-profile batches, retaining existing heads and coalescing callers. Foreground promotion during backoff changes the next attempt's priority. | -| Profile settings / community profile inspection | `communities/api.ts`, `ProfileSettings.tsx` | Session reads keep their single finite owner; direct-host reads own a bounded retry. Each profile read retains a 12-second bound and a caller signal. Save remains explicit; dispatched confirmation keeps its own bounded lifetime. | -| Retry people / community search / recipient directory | `session.ts` public search and directory boundary | Finite retries; preserve the existing single immediate access-invalidation reread, cache/session cancellation and request deadline. | -| Retry messages in search, including later pages | `session.ts` public `readPage` / `readRemote` | Finite retries at the public consumer boundary. Internal verified readers are single-attempt. | -| Retry channels / channel completion search | `channels/usePublicChannelSearch.ts` | Retry the finite name lookup under its typing/scope controller. The channel store retains the prior useful directory. | -| Agent browse, relay notes, Huddle discussions, profile observations, workflow lists/landing/runs | `session.ts` public reads and `observe().refresh()` | Finite retries; retained views keep loaded data while refreshing and coalesce active refreshes. | -| Retry inbox | `relay/inbox-feed.ts` | Retry addressed history and overlay reads; retain incomplete-target fences and existing access/purge controllers. | -| Retry reminders | `relay/reminders.ts` | Retry the history query; reconnect supersedes its read generation. Reminder create/snooze/complete/cancel remain explicit writes. | -| Retry thread / media comments | `relay/threads.ts` | Retry finite selection, root and page reads under existing controllers. Cached membership does not authorize a network retry. | -| Retry older messages | `relay/store.ts` pagination | Retry continuation reads only; initial/head reads retain session recovery. Loaded rows remain available. | -| Retry emoji / custom emoji catalog | `relay/emoji-directory.ts` | Retry catalog refresh; publishing sets and uploads remain explicit. | -| Retry GIF search | `relay/gifs.ts` | Retry typed network/408/429/5xx read failures; unavailable capability and invalid provider data remain explicit. | -| Retry agents / agent library | `agents/library.ts`, `agents/choices.ts` | Retry the read-only library source. Choices continue to project existing sources rather than introduce another read loop. | -| Retry archive information | `relay/identity-archives.ts` | Retry the visibility snapshot within its existing deadline; invalid or truncated authority is never treated as an empty archive. | -| Retry loading memories | `agents/memory.ts` mounted views | Retry the host read within its existing ten-second lifetime. Decode/permission failures remain explicit. Runtime memory reads keep their separate owner. | -| Retry older config revisions | `agents/agent-config-history.ts` | Retry the requested history page; retain prior revisions on failure. | -| Repository metadata / Retry files / commits | `session.ts` repository read boundary, `repos/git.ts` | Retry immutable metadata/Git reads. Shared browser/native validation still owns destination, signatures, bounds and snapshot parsing. Missing/invalid Git requests remain immediate, including 404 not-found. | -| GitHub panel / PR content | `bundled/github/data.ts`, `object.ts` | Retry public API network/408/5xx and typed GraphQL internal/unavailable failures. Public quota, auth, missing objects and ambiguous CLI setup remain actionable. | -| Staff lists, searches, details and read-only lookups | `bundled/relay-staff/session.tsx` | Explicit read-route allowlist; retry typed intercepted/ambiguous transport failures. Scope cleanup suppresses obsolete results and future requests. | -| Staff attachment preview | `bundled/relay-staff/Feedback.tsx` | Retry reading preview bytes. Native Save remains explicit. | -| Retry models / provider catalogs / read-only history/config tabs | Goose/Pi tab read owners; Claude setup; Codex catalog owner; Builderlab read-route owner | Retry typed transport/timeouts or stopped Pi-host reads. Preserve auth, validation and setup errors. Builderlab limits this to list/catalog endpoints. Codex shared model listing stays single-attempt. Initial turns also await catalog discovery: its retries share a 30-second deadline, and each turn can cancel its own wait without cancelling the shared check. | -| Retry voice note / video resource loading | `messages/use-media-read-recovery.ts` | Browser network-error code 2 gets three reloads after the original failed load. Closing, source changes and voice-note cancellation stop future reloads. No call to `play()` is added. | - -Paths above are relative to `src/features/` unless a bundled/app component is -named. Staff native APIs do not expose in-flight abort: an already-issued request -can finish after cleanup, but its result and further retries are discarded. - -## Existing automatic owners retained - -| Controls / consumers | Owner and disposition | -| --- | --- | -| Retry connection / Retry live updates / Retry now | `relay/service.ts`, live transport and `relay/recovery.ts`; existing indefinite background recovery stays intact. | -| Retry channels in sidebar/profile; initial conversation messages; catch-up heads | `relay/store.ts`, session live recovery; preserve transient state, cooldown and reconnect-triggered repair. | -| Retry templates / teams / sidebar preferences | Channel-kit and preferences recovery sources; no second finite wrapper around their internal reads. | -| Status editor readback | `relay/user-status.ts`; retain existing bounded readback retries and explicit status publication. | -| Retry config/workspace/team reads | `agents/config-sync.ts`; retain its existing bounded retry owner around lookup and current/head reads. Shared artifact/read helpers stay single-attempt. Save and revert remain explicit. | -| Usage refresh / Try again | `bundled/usage/UsagePage.tsx`; retain its existing single 500 ms retry rather than stack another owner. | -| Avatars | Shared Avatar recovery at 2/10/30 seconds; retain that owner. | -| Pairing checks | Existing polling owner; permission/authentication and pairing confirmation remain explicit. | -| App update checks | `updates/updates.ts`; retain scheduled checks. Download/install/relaunch remain explicit lifecycle actions. | - -## Explicit actions retained - -| Controls / consumers | Reason | -| --- | --- | -| Send, Outbox, reaction, edit, delete, report, moderation, invitations, feedback | Mutations or ambiguous delivery. Existing durable operation IDs and explicit confirmation/retry remain authoritative. | -| Channel creation/join, hide/unhide, sort/filter/move, Canvas/todos, unread updates, agent configuration/sharing/archive/stop, workspace actions | User intent or persistence/publication; no automatic replay. Read-only precursors use the owners above. | -| File/media downloads, native Save, copying links/images, avatar/upload retries | User-directed local writes, OS dialogs, clipboard or uploads. Preview/data reads are covered separately. | -| Appearance/notification/shortcut/search/developer settings, local drafts and repository-folder choices | Local persistence or device configuration; unknown native failures do not establish a recoverable network read. | -| Secure storage, identity restore/reveal/setup, OAuth sign-in, microphone/playback permission, local provider installation | Authentication, permission or setup can require interaction; do not repeat prompts or installation silently. | -| Media errors 1/3/4, unsupported source/listener, playback rejection | Cancellation, decoding, unsupported resource or browser permission; only explicit media network failures receive reload recovery. | -| Plugin import/install/update/rollback/delete, page/sidebar/plugin/module crash recovery, navigation/window-opening failure | Code/process lifecycle or interactive setup; preserve explicit recovery. Read-only navigation data uses the owners above. | - -## Validation boundaries - -Deterministic tests cover shared budgets, nested exhaustion, admission pauses, -caller/cache/access/dispose cancellation, retained views, staff outcomes, immutable -Git validation, provider read versus write replay, and media reload cleanup. -Existing manual-recovery presentation tests use terminal failures; transient API -failures have separate owner tests with controlled clocks. - -The representative app journeys exercise hidden-conversation exhaustion/manual -retry and relay recovery from edge/VPN errors in Chromium and WebKit. These browser -checks do not establish native signing/IPC, a real relay, native media playback, -or every screen/OS permission flow. diff --git a/src/app/App.profile.test.tsx b/src/app/App.profile.test.tsx index dddeb4c97c..97f74dd22e 100644 --- a/src/app/App.profile.test.tsx +++ b/src/app/App.profile.test.tsx @@ -624,7 +624,6 @@ it("reopens profile settings from the same session cache while refreshing", asyn expect(refresh).toHaveBeenCalledWith( origin, current.relay.snapshot().session, - expect.any(AbortSignal), ); const reopened = screen.getByRole("dialog", { name: "Settings" }); expect( diff --git a/src/app/ProfileSettings.avatar.test.tsx b/src/app/ProfileSettings.avatar.test.tsx index af152d0bff..f9867920a5 100644 --- a/src/app/ProfileSettings.avatar.test.tsx +++ b/src/app/ProfileSettings.avatar.test.tsx @@ -222,11 +222,7 @@ it("a dispatched save remains bound to A and its late completion cannot clear B' late.resolve(); await late.promise; }); - expect(api.inspectProfile).toHaveBeenLastCalledWith( - a, - undefined, - expect.any(AbortSignal), - ); + expect(api.inspectProfile).toHaveBeenLastCalledWith(a); expect(fixture.saveProfile).toHaveBeenCalledWith({ name: "Changed", picture: "", @@ -417,11 +413,7 @@ it("confirms a dispatched save through its captured session after leaving Settin late.resolve(); await late.promise; }); - expect(api.inspectProfile).toHaveBeenLastCalledWith( - a, - owner.session, - expect.any(AbortSignal), - ); + expect(api.inspectProfile).toHaveBeenLastCalledWith(a, owner.session); expect(fixture.saveProfile).toHaveBeenCalledExactlyOnceWith({ name: "Changed", picture: "", diff --git a/src/app/ProfileSettings.test.tsx b/src/app/ProfileSettings.test.tsx index a65ded5fa4..ea1b089eda 100644 --- a/src/app/ProfileSettings.test.tsx +++ b/src/app/ProfileSettings.test.tsx @@ -123,11 +123,7 @@ it("loads and publishes the selected community profile before updating the local about: "Community bio", }); expect(await screen.findByText("Profile updated")).toBeVisible(); - expect(inspect).toHaveBeenCalledWith( - "community-id", - undefined, - expect.any(AbortSignal), - ); + expect(inspect).toHaveBeenCalledWith("community-id"); }); it("keeps a newer profile seed when an older community publication finishes last", async () => { @@ -661,11 +657,7 @@ it("reuses the verified same-community profile while refreshing and preserves ed "Fresh about", ); expect(screen.getByRole("button", { name: "Save" })).toBeEnabled(); - expect(inspect).toHaveBeenCalledWith( - "primary", - owner.session, - expect.any(AbortSignal), - ); + expect(inspect).toHaveBeenCalledWith("primary", owner.session); view.unmount(); } finally { refresh.resolve({ diff --git a/src/app/ProfileSettings.tsx b/src/app/ProfileSettings.tsx index 5b4f1976bf..fe308ad60a 100644 --- a/src/app/ProfileSettings.tsx +++ b/src/app/ProfileSettings.tsx @@ -130,12 +130,10 @@ export function ProfileSettings({ const [dispatched, setDispatched] = useState(false); const [uploading, setUploading] = useState(false); const mounted = useRef(false); - const saveRead = useRef(undefined); useEffect(() => { mounted.current = true; return () => { mounted.current = false; - saveRead.current?.abort(); }; }, []); const [error, setError] = useState(""); @@ -145,7 +143,6 @@ export function ProfileSettings({ useLayoutEffect(() => { if (previousScope.current === profileScope) return; previousScope.current = profileScope; - saveRead.current?.abort(); setDraft(null); setLoaded(cachedProfile(selectedProfileSession(communities, communityId))); setLoadStatus(communityId ? "idle" : "ready"); @@ -161,17 +158,14 @@ export function ProfileSettings({ } if (!client.relayAvailable || !client.viewer) return; let current = true; - const controller = new AbortController(); const session = selectedProfileSession(communities, communityId); const seed = cachedProfile(session); if (seed) setLoaded(seed); setLoadStatus("loading"); setLoadError(""); - const read = communityApi.inspectProfile( - communityId, - session, - controller.signal, - ); + const read = session + ? communityApi.inspectProfile(communityId, session) + : communityApi.inspectProfile(communityId); void read.then( (result) => { if (!current) return; @@ -195,7 +189,6 @@ export function ProfileSettings({ ); return () => { current = false; - controller.abort(); }; }, [ communities, @@ -341,10 +334,10 @@ export function ProfileSettings({ communities, community?.id, ); - const preflight = new AbortController(); - saveRead.current = preflight; - const inspect = (id: string, signal: AbortSignal) => - communityApi.inspectProfile(id, session, signal); + const inspect = (id: string) => + session + ? communityApi.inspectProfile(id, session) + : communityApi.inspectProfile(id); const saveGeneration = beginProfileSave(communities); setSaving(true); setSaved(false); @@ -353,23 +346,16 @@ export function ProfileSettings({ try { if (community && loaded) { // Preserve fields outside this editor even if another client edited them. - const latest = await inspect( - community.id, - preflight.signal, - ); + const latest = await inspect(community.id); if (!mounted.current) return; setDispatched(true); - saveRead.current = undefined; await communityApi.publishProfile( community.id, next, latest.existing, ); // Once dispatched, finish on this session even if Settings closes. - const confirmed = await inspect( - community.id, - AbortSignal.timeout(12000), - ); + const confirmed = await inspect(community.id); if ( !confirmed.exists || !profilesEqual(confirmed.profile, next) @@ -390,15 +376,13 @@ export function ProfileSettings({ setDraft(null); setSaved(true); } catch (reason) { - if (mounted.current && !preflight.signal.aborted) + if (mounted.current) setError( reason instanceof Error ? reason.message : String(reason), ); } finally { - if (saveRead.current === preflight) - saveRead.current = undefined; if (mounted.current) { setSaving(false); setDispatched(false); diff --git a/src/bundled/builderlab/agents/client.test.ts b/src/bundled/builderlab/agents/client.test.ts index f7ed77ca2f..05e00496d8 100644 --- a/src/bundled/builderlab/agents/client.test.ts +++ b/src/bundled/builderlab/agents/client.test.ts @@ -781,7 +781,7 @@ it("shares one binding attempt between overlapping links", async () => { ).toEqual(["current", "attest-agent"]); }); -it.each(["network", "503", "429"])( +it.each(["network", "429"])( "quietly recovers read-only Builderlab discovery after %s", async (failure) => { const h = await fixture(); diff --git a/src/bundled/builderlab/agents/client.ts b/src/bundled/builderlab/agents/client.ts index 526d9e2ed6..81ece4633c 100644 --- a/src/bundled/builderlab/agents/client.ts +++ b/src/bundled/builderlab/agents/client.ts @@ -1,3 +1,4 @@ +import { transientStatus } from "../../../features/relay/errors"; import { retryRead } from "../../../features/relay/read-retry"; import { invoke } from "@tauri-apps/api/core"; import { nativeIdentityEnabled } from "../../../features/identity/service"; @@ -200,9 +201,7 @@ export function createAgentClient( (error instanceof Error && "status" in error && typeof error.status === "number" && - (error.status === 408 || - error.status === 429 || - error.status >= 500)), + transientStatus(error.status)), ); } /** Builderlab's identity routes answer failures with `{ error: { code } }`. */ diff --git a/src/bundled/channels/ChannelMembersDialog.test.tsx b/src/bundled/channels/ChannelMembersDialog.test.tsx index 741c4c2613..12e62cc372 100644 --- a/src/bundled/channels/ChannelMembersDialog.test.tsx +++ b/src/bundled/channels/ChannelMembersDialog.test.tsx @@ -355,14 +355,12 @@ it("DMs are view-only and never search outside their members", async () => { }); it("Escape closes the dialog and restores focus to its header button", async () => { const t = await setup(); - await vi.waitFor(() => expect(screen.getByRole("searchbox")).toHaveFocus()); + expect(screen.getByRole("searchbox")).toHaveFocus(); await t.user.keyboard("{Escape}"); await vi.waitFor(() => expect(screen.queryByRole("dialog")).not.toBeInTheDocument(), ); - await vi.waitFor(() => - expect(screen.getByRole("button", { name: "Access" })).toHaveFocus(), - ); + expect(screen.getByRole("button", { name: "Access" })).toHaveFocus(); }); it("failed directory searches recover through the shared refresh, not a false empty result", async () => { const t = await setup(); diff --git a/src/bundled/claude-code/setup.ts b/src/bundled/claude-code/setup.ts index 03fad60226..1370c92857 100644 --- a/src/bundled/claude-code/setup.ts +++ b/src/bundled/claude-code/setup.ts @@ -3,7 +3,6 @@ // process; the copy lives apart from any `claude` the user installed, and // shares their `~/.claude` sign-in and settings. import type { AgentProviderSnapshot } from "../../features/agents/service"; -import { isRetryableRead, retryRead } from "../../features/relay/read-retry"; import type { HostProcess, HostRequest, @@ -77,11 +76,7 @@ async function queryModels(spawn: Spawn, signal: AbortSignal) { signal.addEventListener("abort", abort, { once: true }); if (signal.aborted) abort(); const timer = setTimeout(() => { - request.abort( - Object.assign(new Error("Claude Code model discovery timed out"), { - name: "TimeoutError", - }), - ); + request.abort(new Error("Claude Code model discovery timed out")); void child?.kill(); }, CHECK_TIMEOUT_MS); try { @@ -318,9 +313,8 @@ export class ClaudeSetup { /** Loads optional model copy only for surfaces that display the catalog. */ async loadModels(retry = false) { - const spawn = this.spawn; if ( - !spawn || + !this.spawn || this.state.status?.state !== "ready" || this.state.checking || this.modelRequest || @@ -331,14 +325,7 @@ export class ClaudeSetup { this.modelRequest = request; this.set({ modelsChecking: true, modelsError: "" }); try { - const began = performance.now(); - const models = await retryRead( - (signal) => queryModels(spawn, signal), - request.signal, - (error) => - performance.now() - began < CHECK_TIMEOUT_MS && - isRetryableRead(error), - ); + const models = await queryModels(this.spawn, request.signal); if (!models.length) throw new Error("Claude did not list its models"); if (this.modelRequest === request) this.set({ models }); } catch (error) { diff --git a/src/bundled/codex/index.test.ts b/src/bundled/codex/index.test.ts index c6ec8f33b6..35f4c7366b 100644 --- a/src/bundled/codex/index.test.ts +++ b/src/bundled/codex/index.test.ts @@ -5,7 +5,6 @@ import { CodexLogoIcon } from "../../shared/design-system/icons"; import { CodexRuntime } from "./runtime"; import { CodexSetup } from "./setup"; import { CodexProvider } from "./tabs"; -import type { Spawn, Wire } from "./rpc"; import { apply } from "./index"; const desktop = vi.hoisted(() => ({ tauri: true })); @@ -286,107 +285,3 @@ it("preserves a compatible shared Codex model", () => { model: "gpt-shared", }); }); - -it.each(["run", "runOnce"] as const)( - "cancels %s during catalog checking and bounds a hung model list", - async (method) => { - vi.useFakeTimers(); - vi.spyOn(CodexSetup.prototype, "update").mockResolvedValue(); - vi.spyOn(CodexSetup.prototype, "installed").mockResolvedValue(); - vi.spyOn(CodexSetup.prototype, "check").mockResolvedValue({ - state: "ready", - version: "0.162.1", - }); - const run = vi - .spyOn(CodexRuntime.prototype, method) - .mockResolvedValue({ ok: true }); - const requested: string[] = []; - const spawn: Spawn = vi.fn(async (_id, options) => { - let exit!: (code: number | null) => void; - const exited = new Promise((resolve) => { - exit = resolve; - }); - return { - write: async (text: string) => { - const wire: Wire = JSON.parse(text); - if (wire.id == null) return; - requested.push(wire.method ?? ""); - if (wire.method === "model/list") return; - options?.onStdout?.( - `${JSON.stringify({ - id: wire.id, - result: - wire.method === "account/read" - ? { account: {}, requiresOpenaiAuth: false } - : {}, - })}\n`, - ); - }, - end: async () => { - exit(0); - }, - kill: async () => { - exit(null); - }, - exited, - }; - }); - let type!: AgentType; - let dispose = () => {}; - apply({ - host: { spawn, request: vi.fn() }, - agents: { - register: (registered: AgentType) => { - type = registered; - }, - snapshot: () => ({ status: "ready", agents: [], running: [] }), - relay: () => undefined, - subscribe: () => () => {}, - }, - effect: (setup: () => () => void) => { - dispose = setup(); - }, - } as never); - try { - const provider = type.provider; - const start = type[method]; - if (!provider || !start) throw new Error("Missing Codex provider or run"); - const check = provider.check(); - await vi.advanceTimersByTimeAsync(0); - expect(requested).toContain("model/list"); - const stopped = new AbortController(); - let outcome: unknown; - const first = Promise.resolve( - start({ signal: stopped.signal } as never), - ).then( - () => { - outcome = "started"; - }, - (error) => { - outcome = error; - }, - ); - await vi.advanceTimersByTimeAsync(1000); - stopped.abort(new Error("Turn stopped")); - await vi.advanceTimersByTimeAsync(0); - expect(outcome).toBe(stopped.signal.reason); - expect(run).not.toHaveBeenCalled(); - // Cancelling one waiter does not cancel the device-wide catalog for others. - const second = start({ - signal: new AbortController().signal, - } as never); - await vi.advanceTimersByTimeAsync(28_999); - expect(provider.snapshot().readiness).toBe("checking"); - expect(run).not.toHaveBeenCalled(); - await vi.advanceTimersByTimeAsync(1); - await Promise.all([check, first, second]); - expect(provider.snapshot().readiness).toBe("attention"); - expect(run).toHaveBeenCalledOnce(); - await vi.advanceTimersByTimeAsync(120_000); - expect(spawn).toHaveBeenCalledOnce(); - } finally { - dispose(); - vi.useRealTimers(); - } - }, -); diff --git a/src/bundled/codex/index.ts b/src/bundled/codex/index.ts index 97d8237512..c2d2b47448 100644 --- a/src/bundled/codex/index.ts +++ b/src/bundled/codex/index.ts @@ -197,7 +197,7 @@ export const apply: PluginModule["apply"] = (ctx) => { ...codexRun(runtime, async (signal) => { await setup.installed(signal); if (providerController.snapshot().readiness === "checking") - await providerController.check(signal); + await providerController.check(); }), }); }; diff --git a/src/bundled/codex/rpc.test.ts b/src/bundled/codex/rpc.test.ts index 510326ab54..370d66ead6 100644 --- a/src/bundled/codex/rpc.test.ts +++ b/src/bundled/codex/rpc.test.ts @@ -1,6 +1,6 @@ import { expect, it, vi } from "vitest"; import type { HostProcessOptions } from "../../features/host/service"; -import { AppServer, listModels, RequestTimeout, type Wire } from "./rpc"; +import { AppServer, type Wire } from "./rpc"; function fixture() { let options: HostProcessOptions | undefined; let exit!: (code: number) => void; @@ -85,23 +85,3 @@ it("contains consumer exceptions and closes malformed output while rejecting pen consoleError.mockRestore(); } }); - -it("keeps turn-shared model listing single-attempt after its timeout", async () => { - vi.useFakeTimers(); - const f = fixture(); - try { - await f.open(); - const rejected = expect(listModels(f.rpc)).rejects.toBeInstanceOf( - RequestTimeout, - ); - await vi.advanceTimersByTimeAsync(30000); - await rejected; - await vi.advanceTimersByTimeAsync(7000); - expect(f.sent.filter((wire) => wire.method === "model/list")).toHaveLength( - 1, - ); - } finally { - await f.rpc.close(); - vi.useRealTimers(); - } -}); diff --git a/src/bundled/codex/rpc.ts b/src/bundled/codex/rpc.ts index e9fd06da06..efad5a4508 100644 --- a/src/bundled/codex/rpc.ts +++ b/src/bundled/codex/rpc.ts @@ -33,8 +33,6 @@ export type ToolReply = { /** Codex's answer refusing a request, which it therefore did not carry out. */ export class Refused extends Error {} -/** Typed transport timeout; only read-only catalog owners opt into retrying it. */ -export class RequestTimeout extends Error {} /** One request owner for the JSONL app-server connection. No Node sidecar. */ export class AppServer { @@ -204,7 +202,7 @@ export class AppServer { return new Promise((resolve, reject) => { const timer = setTimeout(() => { this.pending.delete(id); - reject(new RequestTimeout(`Codex ${method} timed out`)); + reject(new Error(`Codex ${method} timed out`)); }, 30_000); this.pending.set(id, { resolve: (value) => resolve(value as T), diff --git a/src/bundled/codex/tabs.test.tsx b/src/bundled/codex/tabs.test.tsx index 20434371ee..a015050067 100644 --- a/src/bundled/codex/tabs.test.tsx +++ b/src/bundled/codex/tabs.test.tsx @@ -32,7 +32,7 @@ import { defaults, type Config } from "./config"; import { sessionTag } from "./prompt"; import { CodexProvider, createTabs } from "./tabs"; import { CodexRuntime } from "./runtime"; -import { RequestTimeout, type Wire } from "./rpc"; +import type { Wire } from "./rpc"; import { CodexSetup } from "./setup"; function deferred() { let resolve!: (value: T) => void; @@ -1550,96 +1550,3 @@ it("shows the recent empty state when only the current runtime session exists", ).toBeVisible(); expect(screen.queryByRole("list")).not.toBeInTheDocument(); }); - -it("retries a typed transport timeout only within the catalog owner", async () => { - const f = fixture(); - const spawn = vi - .fn(f.spawn) - .mockRejectedValueOnce(new RequestTimeout("Catalog timed out")); - const provider = new CodexProvider(spawn, localStore()); - vi.useFakeTimers(); - try { - const pending = provider.check(); - await vi.advanceTimersByTimeAsync(499); - expect(spawn).toHaveBeenCalledOnce(); - await vi.advanceTimersByTimeAsync(501); - await pending; - expect(provider.snapshot().readiness).toBe("ready"); - expect(spawn).toHaveBeenCalledTimes(2); - } finally { - provider.dispose(); - vi.useRealTimers(); - } -}); - -it.each([ - ["signed-out", "Log in to Codex", "Codex needs you to sign in."], - ["missing", "Install Codex", "Codex is not installed."], -] as const)( - "preserves a slow %s setup outcome", - async (state, label, error) => { - const f = fixture(); - vi.spyOn(f.setup, "check").mockImplementation( - () => - new Promise((resolve) => { - setTimeout(() => resolve({ state, version: "0.162.1" }), 31_000); - }), - ); - vi.useFakeTimers(); - try { - const pending = f.provider.check(); - await vi.advanceTimersByTimeAsync(31_000); - await pending; - expect(f.provider.snapshot()).toMatchObject({ - readiness: "attention", - attentionLabel: label, - error, - }); - expect(f.spawn).not.toHaveBeenCalled(); - } finally { - f.provider.dispose(); - vi.useRealTimers(); - } - }, -); - -it("gives the catalog its full lifetime after slow successful setup", async () => { - const f = fixture(); - vi.spyOn(f.setup, "check").mockImplementation( - () => - new Promise((resolve) => { - setTimeout( - () => resolve({ state: "ready", version: "0.162.1" }), - 31_000, - ); - }), - ); - const spawn = vi.fn(async (...args: Parameters) => { - const process = await f.spawn(...args); - return { - ...process, - write: async (text: string) => { - const wire: Wire = JSON.parse(text); - if (wire.method === "model/list") - await new Promise((resolve) => setTimeout(resolve, 15_000)); - await process.write(text); - }, - }; - }); - const provider = new CodexProvider(spawn, localStore(), f.setup); - vi.useFakeTimers(); - try { - const pending = provider.check(); - await vi.advanceTimersByTimeAsync(31_000); - expect(provider.snapshot().checking).toBe(true); - await vi.advanceTimersByTimeAsync(14_999); - expect(provider.snapshot().readiness).toBe("checking"); - await vi.advanceTimersByTimeAsync(1); - await pending; - expect(provider.snapshot().readiness).toBe("ready"); - expect(spawn).toHaveBeenCalledOnce(); - } finally { - provider.dispose(); - vi.useRealTimers(); - } -}); diff --git a/src/bundled/codex/tabs.tsx b/src/bundled/codex/tabs.tsx index f7882d6299..685b7532b2 100644 --- a/src/bundled/codex/tabs.tsx +++ b/src/bundled/codex/tabs.tsx @@ -1,4 +1,3 @@ -import { isRetryableRead, retryRead } from "../../features/relay/read-retry"; import activityStyles from "../../features/agents/ActivityLayout.module.css"; import { formatItemTimestamp } from "../../shared/datetime"; import { InlineHeader } from "../../shared/design-system/ui/Header"; @@ -50,13 +49,7 @@ import { type Config, type Model, } from "./config"; -import { - AppServer, - listModels, - needsSignIn, - RequestTimeout, - type Spawn, -} from "./rpc"; +import { AppServer, listModels, needsSignIn, type Spawn } from "./rpc"; import type { CodexRuntime, SavedConversation } from "./runtime"; import type { CodexSetup } from "./setup"; import { readTranscript, TURN_LIMIT, type CodexTranscript } from "./transcript"; @@ -83,8 +76,6 @@ const STORAGE_KEY = "buzz.codex.provider-defaults.v1"; const SIGN_IN_ERROR = "Codex needs you to sign in."; const NOT_INSTALLED = "Codex is not installed."; const CONVERSATION_LIMIT = 30; -// Initial mentions await this catalog too; do not multiply RPC timeouts. -const CATALOG_TIMEOUT_MS = 30_000; /** Read-only setup/catalog discovery uses the same declared native transport. */ async function catalog(spawn: Spawn, signal: AbortSignal) { const rpc = new AppServer(); @@ -144,39 +135,17 @@ export class CodexProvider { this.state = Object.freeze({ ...this.state, ...change }); for (const listener of this.listeners) listener(); } - check(signal?: AbortSignal) { - signal?.throwIfAborted(); - if (!this.pending) { - const pending = this.runCheck(); - this.pending = pending; - void pending.finally(() => { - if (this.pending === pending) this.pending = undefined; - }); - } - const pending = this.pending; - if (!signal) return pending; - // A turn stops waiting without retiring discovery shared by other callers. - return new Promise((resolve, reject) => { - const abort = () => reject(signal.reason); - signal.addEventListener("abort", abort, { once: true }); - void pending.then( - () => { - signal.removeEventListener("abort", abort); - resolve(); - }, - (error) => { - signal.removeEventListener("abort", abort); - reject(error); - }, - ); - if (signal.aborted) abort(); + check() { + if (this.pending) return this.pending; + const pending = this.runCheck(); + this.pending = pending; + void pending.finally(() => { + if (this.pending === pending) this.pending = undefined; }); + return pending; } private async runCheck() { const abort = new AbortController(); - const deadline = new AbortController(); - const signal = AbortSignal.any([abort.signal, deadline.signal]); - let timer: ReturnType | undefined; this.abort = abort; this.set({ ...(this.state.data ? {} : { readiness: "checking" as const }), @@ -193,15 +162,7 @@ export class CodexProvider { ? SIGN_IN_ERROR : status.message, ); - timer = setTimeout( - () => deadline.abort(new RequestTimeout("Codex catalog timed out")), - CATALOG_TIMEOUT_MS, - ); - const data = await retryRead( - (signal) => catalog(this.spawn, signal), - signal, - (error) => error instanceof RequestTimeout || isRetryableRead(error), - ); + const data = await catalog(this.spawn, abort.signal); if (this.abort === abort) { this.set({ readiness: "ready", @@ -226,7 +187,6 @@ export class CodexProvider { error: reason, }); } finally { - clearTimeout(timer); if (this.abort === abort) this.abort = undefined; } } diff --git a/src/bundled/github/data.test.tsx b/src/bundled/github/data.test.tsx index ae0c6c75be..e21f044ffe 100644 --- a/src/bundled/github/data.test.tsx +++ b/src/bundled/github/data.test.tsx @@ -1,5 +1,7 @@ import { assert, afterEach, expect, it, vi } from "vitest"; import { loadGitHubDetails } from "./data"; +import { GitHubError } from "./gh"; +import { loadGitHubObject } from "./object"; import { parseGitHubReference } from "./references"; afterEach(() => vi.unstubAllGlobals()); @@ -64,16 +66,9 @@ it.each([ "fetch", vi.fn().mockResolvedValue(new Response(null, { status: Number(status) })), ); - vi.useFakeTimers(); - try { - const rejected = expect( - loadGitHubDetails(reference, new AbortController().signal), - ).rejects.toThrow(String(message)); - await vi.advanceTimersByTimeAsync(7000); - await rejected; - } finally { - vi.useRealTimers(); - } + await expect( + loadGitHubDetails(reference, new AbortController().signal), + ).rejects.toThrow(String(message)); }); it("does not expose check head plumbing on non-PR objects", async () => { @@ -223,23 +218,33 @@ it.each([null, "bad", undefined])( }, ); -it("recovers a transient GitHub API page quietly within one budget", async () => { - vi.useFakeTimers(); - try { - const request = vi - .fn() - .mockResolvedValueOnce(new Response(null, { status: 502 })) - .mockImplementation(async () => - Response.json({ title: "Recovered", body: "Kept content" }), +it.each(["public", "gh"])( + "recovers a transient %s GitHub read quietly", + async (source) => { + vi.useFakeTimers(); + try { + const request = vi + .fn() + .mockResolvedValueOnce(new Response(null, { status: 502 })) + .mockImplementation(async () => Response.json({ title: "Recovered" })); + vi.stubGlobal("fetch", request); + const gh = vi + .fn() + .mockRejectedValueOnce(new GitHubError("down", "SERVICE_UNAVAILABLE")) + .mockRejectedValue(new Error("stop")); + const pending = loadGitHubObject( + reference, + new AbortController().signal, + source === "gh" ? gh : undefined, ); - vi.stubGlobal("fetch", request); - const pending = loadGitHubDetails(reference, new AbortController().signal); - await vi.advanceTimersByTimeAsync(499); - expect(request).toHaveBeenCalledOnce(); - await vi.advanceTimersByTimeAsync(501); - expect((await pending).title).toBe("Recovered"); - expect(request).toHaveBeenCalledTimes(2); - } finally { - vi.useRealTimers(); - } -}); + const settled = pending.catch((error: unknown) => error); + await vi.advanceTimersByTimeAsync(1000); + if (source === "public") + expect((await pending).details.title).toBe("Recovered"); + else expect(await settled).toMatchObject({ message: "stop" }); + expect(source === "gh" ? gh : request).toHaveBeenCalledTimes(2); + } finally { + vi.useRealTimers(); + } + }, +); diff --git a/src/bundled/github/data.ts b/src/bundled/github/data.ts index b76528d23d..3392cd2dbe 100644 --- a/src/bundled/github/data.ts +++ b/src/bundled/github/data.ts @@ -1,5 +1,4 @@ -import { ReadError } from "../../features/relay/errors"; -import { retryRead } from "../../features/relay/read-retry"; +import { ReadError, transientStatus } from "../../features/relay/errors"; import type { GitHubReference } from "./references"; export type Branch = { label: string; url?: string | undefined }; @@ -89,23 +88,14 @@ export async function loadGitHubDetails( issue: `/issues/${id}`, commit: `/commits/${id}`, }[reference.kind]; - const response = await retryRead(async (signal) => { - const response = await fetch( - `https://api.github.com/repos/${reference.repository}${surface}`, - { - signal, - headers: { Accept: "application/vnd.github.full+json" }, - credentials: "omit", - }, - ); - if (response.status === 408 || response.status >= 500) - throw new ReadError( - "unavailable", - `GitHub couldn’t load this object (${response.status}).`, - response.status, - ); - return response; - }, signal); + const response = await fetch( + `https://api.github.com/repos/${reference.repository}${surface}`, + { + signal, + headers: { Accept: "application/vnd.github.full+json" }, + credentials: "omit", + }, + ); if (!response.ok) { if (response.status === 404) throw new Error( @@ -115,7 +105,11 @@ export async function loadGitHubDetails( throw new Error( "GitHub’s public API limit was reached. Try again later or open it on GitHub.", ); - throw new Error(`GitHub couldn’t load this object (${response.status}).`); + const message = `GitHub couldn’t load this object (${response.status}).`; + // `loadGitHubObject` retries server failures quietly. + throw transientStatus(response.status) + ? new ReadError("unavailable", message, response.status) + : new Error(message); } const data: ResponseData = await response.json(); const facts: GitHubDetails["facts"] = []; diff --git a/src/bundled/github/object.ts b/src/bundled/github/object.ts index e32e046503..d8093b01f2 100644 --- a/src/bundled/github/object.ts +++ b/src/bundled/github/object.ts @@ -1,6 +1,6 @@ -import { isRetryableRead, retryRead } from "../../features/relay/read-retry"; // What the panel shows for a GitHub link: read with the person's `gh` sign-in // when there is one, so private repositories work, else from the public API. +import { isRetryableRead, retryRead } from "../../features/relay/read-retry"; import { loadGitHubDetails, type GitHubDetails } from "./data"; import { GhUnavailable, GitHubError, type GitHubCli } from "./gh"; import { loadPull, type PullChecklist } from "./pull"; diff --git a/src/bundled/goose/tabs.tsx b/src/bundled/goose/tabs.tsx index 1bc29eec6d..3655802fab 100644 --- a/src/bundled/goose/tabs.tsx +++ b/src/bundled/goose/tabs.tsx @@ -1,5 +1,4 @@ import { ModelDiscoveryRecovery } from "../../features/agents/ModelDiscoveryRecovery"; -import { retryRead } from "../../features/relay/read-retry"; // goose's New agent choices and agent settings. Providers, their keys and // their models are goose's own: listed from it, and saved into its config. import { @@ -91,13 +90,12 @@ function useRead(read: () => Promise, key: string, scope = key) { // biome-ignore lint/correctness/useExhaustiveDependencies: `read` follows `key`. useEffect(() => { let current = true; - const controller = new AbortController(); const kept = (prior: Read) => prior.value === undefined || prior.scope !== scope ? {} : { value: prior.value, scope }; setState((prior) => ({ status: "loading", ...kept(prior) })); - retryRead(() => read(), controller.signal).then( + read().then( (value) => current && setState({ status: "ready", value, scope }), (error: unknown) => current && @@ -109,7 +107,6 @@ function useRead(read: () => Promise, key: string, scope = key) { ); return () => { current = false; - controller.abort(); }; }, [key, round]); return [state, () => setRound((value) => value + 1)] as const; diff --git a/src/bundled/pi/tabs.tsx b/src/bundled/pi/tabs.tsx index b8c6f8a606..65eb49dc81 100644 --- a/src/bundled/pi/tabs.tsx +++ b/src/bundled/pi/tabs.tsx @@ -8,8 +8,7 @@ import { useSyncExternalStore, } from "react"; import { errorText } from "../../buzz-mcp/rpc"; -import { retryRead } from "../../features/relay/read-retry"; -import { isTransientFailure } from "../../features/relay/errors"; +import { isRetryableRead, retryRead } from "../../features/relay/read-retry"; import { PiStopped } from "./pi"; import activityStyles from "../../features/agents/ActivityLayout.module.css"; import { ActivityTranscript } from "../../features/agents/ActivityTranscript"; @@ -119,7 +118,8 @@ function useRead( retryRead( read, abort.signal, - (error) => error instanceof PiStopped || isTransientFailure(error), + // The next request starts another host. + (error) => error instanceof PiStopped || isRetryableRead(error), ).then( (value) => { if (!abort.signal.aborted) setState({ status: "ready", value, scope }); diff --git a/src/bundled/relay-staff/session.tsx b/src/bundled/relay-staff/session.tsx index 0dc37c2705..c69f43d244 100644 --- a/src/bundled/relay-staff/session.tsx +++ b/src/bundled/relay-staff/session.tsx @@ -21,7 +21,7 @@ import type { import type { Staff } from "./staff"; import type { Held, Writes } from "./writes"; import { retryRead } from "../../features/relay/read-retry"; -import { ReadRetriesExhausted } from "../../features/relay/errors"; +import { transientStatus } from "../../features/relay/errors"; const READ_ROUTES = new Set([ "probe", @@ -45,10 +45,7 @@ function transientRead(failure: StaffFailure) { return ( failure.category === "intercepted" || (failure.category === "ambiguous" && - (failure.status === null || - failure.status === 408 || - failure.status === 429 || - failure.status >= 500)) + (failure.status === null || transientStatus(failure.status))) ); } /** Only read owners call this; frozen write intents keep their explicit retry. */ @@ -69,10 +66,8 @@ export async function retryStaffRead( ); } catch (error) { if (signal.aborted) return; - const original = - error instanceof ReadRetriesExhausted ? error.cause : error; - if (original instanceof StaffReadFailure) - return { ok: false, failure: original.failure }; + if (error instanceof StaffReadFailure) + return { ok: false, failure: error.failure }; throw error; } } diff --git a/src/features/agent-harness/acp/process.ts b/src/features/agent-harness/acp/process.ts index 6fd0d25bca..67132cdb73 100644 --- a/src/features/agent-harness/acp/process.ts +++ b/src/features/agent-harness/acp/process.ts @@ -364,11 +364,8 @@ export class AcpProcess { const timer = setTimeout(() => { this.calls.delete(id); reject( - Object.assign( - new Error( - `${label} did not answer ${method} in ${Math.round(timeout / 1000)}s`, - ), - { name: "TimeoutError" }, + new Error( + `${label} did not answer ${method} in ${Math.round(timeout / 1000)}s`, ), ); void this.kill(); diff --git a/src/features/agents/memory.ts b/src/features/agents/memory.ts index a9663a3ca5..2daa876341 100644 --- a/src/features/agents/memory.ts +++ b/src/features/agents/memory.ts @@ -1,5 +1,5 @@ -import { retryRead } from "../relay/read-retry.ts"; /** Owner-view NIP-AE snapshot from a purpose-bound key-owning host. */ +import { retryRead } from "../relay/read-retry.ts"; /** Budgets of the development broker's single memory query. */ export const MEMORY_EVENT_LIMIT = 256; export const MEMORY_WIRE_BYTES = 2 * 1024 * 1024; diff --git a/src/features/communities/CommunityDialog.tsx b/src/features/communities/CommunityDialog.tsx index 9b78778fc5..d1080b2886 100644 --- a/src/features/communities/CommunityDialog.tsx +++ b/src/features/communities/CommunityDialog.tsx @@ -93,34 +93,22 @@ export function CommunityDialog({ const [busy, setBusy] = useState(false); const [error, setError] = useState(recovery.error); const mounted = useRef(true); - const pendingRead = useRef(undefined); - const readScope = JSON.stringify([client.viewer, mode, connectionFrom]); useEffect(() => { - void readScope; // A different identity/destination retires pending reads. mounted.current = true; - setBusy(false); return () => { mounted.current = false; - pendingRead.current?.abort(); - pendingRead.current = undefined; }; - }, [readScope]); - async function work(action: (signal: AbortSignal) => Promise) { - pendingRead.current?.abort(); - const controller = new AbortController(); - pendingRead.current = controller; + }, []); + async function work(action: () => Promise) { setBusy(true); setError(""); try { - await action(controller.signal); + await action(); } catch (reason) { - if (mounted.current && pendingRead.current === controller) + if (mounted.current) setError(reason instanceof Error ? reason.message : String(reason)); } finally { - if (pendingRead.current === controller) { - pendingRead.current = undefined; - if (mounted.current) setBusy(false); - } + if (mounted.current) setBusy(false); } } const policy = info?.policy; @@ -142,19 +130,17 @@ export function CommunityDialog({ async function submit() { if (uploading || unavailable) return; if (step === "destination") { - await work(async (signal) => { + await work(async () => { const next = communityDestination(relayOrigin(url)); setDestination(next); const value = await communityRequest(next.id, "info"); if (!mounted.current) return; const pending = journal?.get(next.id); // Restoring an existing admitted profile is not a new join or policy acceptance. - const found = await inspectProfile(next.id, undefined, signal).catch( - (reason: unknown) => { - if (pending && readErrorKind(reason) !== "denied") throw reason; - return undefined; - }, - ); + const found = await inspectProfile(next.id).catch((reason: unknown) => { + if (pending && readErrorKind(reason) !== "denied") throw reason; + return undefined; + }); if (current(pending)) { setInfo(value); if (found && (found.exists || pending)) showProfile(found, pending); @@ -163,17 +149,15 @@ export function CommunityDialog({ }); } else if (step === "access") { if (!allowed) return; - await work(async (signal) => { + await work(async () => { const pending = journal?.get(id); if (pending) { // A lost claim response may already have admitted this identity, even // when its invite has since expired. Read before attempting another claim. - const found = await inspectProfile(id, undefined, signal).catch( - (reason: unknown) => { - if (readErrorKind(reason) !== "denied") throw reason; - return undefined; - }, - ); + const found = await inspectProfile(id).catch((reason: unknown) => { + if (readErrorKind(reason) !== "denied") throw reason; + return undefined; + }); if (!current(pending)) return; if (found) { showProfile(found, pending); @@ -212,21 +196,19 @@ export function CommunityDialog({ throw new Error("Membership was not confirmed"); } if (!current(transaction)) return; - const found = await inspectProfile(id, undefined, signal); + const found = await inspectProfile(id); if (!current(transaction)) return; showProfile(found, transaction); }); } else { if (!profile.name.trim()) return; - await work(async (signal) => { + await work(async () => { if (mode === "profile") communities.saveProfile({ ...profile, name: profile.name.trim() }); else { if (!destination) throw new Error("Choose a community first"); const transaction = journal?.begin(id, profile); - const found = journal - ? await inspectProfile(id, undefined, signal) - : original; + const found = journal ? await inspectProfile(id) : original; if (!current(transaction)) return; const next = found?.exists && profilesEqual(profile, found.profile) @@ -240,7 +222,7 @@ export function CommunityDialog({ await publishProfile(id, next, found?.existing ?? {}); if (!current(transaction)) return; // An accepted replaceable event may already be superseded. - const confirmed = await inspectProfile(id, undefined, signal); + const confirmed = await inspectProfile(id); if (!current(transaction)) return; if (!confirmed.exists || !profilesEqual(confirmed.profile, next)) throw new Error( diff --git a/src/features/communities/api.ts b/src/features/communities/api.ts index 527d2bc00d..89584b5194 100644 --- a/src/features/communities/api.ts +++ b/src/features/communities/api.ts @@ -51,21 +51,15 @@ export async function communityRequest( ); return result as T; } -export async function inspectProfile( - id: string, - session: RelaySession | undefined, - caller: AbortSignal, -) { - caller.throwIfAborted(); +export async function inspectProfile(id: string, session?: RelaySession) { // Existing-community editors use the captured session so confirmed reads also // update its shared profile directory. Joining uses the selected host adapter. const transport = session ?? (await retryRead( (signal) => connectCommunityTransport(id, signal), - AbortSignal.any([caller, AbortSignal.timeout(12000)]), + AbortSignal.timeout(12000), )); - const signal = AbortSignal.any([caller, AbortSignal.timeout(12000)]); if (!transport.viewer) throw new Error("Profile identity is unavailable"); const filters = [ { @@ -77,9 +71,14 @@ export async function inspectProfile( ]; const events = "read" in transport - ? await transport.read(filters, { signal, fresh: true }) - : await retryRead((signal) => transport.query(filters, signal), signal); - signal.throwIfAborted(); + ? await transport.read(filters, { + signal: AbortSignal.timeout(12000), + fresh: true, + }) + : await retryRead( + (signal) => transport.query(filters, signal), + AbortSignal.timeout(12000), + ); const event = events .filter((e) => e.kind === 0 && e.pubkey === transport.viewer) .sort((a, b) => b.created_at - a.created_at || a.id.localeCompare(b.id))[0]; diff --git a/src/features/communities/native-api.test.ts b/src/features/communities/native-api.test.ts index 134c5253b5..b28533437f 100644 --- a/src/features/communities/native-api.test.ts +++ b/src/features/communities/native-api.test.ts @@ -249,11 +249,7 @@ it("restores a signed community profile and preserves extra fields when publishi path === "/query" ? { body: [event] } : { body: { accepted: true, event_id: (body as { id: string }).id } }; - const found = await inspectProfile( - community, - undefined, - new AbortController().signal, - ); + const found = await inspectProfile(community); expect(found.profile).toEqual({ name: "Existing", picture: "", diff --git a/src/features/communities/profile-read-retry.test.ts b/src/features/communities/profile-read-retry.test.ts deleted file mode 100644 index 374bea6873..0000000000 --- a/src/features/communities/profile-read-retry.test.ts +++ /dev/null @@ -1,77 +0,0 @@ -import { afterEach, beforeEach, expect, it, vi } from "vitest"; -import { inspectProfile } from "./api"; -import { createRelaySession } from "../relay/session"; -import { ReadError } from "../relay/errors"; -import { keypair } from "../relay/testing"; -import type { RelayEvent } from "../relay/events"; - -const owners: ReturnType[] = []; -beforeEach(() => { - vi.useFakeTimers(); - vi.spyOn(Math, "random").mockReturnValue(0.5); - vi.spyOn(AbortSignal, "timeout").mockImplementation((ms) => { - const controller = new AbortController(); - setTimeout( - () => - controller.abort( - new DOMException("Profile read timed out", "TimeoutError"), - ), - ms, - ); - return controller.signal; - }); -}); -afterEach(() => { - for (const owner of owners.splice(0)) owner.dispose(); - vi.clearAllTimers(); - vi.restoreAllMocks(); - vi.useRealTimers(); -}); -function setup() { - const viewer = keypair(); - const query = vi.fn(async (): Promise => []); - const owner = createRelaySession({ - viewer: viewer.pubkey, - relayAuthor: keypair().pubkey, - query, - media: () => undefined, - }); - owners.push(owner); - return { owner, query }; -} - -it("keeps one session budget and the profile's twelve-second deadline", async () => { - const h = setup(); - h.query.mockImplementation(async () => { - await new Promise((resolve) => setTimeout(resolve, 4000)); - throw new ReadError("unavailable", "offline"); - }); - const pending = inspectProfile( - "community", - h.owner.session, - new AbortController().signal, - ); - const rejected = expect(pending).rejects.toMatchObject({ - name: "TimeoutError", - }); - await vi.advanceTimersByTimeAsync(12_000); - await rejected; - expect(h.query).toHaveBeenCalledTimes(3); - await vi.advanceTimersByTimeAsync(30_000); - expect(h.query).toHaveBeenCalledTimes(3); -}); - -it("cancels profile backoff with the editor's signal", async () => { - const h = setup(); - h.query.mockRejectedValue(new ReadError("unavailable", "offline")); - const caller = new AbortController(); - const pending = inspectProfile("community", h.owner.session, caller.signal); - const rejected = expect(pending).rejects.toMatchObject({ - name: "AbortError", - }); - await vi.advanceTimersByTimeAsync(0); - caller.abort(); - await rejected; - await vi.advanceTimersByTimeAsync(7000); - expect(h.query).toHaveBeenCalledOnce(); -}); diff --git a/src/features/relay/channel-lifecycle.test.ts b/src/features/relay/channel-lifecycle.test.ts index 540afa735e..827d3509d4 100644 --- a/src/features/relay/channel-lifecycle.test.ts +++ b/src/features/relay/channel-lifecycle.test.ts @@ -13,7 +13,6 @@ import { vi, } from "vitest"; import { ReadError } from "./errors"; -import { createRecovery } from "./recovery"; import { finalizeEvent, getPublicKey, type EventTemplate } from "nostr-tools"; import { PublishRejected } from "./outbox"; import { @@ -204,26 +203,9 @@ describe("bounded visibility recovery", () => { h.owner.dispose(); }); - it("exhausts exactly four reads without restarting through session recovery", async () => { + it("exhausts four reads, then surfaces an error a manual retry can clear", async () => { const h = harness(); const lifecycle = h.owner.capability; - const recovery = createRecovery([ - { - subscribe: lifecycle.subscribe, - // Match the session adapter: only errors carrying `transient` opt in. - status: () => { - const state = lifecycle.snapshot(); - if ( - state.status === "error" && - "transient" in state && - state.transient - ) - return "failed"; - return state.status === "loading" ? "busy" : "settled"; - }, - retry: () => void lifecycle.refreshVisibility(), - }, - ]); h.read.mockRejectedValue(new ReadError("unavailable", "offline", 503)); const pending = lifecycle.refreshVisibility(); await vi.advanceTimersByTimeAsync(0); @@ -244,14 +226,11 @@ describe("bounded visibility recovery", () => { h.read.mockResolvedValue([]); await lifecycle.refreshVisibility(); expect(lifecycle.snapshot().status).toBe("ready"); - recovery.dispose(); h.owner.dispose(); }); it.each([ new ReadError("denied", "denied", 403), - new ReadError("invalid-response", "validation", 400), - new ReadError("invalid-response", "bad response"), new Error("Invalid DM visibility snapshot"), ])("surfaces permanent failure immediately: %s", async (error) => { const h = harness(); @@ -266,41 +245,6 @@ describe("bounded visibility recovery", () => { h.owner.dispose(); }); - it.each([400, 401, 403, 404])( - "retries transport-classified edge HTTP %s failures", - async (status) => { - const h = harness(); - h.read.mockRejectedValueOnce( - new ReadError("unavailable", "Edge unavailable", status), - ); - const pending = h.owner.capability.refreshVisibility(); - await vi.advanceTimersByTimeAsync(749); - expect(h.read).toHaveBeenCalledOnce(); - expect(h.owner.capability.snapshot().status).toBe("loading"); - await vi.advanceTimersByTimeAsync(1); - await pending; - expect(h.read).toHaveBeenCalledTimes(2); - expect(h.owner.capability.snapshot().status).toBe("ready"); - h.owner.dispose(); - }, - ); - - it("surfaces a long admission pause without holding the refresh pending", async () => { - const h = harness(); - h.read.mockRejectedValueOnce( - new ReadError("unavailable", "quota", 429, 5000), - ); - const pending = h.owner.capability.refreshVisibility(); - await pending; - expect(h.read).toHaveBeenCalledOnce(); - expect(h.owner.capability.snapshot()).toMatchObject({ - status: "error", - error: "quota", - }); - expect(vi.getTimerCount()).toBe(0); - h.owner.dispose(); - }); - it("coalesces concurrent triggers during reads and backoff", async () => { const h = harness(); const gate = deferred(); @@ -385,19 +329,6 @@ describe("bounded visibility recovery", () => { expect(h.owner.capability.snapshot().status).toBe("ready"); h.owner.dispose(); }); - - it("keeps the retry budget through channel-scoped invalidation during backoff", async () => { - const h = harness(); - h.read.mockRejectedValueOnce(new TypeError("offline")); - const pending = h.owner.capability.refreshVisibility(); - await vi.advanceTimersByTimeAsync(0); - h.owner.cancel([id]); - await vi.advanceTimersByTimeAsync(750); - await pending; - expect(h.read).toHaveBeenCalledTimes(2); - expect(h.owner.capability.snapshot().status).toBe("ready"); - h.owner.dispose(); - }); }); it.each(["archive", "delete", "leave", "hide"] as const)( @@ -854,8 +785,6 @@ it("a clear cannot be repopulated by a late visibility response", async () => { const gate = deferred(); h.read.mockImplementationOnce(() => gate.promise); const pending = h.owner.capability.refreshVisibility(); - await Promise.resolve(); - expect(h.read).toHaveBeenCalledOnce(); h.owner.clear(); gate.resolve([ h.record(30622, [ diff --git a/src/features/relay/channel-lifecycle.ts b/src/features/relay/channel-lifecycle.ts index 6b07de0b2f..c97bbfab35 100644 --- a/src/features/relay/channel-lifecycle.ts +++ b/src/features/relay/channel-lifecycle.ts @@ -194,8 +194,6 @@ export function createChannelLifecycle({ { signal, fresh: true, priority: "foreground" }, ), profileSignal, - (error) => - readErrorKind(error) !== "cancelled" && isTransientFailure(error), ); profileSignal.throwIfAborted(); assertAccess(id); @@ -298,19 +296,17 @@ export function createChannelLifecycle({ ...(caller ? [caller] : []), ]); refreshSignal = signal; - // Four reads total. Reuse session recovery's first three backoff ceilings - // (1/2/4s), with equal jitter. Exhausted errors omit `transient` so the - // session recovery coordinator cannot start another retry sequence. - const operation = Promise.resolve() - .then(() => - retryRead( - async () => { - await owned({ caller: signal }, readVisibility); - }, - signal, - isTransientFailure, - ), - ) + emit({ status: "loading", hidden: snapshot.hidden }); + // Exhausted errors omit `transient`, so session recovery cannot restart + // them. A read retired by another channel's access change (`cancelled`) + // retries within the same budget. + const operation = retryRead( + async () => { + await owned({ caller: signal }, readVisibility); + }, + signal, + isTransientFailure, + ) .catch((error: unknown) => { if (started === epoch && !closed && refresh === operation) emit({ @@ -335,7 +331,6 @@ export function createChannelLifecycle({ } }); refresh = operation; - emit({ status: "loading", hidden: snapshot.hidden }); return operation; }, async load(value: string, caller?: AbortSignal) { diff --git a/src/features/relay/directory-session.test.ts b/src/features/relay/directory-session.test.ts index 586f1ec2a4..c1c9e71436 100644 --- a/src/features/relay/directory-session.test.ts +++ b/src/features/relay/directory-session.test.ts @@ -196,9 +196,8 @@ describe("endpoint profile search under session ownership", () => { signal: new AbortController().signal, }); const rejected = expect(pending).rejects.toMatchObject({ - name: "ReadRetriesExhausted", + kind: "unavailable", message: "Relay read timed out", - cause: { kind: "unavailable", message: "Relay read timed out" }, }); const request = h.wire.next(); await vi.advanceTimersByTimeAsync(10_000); diff --git a/src/features/relay/errors.ts b/src/features/relay/errors.ts index 3f49e99b5d..f843e3dd45 100644 --- a/src/features/relay/errors.ts +++ b/src/features/relay/errors.ts @@ -15,15 +15,9 @@ export class ReadError extends Error { this.name = "ReadError"; } } -/** The read has used its automatic budget; a deliberate new read may retry. */ -export class ReadRetriesExhausted extends Error { - constructor(readonly cause: unknown) { - super(cause instanceof Error ? cause.message : String(cause)); - this.name = "ReadRetriesExhausted"; - } - override toString() { - return String(this.cause); - } +/** HTTP statuses where the same request can succeed later unchanged. */ +export function transientStatus(status: number) { + return status === 408 || status === 429 || status >= 500; } export function httpReadError(status: number) { return new ReadError( diff --git a/src/features/relay/gifs.ts b/src/features/relay/gifs.ts index f87116399c..4f7371a367 100644 --- a/src/features/relay/gifs.ts +++ b/src/features/relay/gifs.ts @@ -1,5 +1,5 @@ import { retryRead } from "./read-retry"; -import { ReadError } from "./errors"; +import { ReadError, transientStatus } from "./errors"; import { nativeIdentityEnabled } from "../identity/service"; import { relayKlipySearchPath, @@ -150,16 +150,12 @@ async function brokerRequest( ...(signal ? { signal } : {}), }); const result = (await response.json()) as T & { error?: string }; - if (!response.ok) - throw new ReadError( - response.status === 408 || - response.status === 429 || - response.status >= 500 - ? "unavailable" - : "invalid-response", - result.error ?? `GIF request failed (${response.status})`, - response.status, - ); + if (!response.ok) { + const message = result.error ?? `GIF request failed (${response.status})`; + throw transientStatus(response.status) + ? new ReadError("unavailable", message, response.status) + : new Error(message); + } return result; } diff --git a/src/features/relay/identity-archives.ts b/src/features/relay/identity-archives.ts index b2b3c8b1fd..93b5df0de6 100644 --- a/src/features/relay/identity-archives.ts +++ b/src/features/relay/identity-archives.ts @@ -1,4 +1,4 @@ -import { isRetryableRead, retryRead } from "./read-retry"; +import { retryRead } from "./read-retry"; import { getEventHash } from "nostr-tools"; import { authTagOwner, withinTimeBounds } from "../agents/owner-attestation"; import { byteSize } from "./budget"; @@ -80,7 +80,6 @@ export function createIdentityArchives( void (async () => { try { if (closed || owned.signal.aborted) return; - const began = performance.now(); const events = await retryRead( (signal) => reader.read([{ kinds: [13535], authors: [authority], limit: 1 }], { @@ -89,8 +88,6 @@ export function createIdentityArchives( fresh: true, }), owned.signal, - (error) => - performance.now() - began < 10_000 && isRetryableRead(error), ); if (closed || owned.signal.aborted) return; // The relay publishes its first snapshot with its first archive, so diff --git a/src/features/relay/profile-directory.ts b/src/features/relay/profile-directory.ts index db50c94967..c06ad26abc 100644 --- a/src/features/relay/profile-directory.ts +++ b/src/features/relay/profile-directory.ts @@ -137,34 +137,26 @@ export function createProfileDirectory( for (let offset = 0; offset < missing.length; offset += 500) { const authors = missing.slice(offset, offset + 500); let answered = false; - const batch: { - authors: string[]; - priority: Priority; - reading: boolean; - work: Promise; - } = { + const batch = { authors, priority, reading: false, work: Promise.resolve(), }; - const request = async (signal: AbortSignal) => { + batch.work = retryRead(async (signal) => { + // A promotion during backoff applies to the next attempt instead. batch.reading = true; try { const result = await reader.read( [{ kinds: [0], authors, limit: 500 }], - { - signal, - priority: batch.priority, - }, + { signal, priority: batch.priority }, ); answered = true; return result; } finally { batch.reading = false; } - }; - batch.work = retryRead(request, signal) + }, signal) .then((result) => { if (!signal.aborted) accept(result.filter((event) => authors.includes(event.pubkey))); diff --git a/src/features/relay/read-retry.test.ts b/src/features/relay/read-retry.test.ts index 3b816dba3d..c45a87fe3c 100644 --- a/src/features/relay/read-retry.test.ts +++ b/src/features/relay/read-retry.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, expect, it, vi } from "vitest"; -import { isTransientFailure, ReadError, ReadRetriesExhausted } from "./errors"; +import { ReadError } from "./errors"; import { retryRead } from "./read-retry"; beforeEach(() => { @@ -24,16 +24,11 @@ it("retries transient reads at the backoff boundaries and returns success", asyn expect(read).toHaveBeenCalledTimes(2); }); -it("exhausts once across nested owners and preserves the original failure", async () => { +it("rethrows the last failure after one read and three retries", async () => { const error = new ReadError("unavailable", "offline", 503); const read = vi.fn().mockRejectedValue(error); - const signal = new AbortController().signal; - const pending = retryRead(() => retryRead(read, signal), signal); - const rejected = expect(pending).rejects.toMatchObject({ - name: "ReadRetriesExhausted", - message: "offline", - cause: error, - }); + const pending = retryRead(read, new AbortController().signal); + const rejected = expect(pending).rejects.toBe(error); for (const [index, delay] of [750, 1500, 3000].entries()) { await vi.advanceTimersByTimeAsync(delay - 1); expect(read).toHaveBeenCalledTimes(index + 1); @@ -42,18 +37,40 @@ it("exhausts once across nested owners and preserves the original failure", asyn await rejected; await vi.advanceTimersByTimeAsync(120_000); expect(read).toHaveBeenCalledTimes(4); - expect(isTransientFailure(new ReadRetriesExhausted(error))).toBe(true); - expect(String(new ReadRetriesExhausted(error))).toBe(String(error)); - expect( - isTransientFailure( - new Error("context", { cause: new ReadRetriesExhausted(error) }), - ), - ).toBe(true); +}); + +it.each([400, 403, 404])( + "retries transport-classified edge HTTP %s pages", + async (status) => { + const read = vi + .fn() + .mockRejectedValueOnce(new ReadError("unavailable", "edge", status)) + .mockResolvedValue("loaded"); + const pending = retryRead(read, new AbortController().signal); + await vi.advanceTimersByTimeAsync(750); + expect(await pending).toBe("loaded"); + expect(read).toHaveBeenCalledTimes(2); + }, +); + +it("does not start a retry after the 10-second quiet window", async () => { + const error = new ReadError("unavailable", "Relay read timed out"); + const read = vi.fn( + () => + new Promise((_, reject) => setTimeout(() => reject(error), 4000)), + ); + const pending = retryRead(read, new AbortController().signal); + const rejected = expect(pending).rejects.toBe(error); + await vi.advanceTimersByTimeAsync(9999); + expect(read).toHaveBeenCalledTimes(2); + await vi.advanceTimersByTimeAsync(120_000); + await rejected; + expect(read).toHaveBeenCalledTimes(2); }); it.each([ new ReadError("denied", "denied", 403), - new ReadError("invalid-response", "invalid"), + new ReadError("invalid-response", "invalid", 400), new Error("Invalid input"), ])("surfaces permanent failures immediately: %s", async (error) => { const read = vi.fn().mockRejectedValue(error); @@ -69,9 +86,9 @@ it.each([5000, 60000, 86401000])( async (delay) => { const error = new ReadError("unavailable", "quota", 429, delay); const read = vi.fn().mockRejectedValue(error); - await expect( - retryRead(read, new AbortController().signal), - ).rejects.toMatchObject({ cause: error }); + await expect(retryRead(read, new AbortController().signal)).rejects.toBe( + error, + ); expect(read).toHaveBeenCalledOnce(); expect(vi.getTimerCount()).toBe(0); }, @@ -103,24 +120,6 @@ it.each([ expect(vi.getTimerCount()).toBe(0); }); -it("preserves nested exhaustion through context wrappers", async () => { - const signal = new AbortController().signal; - const read = vi.fn().mockRejectedValue(new TypeError("offline")); - const pending = retryRead(async () => { - try { - return await retryRead(read, signal); - } catch (cause) { - throw new Error("context", { cause }); - } - }, signal); - const rejected = expect(pending).rejects.toMatchObject({ - message: "context", - }); - await vi.advanceTimersByTimeAsync(5250); - await rejected; - expect(read).toHaveBeenCalledTimes(4); -}); - it("aborts backoff and releases the timer", async () => { const controller = new AbortController(); const read = vi.fn().mockRejectedValue(new TypeError("offline")); diff --git a/src/features/relay/read-retry.ts b/src/features/relay/read-retry.ts index c049b3de2f..debc1ef8aa 100644 --- a/src/features/relay/read-retry.ts +++ b/src/features/relay/read-retry.ts @@ -1,11 +1,9 @@ -import { - isTransientFailure, - readErrorKind, - ReadError, - ReadRetriesExhausted, -} from "./errors.ts"; +import { isTransientFailure, readErrorKind, ReadError } from "./errors.ts"; import { RECOVERY_DELAYS_MS } from "./recovery.ts"; +const QUIET_MS = 10_000; + +/** Transient, but not the owner's own cancellation. */ export function isRetryableRead(error: unknown): boolean { if (readErrorKind(error) === "cancelled") return false; if (error instanceof Error && error.cause !== undefined) @@ -13,22 +11,16 @@ export function isRetryableRead(error: unknown): boolean { return isTransientFailure(error); } -function exhausted(error: unknown): boolean { - return ( - error instanceof ReadRetriesExhausted || - (error instanceof Error && - error.cause !== undefined && - exhausted(error.cause)) - ); -} - -/** One initial read and three quiet retries. Nested owners share exhaustion, - * rather than multiplying budgets. The caller owns the read's lifetime. */ +/** One initial read and up to three quiet retries under session recovery's + * first three ceilings (1/2/4 s) with equal jitter, none starting after 10 s. + * Exhaustion rethrows the last failure. Use one owner per read path; the + * caller owns its lifetime. */ export async function retryRead( read: (signal: AbortSignal) => Promise, signal: AbortSignal, retryable: (error: unknown) => boolean = isRetryableRead, ): Promise { + const began = performance.now(); for (let attempt = 0; ; attempt++) { signal.throwIfAborted(); try { @@ -37,17 +29,18 @@ export async function retryRead( return value; } catch (error) { signal.throwIfAborted(); - if (exhausted(error) || !retryable(error)) throw error; - if (attempt === 3) throw new ReadRetriesExhausted(error); - const ceiling = RECOVERY_DELAYS_MS[attempt] ?? 4000; - // Admission has its own actionable cooldown. Never hide it behind a - // pending read longer than this retry's normal backoff ceiling. - if (error instanceof ReadError && (error.retryAfterMs ?? 0) > ceiling) - throw new ReadRetriesExhausted(error); - const delay = Math.max( - ceiling * (0.5 + Math.random() / 2), - error instanceof ReadError ? (error.retryAfterMs ?? 0) : 0, - ); + const ceiling = RECOVERY_DELAYS_MS[attempt] ?? 0; + const hint = error instanceof ReadError ? (error.retryAfterMs ?? 0) : 0; + const delay = Math.max(ceiling * (0.5 + Math.random() / 2), hint); + // A longer admission pause keeps its existing cooldown and action; a + // read that already waited out its own deadline surfaces it. + if ( + attempt === 3 || + !retryable(error) || + hint > ceiling || + performance.now() - began + delay > QUIET_MS + ) + throw error; await new Promise((resolve, reject) => { const abort = () => { clearTimeout(timer); diff --git a/src/features/relay/reminders.ts b/src/features/relay/reminders.ts index 2df309f39b..82b101bcb9 100644 --- a/src/features/relay/reminders.ts +++ b/src/features/relay/reminders.ts @@ -1,6 +1,6 @@ -import { retryRead } from "./read-retry"; // NIP-ER reminders (kind 30300): the viewer's self-encrypted, replaceable reminder list. // Encryption and signing stay in the host; this owner sees only decoded intent. +import { retryRead } from "./read-retry"; import type { ReadFilter, RelayEvent } from "./events"; export const REMINDER_KIND = 30300; diff --git a/src/features/relay/session-read-retry.test.ts b/src/features/relay/session-read-retry.test.ts index 163819c612..8abd2c971e 100644 --- a/src/features/relay/session-read-retry.test.ts +++ b/src/features/relay/session-read-retry.test.ts @@ -1,9 +1,7 @@ -import { retryRead } from "./read-retry"; import { afterEach, beforeEach, expect, it, vi } from "vitest"; import { createRelaySession } from "./session"; -import { ReadRetriesExhausted, ReadError } from "./errors"; -import { keypair, profile, roster } from "./testing"; -import type { LiveCallbacks } from "./live"; +import { ReadError } from "./errors"; +import { keypair, profile } from "./testing"; import type { ReadFilter, RelayEvent } from "./events"; const owners: ReturnType[] = []; @@ -23,27 +21,16 @@ function setup() { async (_filters: readonly ReadFilter[], _signal?: AbortSignal) => [] as RelayEvent[], ); - let live!: LiveCallbacks; const owner = createRelaySession({ viewer: viewer.pubkey, relayAuthor: relay.pubkey, query, media: () => undefined, - subscribe(callbacks) { - live = callbacks; - return { update() {}, retry() {}, dispose() {} }; - }, + subscribe: () => ({ update() {}, retry() {}, dispose() {} }), }); owners.push(owner); const filter = [{ kinds: [0], authors: [viewer.pubkey], limit: 1 }]; - return { - owner, - query, - viewer, - relay, - filter, - emit: (events: RelayEvent[]) => live.receive(events), - }; + return { owner, query, viewer, filter }; } it("retries a finite public read without changing transport edge classification", async () => { @@ -89,11 +76,10 @@ it("leaves roster errors transient for their existing recovery owner", async () expect(h.query).toHaveBeenCalledOnce(); }); -it.each(["caller", "cache", "dispose", "access"] as const)( +it.each(["caller", "clearCache", "dispose"] as const)( "%s cancels a finite public read's backoff", async (action) => { const h = setup(); - h.emit([roster(h.relay, "channel", [h.viewer.pubkey], 10)]); const caller = new AbortController(); h.query.mockRejectedValue(new TypeError("offline")); const pending = h.owner.session.read(h.filter, { signal: caller.signal }); @@ -105,9 +91,8 @@ it.each(["caller", "cache", "dispose", "access"] as const)( filters[0]?.kinds?.includes(0), ).length; if (action === "caller") caller.abort(); - else if (action === "cache") await h.owner.clearCache(); - else if (action === "dispose") h.owner.dispose(); - else h.emit([roster(h.relay, "channel", [], 11)]); + else if (action === "clearCache") await h.owner.clearCache(); + else h.owner.dispose(); await rejected; await vi.advanceTimersByTimeAsync(7000); expect( @@ -115,32 +100,3 @@ it.each(["caller", "cache", "dispose", "access"] as const)( ).toHaveLength(before); }, ); - -it("surfaces a long public admission pause without holding the read pending", async () => { - const h = setup(); - const error = new ReadError("unavailable", "quota", 429, 60_000); - h.query.mockRejectedValue(error); - await expect(h.owner.session.read(h.filter)).rejects.toMatchObject({ - cause: error, - }); - expect(h.query).toHaveBeenCalledOnce(); -}); - -it("marks a finite window as exhausted so an outer caller cannot restart it", async () => { - const h = setup(); - h.query.mockImplementation(async () => { - await new Promise((resolve) => setTimeout(resolve, 4000)); - throw new ReadError("unavailable", "offline"); - }); - const signal = new AbortController().signal; - const pending = retryRead( - () => h.owner.session.read(h.filter, { signal }), - signal, - ); - const rejected = expect(pending).rejects.toBeInstanceOf(ReadRetriesExhausted); - await vi.advanceTimersByTimeAsync(20000); - await rejected; - expect(h.query).toHaveBeenCalledTimes(3); - await vi.advanceTimersByTimeAsync(7000); - expect(h.query).toHaveBeenCalledTimes(3); -}); diff --git a/src/features/relay/session.ts b/src/features/relay/session.ts index 04b8e0e266..f4238e6634 100644 --- a/src/features/relay/session.ts +++ b/src/features/relay/session.ts @@ -94,12 +94,7 @@ import { type EventData, } from "./events"; import { eventVisibility } from "./event-access"; -import { - isTransientFailure, - ReadError, - ReadRetriesExhausted, - readErrorKind, -} from "./errors"; +import { isTransientFailure, ReadError, readErrorKind } from "./errors"; import { PublishRejected, browserOutboxStorage, @@ -109,7 +104,7 @@ import { import { relayPartition, transportPartition } from "./partition"; import { browserThreadFollows } from "./thread-follows"; import { createMessages } from "./messages"; -import { isRetryableRead, retryRead } from "./read-retry"; +import { retryRead } from "./read-retry"; import { createThreadView } from "./threads"; import { ByteLru } from "./budget"; import { createRelayProfiler } from "./profiling"; @@ -166,11 +161,13 @@ export function createRelaySession( const requests = createRelayReader(transport, { profiling }); // Finite public consumers have no recovery source. Internal roster, history, // catch-up and preference reads keep their existing coordinator and cooldowns. - const finiteReads = new Map< + // The request scheduler retires in-flight reads on access and cache changes; + // `retireBackoffs` applies the same rule to reads waiting out a backoff. + const backoffs = new Map< AbortController, - { filters: readonly ReadFilter[] | undefined; backoff: boolean } + readonly ReadFilter[] | undefined >(); - async function finiteRead( + function finiteRead( filters: readonly ReadFilter[] | undefined, settings: ReadOptions | undefined, work: (signal: AbortSignal) => Promise, @@ -181,48 +178,31 @@ export function createRelaySession( controller.signal, ...(settings?.signal ? [settings.signal] : []), ]); - const operation = { filters, backoff: false }; - finiteReads.set(controller, operation); - const began = performance.now(); - try { - if ( - !transport || - options.cachedOnly || - filters?.some((filter) => - filter["#h"]?.some((id) => channels.queries.get?.(id)?.cached), - ) + // Offline and cached-membership reads cannot recover by retrying. + if ( + !transport || + options.cachedOnly || + filters?.some((filter) => + filter["#h"]?.some((id) => channels.queries.get?.(id)?.cached), ) + ) + return work(signal); + return retryRead(async (signal) => { + backoffs.delete(controller); + try { return await work(signal); - return await retryRead(async (signal) => { - operation.backoff = false; - try { - return await work(signal); - } catch (error) { - operation.backoff = isRetryableRead(error); - if (operation.backoff && performance.now() - began >= 10_000) - throw new ReadRetriesExhausted(error); - throw error; - } - }, signal); - } finally { - finiteReads.delete(controller); - } + } catch (error) { + backoffs.set(controller, filters); + throw error; + } + }, signal).finally(() => backoffs.delete(controller)); } - function cancelFiniteReads(accessOnly = false) { - // The request scheduler already retires in-flight work. Do not abort a - // response applying its own authority change; its existing epoch guard - // decides whether that response can finish. This owner cancels backoff. - for (const [controller, { filters, backoff }] of finiteReads) - if ( - !accessOnly || - (backoff && - filters && - (!filters.length || - filters.some( - (filter) => !isOwnerInventoryFilter(filter, transport?.viewer), - ))) - ) - controller.abort(); + /** Without `retired`, every backoff ends; filterless reads ignore `retired`. */ + function retireBackoffs( + retired?: (filters: readonly ReadFilter[]) => boolean, + ) { + for (const [controller, filters] of backoffs) + if (!retired || (filters && retired(filters))) controller.abort(); } let revision = 0; let accessEpoch = 0; @@ -373,7 +353,6 @@ export function createRelaySession( revoking++; try { accessEpoch++; - cancelFiniteReads(true); cancelUploads(); // Lifecycle work is keyed by channel and rechecks access after every // read; with known ids, other channels' permission reads need not die. @@ -383,13 +362,13 @@ export function createRelaySession( typing.clear(); // Saved owner inventory does not depend on channel access. Preserve only // that narrow read; broad, mixed, ID and channel reads must still retire. - requests.invalidate( - (filters) => - !filters.length || - filters.some( - (filter) => !isOwnerInventoryFilter(filter, transport?.viewer), - ), - ); + const retired = (filters: readonly ReadFilter[]) => + !filters.length || + filters.some( + (filter) => !isOwnerInventoryFilter(filter, transport?.viewer), + ); + requests.invalidate(retired); + retireBackoffs(retired); const visible = visibility(); const revoked = recent .entries() @@ -2285,10 +2264,8 @@ export function createRelaySession( outbox: writes?.outbox, /** People by profile name only; see `createProfileSearch`. */ searchProfiles: (query: string, settings: ProfileSearchOptions) => - finiteRead( - [{ kinds: [0], search: query, limit: settings.limit }], - settings, - (signal) => searchProfiles(query, { ...settings, signal }), + finiteRead(undefined, settings, (signal) => + searchProfiles(query, { ...settings, signal }), ), /** A fresh verified remote page under the normal access guards, without * merging live observations or local outbox intent into paging evidence. */ @@ -2550,11 +2527,11 @@ export function createRelaySession( controller = owned; update({ status: "loading", error: undefined }); try { - const read = (signal: AbortSignal) => - verified.read(filters, { signal }); - const events = await (!transport || options.cachedOnly - ? read(owned.signal) - : retryRead(read, owned.signal)); + const events = await finiteRead( + filters, + { signal: owned.signal }, + (signal) => verified.read(filters, { signal }), + ); if (disposed || owned.signal.aborted) return; remote = retainEvents([...events, ...remote]); update({ status: "ready" }); @@ -3169,7 +3146,6 @@ export function createRelaySession( accessEpoch++; cancelUploads(); cacheClearEpoch++; - cancelFiniteReads(); dropHintConfirmations(); activity.clear(); memories.clear(); @@ -3192,6 +3168,7 @@ export function createRelaySession( unread.clear(); inboxFeed.clear(); requests.invalidate(); + retireBackoffs(); repositories.clearHomes(); profiles.clear(); emoji.clear(); diff --git a/src/features/repos/git-read-retry.test.ts b/src/features/repos/git-read-retry.test.ts index d96507ecde..70eec0cec2 100644 --- a/src/features/repos/git-read-retry.test.ts +++ b/src/features/repos/git-read-retry.test.ts @@ -35,16 +35,6 @@ it("retries immutable Git reads and still validates their returned snapshot", as expect(request).toHaveBeenCalledTimes(2); for (const [body] of request.mock.calls) expect(body).toEqual(input); }); -it.each([401, 403])( - "does not retry Git authorization status %s", - async (status) => { - const request = vi.fn().mockResolvedValue(new Response(null, { status })); - await expect( - repoGitHost(request).read(input, new AbortController().signal), - ).rejects.toMatchObject({ kind: "denied" }); - expect(request).toHaveBeenCalledOnce(); - }, -); it("rejects a successful but invalid Git snapshot without retrying", async () => { const request = vi .fn() @@ -57,27 +47,6 @@ it("rejects a successful but invalid Git snapshot without retrying", async () => expect(request).toHaveBeenCalledOnce(); }); -it.each(["row", "utf8"])( - "does not retry malformed Git wire data: %s", - async (kind) => { - const request = vi - .fn() - .mockImplementation(async () => - kind === "row" - ? Response.json({ ...empty, head: "a".repeat(40), files: [null] }) - : new Response(Uint8Array.of(255)), - ); - const pending = repoGitHost(request).read( - input, - new AbortController().signal, - ); - const rejected = expect(pending).rejects.toBeInstanceOf(Error); - await vi.advanceTimersByTimeAsync(7000); - await rejected; - expect(request).toHaveBeenCalledOnce(); - }, -); - it("preserves Git not-found without consuming a retry budget", async () => { const request = vi .fn() diff --git a/src/features/repos/git.ts b/src/features/repos/git.ts index 9d7f7dcfe4..d1a18298b3 100644 --- a/src/features/repos/git.ts +++ b/src/features/repos/git.ts @@ -1,4 +1,4 @@ -import { ReadError } from "../relay/errors.ts"; +import { ReadError, transientStatus } from "../relay/errors.ts"; import { isRetryableRead, retryRead } from "../relay/read-retry.ts"; import { entityDtag, entityHex, gitHash } from "./routes.ts"; @@ -92,23 +92,16 @@ export async function gitReadText(response: Response) { if (!response.body) throw new Error("Git response missing"); const reader = response.body.getReader(); const decoder = new TextDecoder("utf-8", { fatal: true }); - const decode = (chunk?: Uint8Array, stream = false) => { - try { - return decoder.decode(chunk, { stream }); - } catch { - throw new ReadError("invalid-response", "Invalid repository response"); - } - }; let bytes = 0, text = ""; try { while (true) { const { value, done } = await reader.read(); - if (done) return text + decode(); + if (done) return text + decoder.decode(); bytes += value.byteLength; if (bytes > GIT_READ_BYTES) throw new Error("Git response exceeds the size limit"); - text += decode(value, true); + text += decoder.decode(value, { stream: true }); } } finally { await reader.cancel().catch(() => {}); @@ -136,68 +129,55 @@ export function repoGitHost( return gitReadText(response); }, signal, - (error) => { - // Git's existing missing/invalid request classification is actionable. - // This is endpoint-specific; relay edge errors keep their kind taxonomy. - if ( - error instanceof ReadError && - error.status !== undefined && - error.status >= 400 && - error.status < 500 && - error.status !== 408 && - error.status !== 429 - ) - return false; - return isRetryableRead(error); - }, + // A missing repository or invalid request stays actionable. + (error) => + error instanceof ReadError && error.status !== undefined + ? transientStatus(error.status) + : isRetryableRead(error), ); + const value = JSON.parse(text) as GitSnapshot; signal.throwIfAborted(); - try { - const value = JSON.parse(text) as GitSnapshot; - if ( - !value || - (value.head !== null && !gitHash.test(value.head)) || - (body.commit && value.head !== body.commit) || - !Array.isArray(value.commits) || - value.commits.length > 100 || - value.commits.some( - (c) => - !gitHash.test(c.hash) || - typeof c.author !== "string" || - !Number.isSafeInteger(c.date) || - typeof c.subject !== "string", - ) || - !Array.isArray(value.files) || - value.files.length > 20000 || - value.files.some( - (f) => - typeof f.path !== "string" || - !gitHash.test(f.hash) || - !Number.isSafeInteger(f.size) || - f.size < 0, - ) || - (value.head === null && - (value.commits.length > 0 || - value.files.length > 0 || - value.readme !== null || - value.file !== null || - value.diff !== null)) || - (value.readme !== null && typeof value.readme !== "string") || - (value.diff !== null && typeof value.diff !== "string") || - (body.commit && value.diff === null) || - (body.path && value.file?.path !== body.path) || - (value.file !== null && - (typeof value.file.path !== "string" || - !Number.isSafeInteger(value.file.size) || - value.file.size < 0 || - (value.file.content !== null && - typeof value.file.content !== "string"))) - ) - throw new Error("Invalid repository response"); - return value; - } catch { - throw new ReadError("invalid-response", "Invalid repository response"); - } + if ( + !value || + (value.head !== null && !gitHash.test(value.head)) || + (body.commit && value.head !== body.commit) || + !Array.isArray(value.commits) || + value.commits.length > 100 || + value.commits.some( + (c) => + !gitHash.test(c.hash) || + typeof c.author !== "string" || + !Number.isSafeInteger(c.date) || + typeof c.subject !== "string", + ) || + !Array.isArray(value.files) || + value.files.length > 20000 || + value.files.some( + (f) => + typeof f.path !== "string" || + !gitHash.test(f.hash) || + !Number.isSafeInteger(f.size) || + f.size < 0, + ) || + (value.head === null && + (value.commits.length > 0 || + value.files.length > 0 || + value.readme !== null || + value.file !== null || + value.diff !== null)) || + (value.readme !== null && typeof value.readme !== "string") || + (value.diff !== null && typeof value.diff !== "string") || + (body.commit && value.diff === null) || + (body.path && value.file?.path !== body.path) || + (value.file !== null && + (typeof value.file.path !== "string" || + !Number.isSafeInteger(value.file.size) || + value.file.size < 0 || + (value.file.content !== null && + typeof value.file.content !== "string"))) + ) + throw new Error("Invalid repository response"); + return value; }, }; } diff --git a/tests/browser/history-loading.spec.mjs b/tests/browser/history-loading.spec.mjs index 16d2002b77..5d7bb8afa7 100644 --- a/tests/browser/history-loading.spec.mjs +++ b/tests/browser/history-loading.spec.mjs @@ -150,7 +150,8 @@ test.describe("quota retry without threshold-driven continuation", () => { if (filters.some(olderAlpha)) browserCursors.push(filters); } }); - app.relay.quotaNextOlder("alpha", 0); + // A hint past the first backoff ceiling surfaces instead of retrying quietly. + app.relay.quotaNextOlder("alpha"); const cursors = () => app.report.queries.filter(({ filter }) => olderAlpha(filter)); await history(page).hover(); @@ -594,7 +595,7 @@ test.describe("pages inside one second", () => { app, }) => { await open(page, app); - app.relay.quotaNextOlder("alpha", 0); + app.relay.quotaNextOlder("alpha"); await history(page).hover(); await page.mouse.wheel(0, -100000); await expect(history(page).getByRole("alert")).toContainText( From 36fbdfdc44d6a20e5cbee5f3ee0acdf9ab4c863f Mon Sep 17 00:00:00 2001 From: Kalvin Chau Date: Wed, 28 Oct 2026 10:09:31 -0700 Subject: [PATCH 3/3] fix: keep one bounded retry owner per read after review drop the hidden-dm hook's own read loop, leaving session.read as the only retry owner. spend one quiet media recovery budget per source so a failure after loadedmetadata surfaces. retry the relay source inside inventoryReader instead of a library wrapper that could not fire. keep the hidden-conversation refresh signal per lifecycle so roster commits join it. route inbox reads through finiteRead so cached-only failures surface at once. Co-authored-by: peon <9ac6794b000690b7e814eb1805ad32405d0bec7d52838de3a86cf967565dacc0@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 Signed-off-by: Kalvin Chau --- src/bundled/channels/useHiddenDms.test.tsx | 7 +- src/bundled/channels/useHiddenDms.ts | 99 +++++++------------ src/features/agents/inventory.test.ts | 24 +++++ src/features/agents/inventory.ts | 4 +- src/features/agents/library.ts | 3 +- .../ChannelInventory.test.tsx | 12 ++- .../channel-navigation/ChannelInventory.tsx | 11 ++- .../messages/use-media-read-recovery.test.tsx | 13 +++ .../messages/use-media-read-recovery.ts | 5 + src/features/relay/inbox-feed.ts | 50 +++++----- src/features/relay/session-read-retry.test.ts | 36 +++++-- src/features/relay/session.ts | 39 +++++--- 12 files changed, 181 insertions(+), 122 deletions(-) diff --git a/src/bundled/channels/useHiddenDms.test.tsx b/src/bundled/channels/useHiddenDms.test.tsx index 8cc390218f..d160d8db3a 100644 --- a/src/bundled/channels/useHiddenDms.test.tsx +++ b/src/bundled/channels/useHiddenDms.test.tsx @@ -300,7 +300,7 @@ it("restores for an offline message that sorts behind the previous head", async expect(restored.result.current.hiddenIds.has("dm")).toBe(false); }); -it("retries a transient hidden-DM history failure", async () => { +it("leaves hidden-DM history retries to the session read", async () => { vi.useFakeTimers(); const h = fixture(); h.failNextRead(); @@ -308,11 +308,10 @@ it("retries a transient hidden-DM history failure", async () => { useHiddenDms("community:alice", h.session, h.list), ); act(() => view.result.current.hide("dm")); - expect(h.read).toHaveBeenCalledTimes(1); await act(async () => { - await vi.advanceTimersByTimeAsync(500); + await vi.advanceTimersByTimeAsync(10_000); }); - expect(h.read).toHaveBeenCalledTimes(2); + expect(h.read).toHaveBeenCalledTimes(1); expect(view.result.current.hiddenIds.has("dm")).toBe(true); }); diff --git a/src/bundled/channels/useHiddenDms.ts b/src/bundled/channels/useHiddenDms.ts index b47a1785fb..195d4b4260 100644 --- a/src/bundled/channels/useHiddenDms.ts +++ b/src/bundled/channels/useHiddenDms.ts @@ -25,19 +25,6 @@ type HiddenDm = { }; const key = "hidden-dms"; -function pause(ms: number, signal: AbortSignal): Promise { - return new Promise((resolve) => { - if (signal.aborted) return resolve(); - const done = () => { - clearTimeout(timer); - signal.removeEventListener("abort", done); - resolve(); - }; - const timer = setTimeout(done, ms); - signal.addEventListener("abort", done, { once: true }); - }); -} - function restore(scope: string): HiddenDm[] { const saved = readView(scope, key, []); if (!Array.isArray(saved)) return []; @@ -209,57 +196,47 @@ export function useHiddenDms( const entry = current.current.find((item) => item.id === id); if (!entry) continue; if (!dmIds.has(id)) continue; - for ( - let attempt = 0; - attempt < 3 && !controller.signal.aborted; - attempt++ - ) { - try { - // Save a deeper initial window so deleting a recent head does not - // make older pre-hide history look like a new message. - const events = await session.read( - [ - { - kinds: [9, 40002, 40008], - "#h": [id], - limit: entry.knownIds ? 50 : 100, - }, - ], + // `session.read` owns the quiet transient retry for this finite read. + try { + // Save a deeper initial window so deleting a recent head does not + // make older pre-hide history look like a new message. + const events = await session.read( + [ { - signal: controller.signal, - priority: "background", + kinds: [9, 40002, 40008], + "#h": [id], + limit: entry.knownIds ? 50 : 100, }, + ], + { + signal: controller.signal, + priority: "background", + }, + ); + if (!current.current.includes(entry)) continue; + const latest = session.unread.snapshot({ + kind: "channel", + channelId: id, + }).latestMessage; + const ids = events.map((event) => event.id); + const changed = + entry.knownIds && + ids.some((eventId) => !entry.knownIds?.includes(eventId)); + if (changed) show([id]); + else if (entry.baseline === undefined || !entry.knownIds) + update( + current.current.map((item) => + item === entry + ? { + ...item, + baseline: latest ?? null, + knownIds: ids, + } + : item, + ), ); - if (current.current.includes(entry)) { - const latest = session.unread.snapshot({ - kind: "channel", - channelId: id, - }).latestMessage; - const ids = events.map((event) => event.id); - const changed = - entry.knownIds && - ids.some((eventId) => !entry.knownIds?.includes(eventId)); - if (changed) { - show([id]); - break; - } - if (entry.baseline === undefined || !entry.knownIds) - update( - current.current.map((item) => - item === entry - ? { - ...item, - baseline: latest ?? null, - knownIds: ids, - } - : item, - ), - ); - } - break; - } catch { - if (attempt < 2) await pause(500 * 2 ** attempt, controller.signal); - } + } catch { + // Keep the hidden entry; the next roster or hide change rechecks it. } } })(); diff --git a/src/features/agents/inventory.test.ts b/src/features/agents/inventory.test.ts index 78a41e0ded..df2838d019 100644 --- a/src/features/agents/inventory.test.ts +++ b/src/features/agents/inventory.test.ts @@ -92,6 +92,30 @@ it("keeps either available source visible and makes partial failure explicit", a ); }); +it("quietly retries a transient relay inventory failure before merging", async () => { + vi.useFakeTimers(); + vi.spyOn(Math, "random").mockReturnValue(0.5); + try { + const identity = { pubkey: "b".repeat(64), name: "Relay" }; + const relay = vi + .fn() + .mockRejectedValueOnce(new TypeError("offline")) + .mockResolvedValue({ definitions: [], identities: [identity] }); + const pending = inventoryReader( + async () => ({ definitions: [], identities: [] }), + relay, + )(new AbortController().signal); + await vi.advanceTimersByTimeAsync(750); + const result = await pending; + expect(relay).toHaveBeenCalledTimes(2); + expect(result.identities).toEqual([identity]); + expect(result).not.toHaveProperty("error"); + } finally { + vi.restoreAllMocks(); + vi.useRealTimers(); + } +}); + it("actual session reads owner-signed relay inventory and clears/disposes it", async () => { const viewer = keypair(); const query = vi.fn().mockResolvedValue([ diff --git a/src/features/agents/inventory.ts b/src/features/agents/inventory.ts index 2d08cd7f53..cddf524645 100644 --- a/src/features/agents/inventory.ts +++ b/src/features/agents/inventory.ts @@ -1,5 +1,6 @@ import type { AgentLibrary, AgentLibraryReader } from "./library"; import { definitionSlug } from "./relay-library"; +import { retryRead } from "../relay/read-retry"; /** Preserve local names/artwork; add relay-only keys. Names never join identities. */ export function combineInventory( @@ -66,7 +67,8 @@ export function inventoryReader( const empty = { definitions: [], identities: [] }; const [saved, remote] = await Promise.allSettled([ local ? local(signal) : Promise.resolve(empty), - relay(signal), + // Only the relay source is a finite network read; it owns its retries. + retryRead(relay, signal), ]); signal.throwIfAborted(); if (remote.status === "rejected" && (!local || saved.status === "rejected")) diff --git a/src/features/agents/library.ts b/src/features/agents/library.ts index 0d1770662d..9f8388c89b 100644 --- a/src/features/agents/library.ts +++ b/src/features/agents/library.ts @@ -1,4 +1,3 @@ -import { retryRead } from "../relay/read-retry.ts"; /** A display-only projection of the local and owner-authored relay inventory, not proof of * ownership, membership, custody or running state. No prompts/configuration. */ export type AgentLibrary = Readonly<{ @@ -72,7 +71,7 @@ export function createAgentLibrary( .then(() => { if (closed || owned.signal.aborted) throw new Error("Library read cancelled"); - return retryRead(read, owned.signal); + return read(owned.signal); }) .then((library) => { if (!closed && !owned.signal.aborted) diff --git a/src/features/channel-navigation/ChannelInventory.test.tsx b/src/features/channel-navigation/ChannelInventory.test.tsx index af307c78a6..8782ee1181 100644 --- a/src/features/channel-navigation/ChannelInventory.test.tsx +++ b/src/features/channel-navigation/ChannelInventory.test.tsx @@ -148,6 +148,12 @@ function fixture( }; for (const listener of listeners) listener(); }); + // A discovery commit, as live roster events produce. + const commit = () => + act(() => { + list = { ...list, asOf: (list.asOf ?? 0) + 1 }; + for (const listener of listeners) listener(); + }); const drop = (id: string) => act(() => { list = { @@ -218,6 +224,7 @@ function fixture( list, session, publish, + commit, drop, connection(next: RelaySnapshot) { act(() => { @@ -259,7 +266,10 @@ it("keeps visibility recovery quiet, then offers the existing retry after exhaus expect( screen.queryByRole("button", { name: "Retry hidden conversations" }), ).not.toBeInTheDocument(); - await act(() => vi.advanceTimersByTimeAsync(5250)); + f.commit(); + await act(() => vi.advanceTimersByTimeAsync(1000)); + f.commit(); + await act(() => vi.advanceTimersByTimeAsync(4250)); expect(read).toHaveBeenCalledTimes(4); expect( screen.getByText("Hidden conversations could not be refreshed."), diff --git a/src/features/channel-navigation/ChannelInventory.tsx b/src/features/channel-navigation/ChannelInventory.tsx index eb4ac1e4a5..28dfeec8b4 100644 --- a/src/features/channel-navigation/ChannelInventory.tsx +++ b/src/features/channel-navigation/ChannelInventory.tsx @@ -146,13 +146,18 @@ export function ChannelInventory({ lifecycle.snapshot, ); const visibilityRequest = useRef(undefined); - // biome-ignore lint/correctness/useExhaustiveDependencies: a completed roster refresh also refreshes per-viewer visibility. + // Roster commits join an in-flight refresh; only unmount or a new lifecycle + // cancels it, so live discovery cannot restart its retry budget. + // biome-ignore lint/correctness/useExhaustiveDependencies: a new lifecycle retires the previous refresh signal. useEffect(() => { const request = new AbortController(); visibilityRequest.current = request; - if (list.status === "ready") - void lifecycle.refreshVisibility(request.signal); return () => request.abort(); + }, [lifecycle]); + // biome-ignore lint/correctness/useExhaustiveDependencies: a completed roster refresh also refreshes per-viewer visibility. + useEffect(() => { + if (list.status === "ready") + void lifecycle.refreshVisibility(visibilityRequest.current?.signal); }, [lifecycle, list.asOf, list.status]); const lifecycleFocus = useRef(undefined); const lifecycleTrigger = useRef(undefined); diff --git a/src/features/messages/use-media-read-recovery.test.tsx b/src/features/messages/use-media-read-recovery.test.tsx index 740c8a94c5..52edf56052 100644 --- a/src/features/messages/use-media-read-recovery.test.tsx +++ b/src/features/messages/use-media-read-recovery.test.tsx @@ -71,6 +71,19 @@ it("reloads a network failure quietly after backoff and preserves explicit playb expect(play).not.toHaveBeenCalled(); }); +it("surfaces a later failure of a recovered source instead of reloading again", async () => { + const h = setup(); + h.error(); + await advance(750); + fireEvent.loadedMetadata(h.element); + await advance(0); + expect(h.recovered).toHaveBeenCalledOnce(); + h.error(); + await advance(7000); + expect(h.element.load).toHaveBeenCalledOnce(); + expect(h.failed).toHaveBeenCalledOnce(); +}); + it("counts the initial browser failure and exposes failure after three reloads", async () => { const h = setup(); h.error(); diff --git a/src/features/messages/use-media-read-recovery.ts b/src/features/messages/use-media-read-recovery.ts index 675c5dbf97..8acf27997f 100644 --- a/src/features/messages/use-media-read-recovery.ts +++ b/src/features/messages/use-media-read-recovery.ts @@ -15,6 +15,7 @@ export function useMediaReadRecovery( recovered: () => void = () => {}, ) { const pending = useRef(undefined); + const spent = useRef(undefined); const callbacks = useRef({ failed, recovered }); callbacks.current = { failed, recovered }; useEffect(() => { @@ -27,6 +28,10 @@ export function useMediaReadRecovery( const recover = (element: HTMLMediaElement): boolean => { if (pending.current) return true; if (!source || element.error?.code !== 2) return false; + // One quiet budget per source: loaded metadata does not prove the body + // downloads, so a later failure surfaces instead of reloading forever. + if (spent.current === source) return false; + spent.current = source; const controller = new AbortController(); pending.current = controller; const initial = mediaFailure(element); diff --git a/src/features/relay/inbox-feed.ts b/src/features/relay/inbox-feed.ts index 8bfd6585a1..c20ae2e03c 100644 --- a/src/features/relay/inbox-feed.ts +++ b/src/features/relay/inbox-feed.ts @@ -1,4 +1,3 @@ -import { retryRead } from "./read-retry"; import { byteSize } from "./budget"; import type { ChannelQueries } from "./contracts"; import type { RelayEvent, ReadFilter } from "./events"; @@ -99,20 +98,16 @@ export function createInboxFeed({ let cursor: ReturnType | undefined; for (;;) { const page = [ - ...(await retryRead( - (signal) => - reader.read( - [ - { - kinds, - "#e": ids, - limit: 500, - ...(cursor ?? {}), - }, - ], - { signal }, - ), - signal, + ...(await reader.read( + [ + { + kinds, + "#e": ids, + limit: 500, + ...(cursor ?? {}), + }, + ], + { signal }, )), ].sort((a, b) => b.created_at - a.created_at || a.id.localeCompare(b.id)); const previous = cursor; @@ -146,20 +141,19 @@ export function createInboxFeed({ "#p": [viewer], limit: 50, }; - const mentions = await retryRead( - (signal) => - addressedRead(mentionFilter, signal, (events) => { - if (closed || generation !== epoch || owned.signal.aborted) - return; - const ids = project(events).map((event) => event.id); - if (ids.length) - publish({ - incomplete: Object.freeze([ - ...new Set([...snapshot.incomplete, ...ids]), - ]), - }); - }), + const mentions = await addressedRead( + mentionFilter, owned.signal, + (events) => { + if (closed || generation !== epoch || owned.signal.aborted) return; + const ids = project(events).map((event) => event.id); + if (ids.length) + publish({ + incomplete: Object.freeze([ + ...new Set([...snapshot.incomplete, ...ids]), + ]), + }); + }, ); if (closed || generation !== epoch) return; // Retry older failed targets even when newer addressed rows pushed them diff --git a/src/features/relay/session-read-retry.test.ts b/src/features/relay/session-read-retry.test.ts index 8abd2c971e..e5ea134f1f 100644 --- a/src/features/relay/session-read-retry.test.ts +++ b/src/features/relay/session-read-retry.test.ts @@ -14,20 +14,23 @@ afterEach(() => { vi.restoreAllMocks(); vi.useRealTimers(); }); -function setup() { +function setup(options?: Parameters[1]) { const viewer = keypair(), relay = keypair(); const query = vi.fn( async (_filters: readonly ReadFilter[], _signal?: AbortSignal) => [] as RelayEvent[], ); - const owner = createRelaySession({ - viewer: viewer.pubkey, - relayAuthor: relay.pubkey, - query, - media: () => undefined, - subscribe: () => ({ update() {}, retry() {}, dispose() {} }), - }); + const owner = createRelaySession( + { + viewer: viewer.pubkey, + relayAuthor: relay.pubkey, + query, + media: () => undefined, + subscribe: () => ({ update() {}, retry() {}, dispose() {} }), + }, + options, + ); owners.push(owner); const filter = [{ kinds: [0], authors: [viewer.pubkey], limit: 1 }]; return { owner, query, viewer, filter }; @@ -100,3 +103,20 @@ it.each(["caller", "clearCache", "dispose"] as const)( ).toHaveLength(before); }, ); + +it("surfaces a cached-only inbox failure without retrying it", async () => { + const h = setup({ cachedOnly: true }); + await h.owner.session.inboxFeed.ensure(); + expect(h.owner.session.inboxFeed.snapshot().status).toBe("error"); + expect(vi.getTimerCount()).toBe(0); + expect(h.query).not.toHaveBeenCalled(); +}); + +it("retries a transient inbox read through the session", async () => { + const h = setup(); + h.query.mockRejectedValueOnce(new TypeError("offline")); + const pending = h.owner.session.inboxFeed.ensure(); + await vi.advanceTimersByTimeAsync(750); + await pending; + expect(h.owner.session.inboxFeed.snapshot().status).toBe("ready"); +}); diff --git a/src/features/relay/session.ts b/src/features/relay/session.ts index f4238e6634..4d79019372 100644 --- a/src/features/relay/session.ts +++ b/src/features/relay/session.ts @@ -1001,24 +1001,33 @@ export function createRelaySession( // Fail closed at raw/admitted admission; do not relax reference visibility. reader: { read: (filters, settings) => - readVerified(filters, settings, true, (visible, raw) => { - const admitted = new Set(visible.map((event) => event.id)); - if ( - raw.some( - (event) => - [40003, 5, 9005].includes(event.kind) && - !admitted.has(event.id), - ) - ) - throw new Error( - "Inbox message updates could not be verified for current access. Retry inbox.", - ); - }), + finiteRead(filters, settings, (signal) => + readVerified( + filters, + { ...settings, signal }, + true, + (visible, raw) => { + const admitted = new Set(visible.map((event) => event.id)); + if ( + raw.some( + (event) => + [40003, 5, 9005].includes(event.kind) && + !admitted.has(event.id), + ) + ) + throw new Error( + "Inbox message updates could not be verified for current access. Retry inbox.", + ); + }, + ), + ), }, retainedEvent: unread.event, retainedEditIds: unread.retainedEditIds, addressedRead: (filter, signal, prepare) => - readVerified([filter], { signal }, true, prepare), + finiteRead([filter], { signal }, (signal) => + readVerified([filter], { signal }, true, prepare), + ), channels: channels.queries, viewer: transport?.viewer ?? "", notify, @@ -1851,6 +1860,8 @@ export function createRelaySession( () => loadState(sidebarPreferences.queries.snapshot()), () => void sidebarPreferences.queries.refresh(), ), + // Visibility owns its quiet retries and exhausts without `transient`, + // so this source only reports `busy`; its retry is not reached. recoverable( lifecycle.capability.subscribe, () => loadState(lifecycle.capability.snapshot()),