From 9cbb08a16c6f219d23cd3aa7de61deb2a6607dc2 Mon Sep 17 00:00:00 2001 From: xarmian Date: Tue, 12 May 2026 12:34:34 -0400 Subject: [PATCH] feat(web): wire ContentError + retry for HTTP fail, collab offline, stuck-connecting (TASK-1376) (#516) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(web): wire ContentError + retry for HTTP fail, collab offline, stuck-connecting (TASK-1376) Three failure modes now surface ContentError with a retry path: 1. HTTP load error (page-level): the {:else if error} branch replaces the literal `
{error}
` with . Users can recover without navigating away. 2. Collab offline: when the WS provider hits the OFFLINE_THRESHOLD (3 consecutive failed reconnects) and `state === 'offline'`, the editable {:else if ydoc} branch surfaces ContentError instead of the empty Y.Doc editor. 3. Stuck-connecting: a 10s timer-driven $effect sets staleConnecting=true if the provider sits in `connecting` without ever syncing. Same ContentError UI as offline. Timer is cleared on state change, hasEverSynced flip, or provider rebuild. Retry path (retryCollabSync) mirrors the server-driven force_refresh dance: 1. Clear staleConnecting (state will reset naturally on rebuild). 2. Refetch items.content so the lazy-seed (TASK-1261) on the new Y.Doc has canonical content. 3. Bump forceRefreshNonce → the existing collab $effect tears down the dead provider, mints a new one. The TASK-1375 reset $effect handles `hasEverSynced=false` and `editorInstance=null` as part of that rebuild, so retry doesn't need to touch them directly. Error gate is placed BEFORE the skeleton gate in the {:else if ydoc} branch so stuck-connecting flips out of shimmer-forever and into a clear error UI at the 10s mark. CONVE-606: the stuck-connecting $effect has a single clean dependency list (collabProvider + state + hasEverSynced); the latch is a pure imperative flag flipped by a setTimeout, not derivable. Parent: PLAN-1373. Resolves BUG-1372 (final piece). * fix(web): preserve local edits on retry, reset staleConnecting per provider (TASK-1376 round 1) Codex round 1 caught three correctness issues in the initial retry wire-up; addressed all of them. P1 — retryCollabSync was overwriting local edits. The original (lifted from onForceRefresh) refetched items.content before bumping forceRefreshNonce. In the server-driven force_refresh case that's correct because the server is the source of truth. In the retry case the LOCAL Y.Doc is the canonical view (it may hold unflushed user typing from the offline/connecting window); shoveling stale server content into \`item\` before the cleanup's flushCollabNow ran risked the lazy-seed on the new Y.Doc re-encoding the stale view, then the next flush PATCHing that back over the user's just-persisted edits. Fix: drop the refetch. The collab \$effect cleanup already calls flushCollabNow on tear-down (lines ~727–729), preserving local edits via PATCH BEFORE the new provider mints a fresh Y.Doc. The new provider's WS replay reconciles against server state via the op-log; if the cursor has been pruned the server sends a real force_refresh which goes through onForceRefresh (which DOES refetch — correctly). P2 (first) — failed retry left staleConnecting=false with no retry affordance. Gone naturally: retryCollabSync is now synchronous with no failure path. P2 (second) — staleConnecting was not reset when collabProvider rebuilt. The early-return-on-null path skipped the false-reset, so a stuck-connecting flag from a previous provider carried into the new one, showing error UI immediately instead of granting the fresh 10s grace. Fix: unconditional \`staleConnecting = false\` at the top of the effect (after the null guard). Only the 10s timer can flip it back to true. Codex round 1. * fix(web): gate offline error UI on !hasEverSynced to protect local edits (TASK-1376 round 2) Codex round 2: the fire-and-forget flushCollabNow in the collab \$effect cleanup is racy — it kicks off a PATCH but doesn't await runCollabFlush or update local item.content. The new provider's lazy-seed reads item.content (stale relative to the local Y.Doc), encodes it into a fresh op-log, then the next 5s flush PATCHes that stale content back over the user's just-flushed edits. Real fix: don't expose retry when there are local edits at risk. The template's error gate now reads: (collabProvider?.state === 'offline' && !hasEverSynced) || staleConnecting Both branches imply !hasEverSynced, so the current Y.Doc has never received a sync and therefore cannot hold user edits. retryCollabSync is safe in that universe — tearing down the provider can't lose unflushed work. For state === 'offline' WITH hasEverSynced=true (was synced, then got disconnected), the editor stays mounted with its bound Y.Doc: - The corner badge (line ~1700) already signals offline via the four-state pending-sync indicator. - CollabProvider's reconnect loop keeps trying with exponential backoff (1s → 30s capped); auto-recovery is the path. - In-progress user edits remain bound to the live Y.Doc; nothing destroys them. - When the WS comes back, normal sync flow reconciles. This is also a better UX than the prior "wipe editor, show error" — a user mid-edit doesn't lose their working canvas when their wifi hiccups. Codex round 2. --- .../[collection]/[slug]/+page.svelte | 77 ++++++++++++++++++- 1 file changed, 75 insertions(+), 2 deletions(-) diff --git a/web/src/routes/[username]/[workspace]/[collection]/[slug]/+page.svelte b/web/src/routes/[username]/[workspace]/[collection]/[slug]/+page.svelte index 1c444e9c..9b219bb5 100644 --- a/web/src/routes/[username]/[workspace]/[collection]/[slug]/+page.svelte +++ b/web/src/routes/[username]/[workspace]/[collection]/[slug]/+page.svelte @@ -26,6 +26,7 @@ import QuickActionsMenu from '$lib/components/common/QuickActionsMenu.svelte'; import BottomSheet from '$lib/components/common/BottomSheet.svelte'; import ContentSkeleton from '$lib/components/common/ContentSkeleton.svelte'; + import ContentError from '$lib/components/common/ContentError.svelte'; import EditCollectionModal from '$lib/components/collections/EditCollectionModal.svelte'; import ShareDialog from '$lib/components/ShareDialog.svelte'; import { copyToClipboard } from '$lib/utils/clipboard'; @@ -579,6 +580,53 @@ if (collabProvider?.synced) hasEverSynced = true; }); + // Stuck-connecting timeout — if the provider stays in + // `connecting` for >10s without ever syncing, surface the same + // ContentError UI as `offline`. Unconditional reset at the top + // of each effect run gives every fresh provider its own 10s + // grace (covers item navigation, rawMode toggle, force_refresh, + // and retry-driven rebuilds — without this, a stuck-connecting + // flag from a previous provider would carry over and the new + // provider would immediately show error UI). Only the timer + // can flip it back to true. Per Codex review round 1. + let staleConnecting = $state(false); + $effect(() => { + if (!collabProvider) return; + staleConnecting = false; + if (collabProvider.state !== 'connecting' || hasEverSynced) { + return; + } + const t = setTimeout(() => { + if (collabProvider?.state === 'connecting' && !hasEverSynced) { + staleConnecting = true; + } + }, 10_000); + return () => clearTimeout(t); + }); + + // Manual recovery from the initial-connect failure modes + // (staleConnecting / offline-while-!hasEverSynced). The template + // gate restricts retry to cases where `!hasEverSynced`, which + // means the current Y.Doc has never received a sync and therefore + // cannot hold user edits — tearing down the provider is safe. + // (Offline AFTER a successful sync keeps the editor mounted; see + // the gate comment in the template for why.) + // + // Bumps forceRefreshNonce so the collab $effect tears down the + // dead provider and rebuilds. We deliberately do NOT refetch + // items.content here — the lazy-seed on the new Y.Doc reads from + // the already-cached item.content, and the new provider's WS + // replay reconciles against canonical server state via the + // op-log. If the cursor is below MIN the server sends a real + // force_refresh which goes through onForceRefresh (which DOES + // refetch — correctly, because the server is the source of truth + // in that case). Per Codex review rounds 1 and 2 of TASK-1376. + function retryCollabSync() { + if (!item) return; + staleConnecting = false; + forceRefreshNonce += 1; + } + $effect(() => { if (!collabKey) return; const itemId = collabKey; @@ -1658,7 +1706,11 @@ {#if loading} {:else if error} -
{error}
+ {:else if item && collection} + {#if (collabProvider?.state === 'offline' && !hasEverSynced) || staleConnecting} + + {:else if collabProvider?.state === 'connecting' && !hasEverSynced} {:else} {#key `${item.id}:true:${forceRefreshNonce}`}