fix(website): retry skill detail fetch and show a clear error (#737)

website-src/src/__tests__/skill-detail.test.jsx

@@ -1,6 +1,12 @@
 /** @vitest-environment jsdom */
-import { afterEach, describe, expect, it } from "vitest";
-import { cleanup, render, screen } from "@testing-library/react";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import {
+  act,
+  cleanup,
+  fireEvent,
+  render,
+  screen,
+} from "@testing-library/react";
 import { HashRouter } from "react-router-dom";
 import SkillDetail from "../components/SkillDetail.jsx";
 
@@ -183,3 +189,127 @@ describe("SkillDetail — repo detail link (issue #623)", () => {
     expect(external.getAttribute("href")).toBe("https://github.com/test/repo");
   });
 });
+
+describe("SkillDetail — full-details fetch resilience (issue #737)", () => {
+  // detailCache is module-level, so every test uses its own detailPath.
+  let seq = 0;
+  const nextSkill = () => ({
+    ...baseSkill,
+    detailPath: `skills/issue-737-${++seq}.json`,
+  });
+  const fullDetail = (slim) => ({
+    ...slim,
+    description: "Full description loaded from the detail file",
+  });
+  const okResponse = (body) => ({
+    ok: true,
+    status: 200,
+    json: async () => body,
+  });
+  const flush = (ms = 0) => act(() => vi.advanceTimersByTimeAsync(ms));
+
+  beforeEach(() => {
+    vi.useFakeTimers();
+  });
+
+  afterEach(() => {
+    cleanup();
+    vi.useRealTimers();
+    vi.unstubAllGlobals();
+  });
+
+  it("retries a transient network failure and then shows the full details", async () => {
+    const slim = nextSkill();
+    const fetchMock = vi
+      .fn()
+      .mockRejectedValueOnce(new TypeError("Failed to fetch"))
+      .mockResolvedValueOnce(okResponse(fullDetail(slim)));
+    vi.stubGlobal("fetch", fetchMock);
+
+    renderDetail({ slim });
+    await flush(5000);
+
+    expect(fetchMock).toHaveBeenCalledTimes(2);
+    expect(
+      screen.getByText("Full description loaded from the detail file"),
+    ).toBeTruthy();
+    expect(screen.queryByText(/Could not load full details/)).toBeNull();
+    expect(screen.queryByText(/Failed to fetch/)).toBeNull();
+  });
+
+  it("shows a clear message and a retry button when the network stays down", async () => {
+    const slim = nextSkill();
+    const fetchMock = vi
+      .fn()
+      .mockRejectedValue(new TypeError("Failed to fetch"));
+    vi.stubGlobal("fetch", fetchMock);
+
+    renderDetail({ slim });
+    await flush(10000);
+
+    const alert = screen.getByRole("alert");
+    expect(alert.textContent).toMatch(/Could not load full details/);
+    expect(alert.textContent).toMatch(/connection/i);
+    expect(alert.textContent).not.toMatch(/Failed to fetch/);
+    // Summary and install command stay usable.
+    expect(screen.getByText("A test skill for unit testing")).toBeTruthy();
+    expect(
+      screen.getByText("asm install github:test/repo:skills/test-skill"),
+    ).toBeTruthy();
+
+    const callsBefore = fetchMock.mock.calls.length;
+    fetchMock.mockResolvedValue(okResponse(fullDetail(slim)));
+    fireEvent.click(screen.getByRole("button", { name: /try again/i }));
+    await flush(5000);
+
+    expect(fetchMock.mock.calls.length).toBe(callsBefore + 1);
+    expect(screen.queryByRole("alert")).toBeNull();
+    expect(
+      screen.getByText("Full description loaded from the detail file"),
+    ).toBeTruthy();
+  });
+
+  it("does not retry a 404", async () => {
+    const slim = nextSkill();
+    const fetchMock = vi
+      .fn()
+      .mockResolvedValue({ ok: false, status: 404, json: async () => ({}) });
+    vi.stubGlobal("fetch", fetchMock);
+
+    renderDetail({ slim });
+    await flush(10000);
+
+    expect(fetchMock).toHaveBeenCalledTimes(1);
+    expect(screen.getByRole("alert").textContent).toMatch(/HTTP 404/);
+  });
+
+  it("retries a 503 before giving up", async () => {
+    const slim = nextSkill();
+    const fetchMock = vi
+      .fn()
+      .mockResolvedValueOnce({ ok: false, status: 503, json: async () => ({}) })
+      .mockResolvedValueOnce(okResponse(fullDetail(slim)));
+    vi.stubGlobal("fetch", fetchMock);
+
+    renderDetail({ slim });
+    await flush(5000);
+
+    expect(fetchMock).toHaveBeenCalledTimes(2);
+    expect(screen.queryByRole("alert")).toBeNull();
+  });
+
+  it("stops retrying once the component unmounts", async () => {
+    const slim = nextSkill();
+    const fetchMock = vi
+      .fn()
+      .mockRejectedValue(new TypeError("Failed to fetch"));
+    vi.stubGlobal("fetch", fetchMock);
+
+    const { unmount } = renderDetail({ slim });
+    await flush(0);
+    unmount();
+    await flush(10000);
+
+    expect(fetchMock).toHaveBeenCalledTimes(1);
+  });
+});

website-src/src/components/SkillDetail.jsx

@@ -18,17 +18,54 @@ import { Card } from "./ui/card.jsx";
  * detailPath eliminates redundant fetches when navigating between
  * skills and back.
  *
+ * A network failure ("Failed to fetch") or a 5xx/408/429 response is
+ * retried with a short backoff before giving up (issue #737). If the
+ * details still cannot be loaded, a plain-language message with a
+ * "Try again" button is shown; the slim row keeps the summary and the
+ * install command usable either way.
+ *
  * Props:
  *   - slim: the slim row from catalog.skills (required)
  */
 const detailCache = new Map();
 
+/** Backoff before each automatic retry; its length is the retry count. */
+const RETRY_DELAYS_MS = [500, 1500];
+
+/** Error raised for a non-2xx response, flagged when worth retrying. */
+class DetailHttpError extends Error {
+  constructor(status) {
+    super("HTTP " + status);
+    this.retryable = status >= 500 || status === 408 || status === 429;
+  }
+}
+
+/**
+ * `fetch` rejects with a TypeError ("Failed to fetch" in Chromium) for
+ * any network-level failure: offline, DNS, reset connection, or a
+ * service worker answering with Response.error().
+ */
+function isRetryable(err) {
+  return err instanceof TypeError || err?.retryable === true;
+}
+
+function describeDetailError(err) {
+  if (err instanceof TypeError) {
+    return "The network request failed. Check your connection and try again.";
+  }
+  if (err instanceof DetailHttpError) {
+    return `The server responded with ${err.message}.`;
+  }
+  return "The details file could not be read.";
+}
+
 export default function SkillDetail({ slim }) {
   const [detail, setDetail] = useState({
     data: null,
     loading: true,
     error: null,
   });
+  const [reloadKey, setReloadKey] = useState(0);
 
   useEffect(() => {
     if (!slim?.detailPath) {
@@ -46,29 +83,41 @@ export default function SkillDetail({ slim }) {
     }
 
     let cancelled = false;
+    let retryTimer = null;
+    const controller = new AbortController();
     setDetail({ data: null, loading: true, error: null });
-    (async () => {
+    const load = async (attempt) => {
       try {
-        const res = await fetch(path);
-        if (!res.ok) throw new Error("HTTP " + res.status);
+        const res = await fetch(path, { signal: controller.signal });
+        if (!res.ok) throw new DetailHttpError(res.status);
         const data = await res.json();
         if (cancelled) return;
         // Store in cache
         detailCache.set(path, data);
         setDetail({ data, loading: false, error: null });
       } catch (err) {
         if (cancelled) return;
+        if (isRetryable(err) && attempt < RETRY_DELAYS_MS.length) {
+          retryTimer = setTimeout(
+            () => load(attempt + 1),
+            RETRY_DELAYS_MS[attempt],
+          );
+          return;
+        }
         setDetail({
           data: null,
           loading: false,
-          error: err instanceof Error ? err.message : String(err),
+          error: describeDetailError(err),
         });
       }
-    })();
+    };
+    load(0);
     return () => {
       cancelled = true;
+      clearTimeout(retryTimer);
+      controller.abort();
     };
-  }, [slim?.detailPath]);
+  }, [slim?.detailPath, reloadKey]);
 
   const skill = useMemo(() => detail.data || slim, [detail.data, slim]);
   if (!skill) return null;
@@ -224,9 +273,25 @@ export default function SkillDetail({ slim }) {
 
       {detail.loading && <p className="shop-meta">Loading details…</p>}
       {detail.error && (
-        <p className="text-xs text-[var(--warn)]">
-          ⚠ Could not load full details: {detail.error}
-        </p>
+        <div
+          role="alert"
+          className="flex flex-wrap items-center gap-x-2 rounded-md border border-[var(--warn)] bg-[var(--warn-bg)] px-3 py-1 text-xs text-[var(--fg)]"
+        >
+          <span>
+            <span className="text-[var(--warn)]" aria-hidden="true">
+              ⚠
+            </span>{" "}
+            Could not load full details. {detail.error} The summary and install
+            command above still work.
+          </span>
+          <button
+            type="button"
+            className="inline-flex min-h-11 items-center px-2 -mx-2 underline underline-offset-2 hover:text-[var(--brand)] transition-colors"
+            onClick={() => setReloadKey((k) => k + 1)}
+          >
+            Try again
+          </button>
+        </div>
       )}
     </div>
   );