gsadmin/pad
1
0
Fork 0
mirror of https://github.com/PerpetualSoftware/pad.git synced 2026-09-21 10:03:29 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat(web): wire ContentError + retry for HTTP fail, collab offline, stuck-connecting (TASK-1376) (#516)

Browse Source 
* 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 `<div class="center-message">{error}</div>`
   with <ContentError onRetry={loadData}>. 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.
This commit is contained in:
xarmian
2026-05-12 12:34:34 -04:00
committed by GitHub
parent 1af63e3a47
commit 9cbb08a16c
1 changed files with 75 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files
web/src/routes/[username]/[workspace]/[collection]/[slug]/+page.svelte
+75 -2
View File
@@ -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}
<ContentSkeleton variant="page" />
{:else if error}
<div class="center-message">{error}</div>
<ContentError
title="Could not load item"
detail={error}
onRetry={loadData}
/>
{:else if item && collection}
<!-- Print-only footer (hidden on screen, fixed-positioned in print).
The repeating print-header was removed as part of BUG-626: a
@@ -2157,7 +2209,28 @@
/>
{/key}
{:else if ydoc}
{#if collabProvider?.state === 'connecting' && !hasEverSynced}
<!--
Error gate FIRST so a stuck-connecting condition surfaces
error UI instead of a perpetual shimmer. Both branches
that fire here imply `!hasEverSynced` (staleConnecting's
effect only arms its timer when !hasEverSynced; offline +
hasEverSynced is handled below). That invariant is what
makes `retryCollabSync` safe to call: with no prior sync,
the current Y.Doc cannot hold user edits, so tearing down
the provider can't lose unflushed work. The offline +
hasEverSynced case deliberately KEEPS the editor mounted
— the corner badge (line ~1700) already signals offline,
the existing reconnect loop in CollabProvider keeps
trying, and any in-progress user edits remain bound to
the live Y.Doc. Per Codex review round 2 of TASK-1376.
-->
{#if (collabProvider?.state === 'offline' && !hasEverSynced) || staleConnecting}
<ContentError
title="Content unavailable"
detail="Could not sync with the server. Reload the editor to try again."
onRetry={retryCollabSync}
/>
{:else if collabProvider?.state === 'connecting' && !hasEverSynced}
<ContentSkeleton variant="inline" />
{:else}
{#key `${item.id}:true:${forceRefreshNonce}`}