fix(contributor-card): retry failed fetches instead of hiding the card forever

src/features/contributor-card.test.ts

@@ -222,6 +222,74 @@ describe("injectContributorCard", () => {
     expect(document.querySelector(BLOCK)).toBeNull();
   });
 
+  it("retries a failed fetch on a later hover instead of suppressing the card forever", async () => {
+    // fetchContributorInfo returns null on ANY error (rate limit, network blip).
+    // Caching that as permanent "no data" hid the card for the rest of the
+    // session after a single transient failure. It must retry — but not storm
+    // the network on every hovercard re-render, so a short cooldown gates it.
+    fetchContributorInfo.mockResolvedValueOnce(null); // first attempt fails
+    injectContributorCard();
+    const card = hovercard("octocat");
+    document.body.appendChild(card);
+    await flush();
+
+    expect(document.querySelector(BLOCK)).toBeNull(); // failed → nothing shown
+    expect(fetchContributorInfo).toHaveBeenCalledTimes(1);
+
+    // Re-hover within the cooldown: no refetch (no storm), still nothing.
+    card.querySelector(".content")!.appendChild(document.createElement("span"));
+    await flush();
+    expect(fetchContributorInfo).toHaveBeenCalledTimes(1);
+    expect(document.querySelector(BLOCK)).toBeNull();
+
+    // Past the cooldown, a hover retries — and this time the fetch succeeds.
+    fetchContributorInfo.mockResolvedValue(baseInfo());
+    vi.setSystemTime(new Date(NOW.getTime() + 61_000));
+    card.querySelector(".content")!.appendChild(document.createElement("span"));
+    await flush();
+
+    expect(fetchContributorInfo).toHaveBeenCalledTimes(2); // retried, not stuck
+    expect(document.querySelector<HTMLElement>(BLOCK)?.textContent).toContain("3 days");
+  });
+
+  it("a stale failed fetch doesn't blank the card now showing a different user", async () => {
+    // Hovercards are reused: hovering A then B swaps the same container's payload.
+    // When A's slow fetch finally fails, it must drop only A's skeleton — the
+    // removal is scoped by login, so a since-passed fetch can't blank the card
+    // that is now loading B.
+    const resolvers: Record<string, (v: ContributorInfo | null) => void> = {};
+    fetchContributorInfo.mockImplementation(
+      (login) => new Promise((r) => (resolvers[login] = r)),
+    );
+    injectContributorCard();
+
+    const card = hovercard("alice");
+    document.body.appendChild(card);
+    await flush();
+    expect(document.querySelector<HTMLElement>(BLOCK)?.dataset.login).toBe("alice");
+
+    // Move to bob on the same reused hovercard (GitHub swaps the payload in place,
+    // which our observer sees as a childList mutation under the content node).
+    const view = card.querySelector<HTMLElement>("[data-hydro-view]")!;
+    view.setAttribute(
+      "data-hydro-view",
+      JSON.stringify({ event_type: "user-hovercard-hover", payload: { card_user_login: "bob" } }),
+    );
+    card.querySelector(".content")!.appendChild(document.createElement("span"));
+    await flush();
+    const bobSkeleton = document.querySelector<HTMLElement>(`${BLOCK}[data-login="bob"]`);
+    expect(bobSkeleton?.dataset.skeleton).toBe("true");
+
+    // alice's fetch resolves null last — must NOT touch bob's in-flight skeleton.
+    // Assert on node identity, not presence: an unscoped removal would yank this
+    // exact node out (a later sync() re-adds a *different* one, masking the bug),
+    // so `isConnected` is what proves bob's skeleton was left alone.
+    resolvers["alice"](null);
+    await flush();
+
+    expect(bobSkeleton!.isConnected).toBe(true); // bob's own skeleton untouched
+  });
+
   it("shows a loading skeleton, then fills it in when data arrives", async () => {
     // The panel is anchored in the stable popover root (not inside the body
     // GitHub re-renders), so a skeleton→data swap is safe — it lets the facts

src/features/contributor-card.ts

@@ -204,8 +204,16 @@ function buildSkeleton(login: string): HTMLElement {
   return panel;
 }
 
-const infoCache = new Map<string, ContributorInfo | null>();
+// Successful fetches only — keyed by repo#login, persists across navigations.
+const infoCache = new Map<string, ContributorInfo>();
 const fetching = new Set<string>();
+// Logins whose last fetch FAILED, with when it failed. A failure must NOT be
+// cached as a permanent "no data": fetchContributorInfo returns null on any
+// error (rate limit, network blip), and caching that forever hid the card for
+// the rest of the session after a single transient failure. Instead we retry
+// after a short cooldown (and on any fresh page load, since this is in-memory).
+const failedFetchAt = new Map<string, number>();
+const FAILED_FETCH_RETRY_MS = 60_000;
 
 interface Card {
   container: HTMLElement;
@@ -272,8 +280,16 @@ function sync(): void {
   const cacheKey = `${repo?.owner ?? ""}/${repo?.repo ?? ""}#${login}`;
   if (infoCache.has(cacheKey)) {
     existing?.remove();
-    const info = infoCache.get(cacheKey);
-    if (info) placePanel(card, buildPanel(login, info));
+    placePanel(card, buildPanel(login, infoCache.get(cacheKey)!));
+    return;
+  }
+
+  // Fetch failed recently → don't re-shimmer or refetch on every hovercard
+  // re-render (which would storm the network); just show nothing for now. The
+  // cooldown lets a later hover retry instead of the card being stuck off.
+  const failedAt = failedFetchAt.get(cacheKey);
+  if (failedAt !== undefined && Date.now() - failedAt < FAILED_FETCH_RETRY_MS) {
+    existing?.remove();
     return;
   }
 
@@ -286,17 +302,35 @@ function sync(): void {
   }
   if (fetching.has(cacheKey)) return;
   fetching.add(cacheKey);
+
+  // A null return and a thrown error are the same outcome — fetchContributorInfo
+  // returns null on any error (rate limit, network blip). Treat both as a
+  // transient failure: clear the in-flight mark, schedule a retry after the
+  // cooldown, and drop *this login's* skeleton. Scope the removal by login so a
+  // slow fetch for a user we've since hovered past doesn't blank the card that
+  // now shows a different user.
+  const onFailure = (): void => {
+    fetching.delete(cacheKey);
+    failedFetchAt.set(cacheKey, Date.now());
+    card.container
+      .querySelector(`.${PANEL_CLASS}[data-skeleton][data-login="${login}"]`)
+      ?.remove();
+  };
   fetchContributorInfo(login, repo?.owner, repo?.repo)
     .then((data) => {
+      if (!data) {
+        // null = transient failure (see failedFetchAt note) — never cache it as
+        // permanent "no data".
+        onFailure();
+        return;
+      }
+      // Success → cache and render. Clear any prior failure mark.
       fetching.delete(cacheKey);
-      infoCache.set(cacheKey, data ?? null);
+      failedFetchAt.delete(cacheKey);
+      infoCache.set(cacheKey, data);
       sync();
     })
-    .catch(() => {
-      fetching.delete(cacheKey);
-      // Don't leave a skeleton shimmering forever on a failed fetch.
-      card.container.querySelector(`.${PANEL_CLASS}[data-skeleton]`)?.remove();
-    });
+    .catch(onFailure);
 }
 
 let observer: MutationObserver | null = null;
@@ -338,5 +372,6 @@ export function cleanupContributorCard(): void {
   }
   fetching.clear();
   infoCache.clear();
+  failedFetchAt.clear();
   document.querySelectorAll(`.${PANEL_CLASS}`).forEach((el) => el.remove());
 }