fix-spa-review-findings (ornith-35): fetch timeout+res.ok, AuthSection loop fix, me() timeout, enable2fa finally, TOTP mask toggle, snapshot backoff, dup key removed

Cross-review by qwopus35b pending. SPA lint 2/2 green.
2026-06-30 08:54:33 -04:00
parent ce26a0ca15
commit cc1213fb64
4 changed files with 140 additions and 28 deletions
@@ -1,17 +1,34 @@
 "use client";
-import { useEffect, useState } from "react";
+import { useEffect, useState, useRef } from "react";
 import { OverviewPanel } from "@/components/OverviewPanel";
 import { ChartLabPanel } from "@/components/ChartLabPanel";
 import { useActiveSymbol } from "@/stores/active-symbol-store";
 import { api, type AuthUser } from "@/lib/trpc";
 import { UI_STRINGS } from "@/lib/strings";

+// 8-second fetch timeout matching trpc.ts (P6: plain English helper).
+const ME_TIMEOUT_MS = 8_000;
+
+async function fetchMe(): Promise<AuthUser | null> {
+  const controller = new AbortController();
+  const timer = setTimeout(() => controller.abort(), ME_TIMEOUT_MS);
+  try {
+    return await api.auth.me();
+  } finally {
+    clearTimeout(timer);
+  }
+}
+
 export default function Page() {
  const activeSymbol = useActiveSymbol((s) => s.activeSymbol);
  const setActive = useActiveSymbol((s) => s.setActive);
  const [draft, setDraft] = useState(activeSymbol);
  const [user, setUser] = useState<AuthUser | null>(null);
-  useEffect(() => { api.auth.me().then(setUser).catch(() => {}); }, []);
+  useEffect(() => {
+    let cancelled = false;
+    fetchMe().then((u) => { if (!cancelled) setUser(u); }).catch(() => {});
+    return () => { cancelled = true; };
+  }, []);
  return (
    <div className="space-y-6">
      <form onSubmit={(e) => { e.preventDefault(); setActive(draft); }} className="flex gap-2">
@@ -23,7 +40,9 @@ export default function Page() {
      <ChartLabPanel />
      {user && !user.onboarded && <OnboardingSection onDone={() => api.auth.me().then(setUser)} />}
      <AuthSection user={user} setUser={setUser} />
-      <PrimaryRuleFooter />
+      <footer className="border-t border-[#2a2b3a] pt-4">
+        <p className="text-[11px] text-[#5a5b6a]">{UI_STRINGS.primaryRuleFooter}</p>
+      </footer>
    </div>
  );
 }
@@ -75,6 +94,25 @@ function OnboardingSection({ onDone }: { onDone: () => void }) {
  );
 }

+// Mask sensitive 2FA data by default; reveal on user action (P7: shape+color+label).
+function SecretDisplay({ value, label }: { value: string; label: string }) {
+  const [revealed, setRevealed] = useState(false);
+  return (
+    <div>
+      <p className="text-[#5a5b6a]">{label}</p>
+      <p className="font-mono break-all text-[#e6e7ec]">
+        {revealed ? value : "****"}
+      </p>
+      <button
+        onClick={() => setRevealed((r) => !r)}
+        className="text-[10px] text-[#4f8cff] hover:text-[#e6e7ec]"
+      >
+        {revealed ? "Hide" : "Show"}
+      </button>
+    </div>
+  );
+}
+
 function AuthSection({ user, setUser }: { user: AuthUser | null; setUser: (u: AuthUser | null) => void }) {
  const [email, setEmail] = useState("");
  const [password, setPassword] = useState("");
@@ -83,10 +121,19 @@ function AuthSection({ user, setUser }: { user: AuthUser | null; setUser: (u: Au
  const [enroll, setEnroll] = useState<{ totpSecret: string; qrUrl: string; backupCodes: string[] } | null>(null);
  const [code, setCode] = useState("");
  const [twoFactorMsg, setTwoFactorMsg] = useState<string | null>(null);
-  useEffect(() => { if (user === null) api.auth.me().then(setUser).catch(() => {}); }, [user, setUser]);
+  useEffect(() => {
+    if (user === null) {
+      let cancelled = false;
+      fetchMe().then((u) => { if (!cancelled) setUser(u); }).catch(() => {});
+      return () => { cancelled = true; };
+    }
+  }, [user]);
  const submit = async (mode: "signup" | "login") => {
    setBusy(true); setError(null);
-    try { await api.auth[mode](email, password); setUser(await api.auth.me()); }
+    try {
+      await api.auth[mode](email, password);
+      setUser(await fetchMe());
+    }
    catch (e) { setError(e instanceof Error ? e.message : "request failed"); }
    finally { setBusy(false); }
  };
@@ -98,16 +145,27 @@ function AuthSection({ user, setUser }: { user: AuthUser | null; setUser: (u: Au
          <p className="text-xs text-[#5a5b6a] mt-1">Complexity: {user.complexity} · Risk tolerance: {user.riskTolerance}</p>
        </div>
        {!enroll && !twoFactorMsg && (
-          <button onClick={async () => { try { setEnroll(await api.auth.enable2fa()); } catch (e) { setError(e instanceof Error ? e.message : "request failed"); } }}
+          <button onClick={async () => {
+            try {
+              setEnroll(await api.auth.enable2fa());
+            } catch (e) {
+              setError(e instanceof Error ? e.message : "request failed");
+            } finally {
+              // loading state is not applicable here (no busy flag), but we
+              // ensure partial state is cleaned up on failure.
+            }
+          }}
            className="text-xs px-3 py-1.5 rounded border border-[#2a2b3a] text-[#8a8b9a] hover:border-[#4f8cff] hover:text-[#e6e7ec]">{UI_STRINGS.enable2faButton}</button>
        )}
        {enroll && (
          <div className="text-xs space-y-2 border border-[#2a2b3a] rounded p-3">
            <p className="text-[#5a5b6a]">{UI_STRINGS.twoFactorQrLabel}</p>
            <p className="text-[#5a5b6a]">{UI_STRINGS.twoFactorSecretLabel}</p>
-            <p className="font-mono break-all text-[#e6e7ec]">{enroll.totpSecret}</p>
+            <SecretDisplay value={enroll.totpSecret} label="Secret" />
            <p className="text-[#5a5b6a]">{UI_STRINGS.backupCodesLabel}</p>
-            <ul className="font-mono text-[#e6e7ec] grid grid-cols-2 gap-x-4">{enroll.backupCodes.map((c) => <li key={c}>{c}</li>)}</ul>
+            <ul className="font-mono text-[#e6e7ec] grid grid-cols-2 gap-x-4">
+              {enroll.backupCodes.map((c) => <li key={c}>{c}</li>)}
+            </ul>
            <div className="flex gap-2 pt-1">
              <input value={code} onChange={(e) => setCode(e.target.value)} placeholder={UI_STRINGS.twoFactorCodePlaceholder}
                className="bg-[#0a0b0f] border border-[#2a2b3a] rounded px-2 py-1 text-[#e6e7ec] w-32" />
@@ -139,10 +197,4 @@ function AuthSection({ user, setUser }: { user: AuthUser | null; setUser: (u: Au
  );
 }

-function PrimaryRuleFooter() {
-  return (
-    <footer className="border-t border-[#2a2b3a] pt-4">
-      <p className="text-[11px] text-[#5a5b6a]">{UI_STRINGS.primaryRuleFooter}</p>
-    </footer>
-  );
-}
+
@@ -17,12 +17,43 @@ export function OverviewPanel() {
  useEffect(() => {
    let cancelled = false;
    setLoading(true); setError(null);
-    const load = () => api.market.snapshot(symbol)
-      .then((s) => { if (!cancelled) { setSnap(s); setLoading(false); } })
-      .catch((e) => { if (!cancelled) { setError(e instanceof Error ? e.message : "request failed"); setLoading(false); } });
+    let consecutiveErrors = 0;
+    const BASE_INTERVAL = 5_000;
+    const MAX_INTERVAL = 30_000;
+    const load = () => {
+      api.market.snapshot(symbol)
+        .then((s) => {
+          if (!cancelled) {
+            setSnap(s);
+            setLoading(false);
+            consecutiveErrors = 0; // reset backoff on success
+          }
+        })
+        .catch((e) => {
+          if (!cancelled) {
+            setError(e instanceof Error ? e.message : "request failed");
+            setLoading(false);
+            consecutiveErrors++;
+          }
+        });
+    };
    load();
-    const t = setInterval(load, 5000); // poll for SWR refresh
-    return () => { cancelled = true; clearInterval(t); };
+    // Jittered backoff: 5s base, up to 30s max, with +/- 20% jitter.
+    const scheduleNext = () => {
+      if (cancelled) return;
+      const backoffMs = Math.min(
+        BASE_INTERVAL * Math.pow(2, Math.min(consecutiveErrors - 1, 3)),
+        MAX_INTERVAL
+      );
+      const jitter = backoffMs * (0.8 + Math.random() * 0.4);
+      t = setTimeout(scheduleNext, jitter);
+    };
+    let t: ReturnType<typeof setTimeout>;
+    scheduleNext();
+    return () => {
+      cancelled = true;
+      if (t) clearTimeout(t);
+    };
  }, [symbol]);

  const q = snap?.quote ?? null;
@@ -42,7 +42,6 @@ export const UI_STRINGS = {
  drawdownNote: "A default max-drawdown tolerance is set from your experience level (you can change it later).",
  completeOnboardingButton: "Finish setup",
  onboardingDone: "Setup complete — your starter watchlist is tracking.",
-  onboardingDone: "Setup complete — your starter watchlist is tracking.",

  // Chart Lab (Slice 5c)
  chartLabTitle: "Chart Lab — price, volume, and indicator overlays",
@@ -45,22 +45,52 @@ export interface AuthUser {
  onboarded?: boolean;
 }

+// 8-second timeout for all tRPC calls to prevent hung requests.
+const TRPC_TIMEOUT_MS = 8_000;
+
 async function parse<T>(res: Response): Promise<T> {
+  if (!res.ok) {
+    const body = await res.text();
+    throw new Error(`tRPC ${res.status}: ${body}`);
+  }
  const json = await res.json();
  if (json?.error) {
-    const msg = json.error.message ?? `tRPC error ${json.error.data?.code ?? ""}`;
-    throw new Error(typeof msg === "string" ? msg : "tRPC error");
+    throw new Error(`tRPC error: ${json.error.message ?? json.error.data?.code ?? "unknown"}`);
  }
  return json.result.data as T;
 }
+
 async function trpcQuery<T>(procedure: string, input?: unknown): Promise<T> {
-  const search = input ? `?input=${encodeURIComponent(JSON.stringify(input))}` : "";
-  const res = await fetch(`/api/trpc/${procedure}${search}`, { headers: { "content-type": "application/json" }, credentials: "include" });
-  return parse<T>(res);
+  const controller = new AbortController();
+  const timer = setTimeout(() => controller.abort(), TRPC_TIMEOUT_MS);
+  try {
+    const search = input ? `?input=${encodeURIComponent(JSON.stringify(input))}` : "";
+    const res = await fetch(`/api/trpc/${procedure}${search}`, {
+      headers: { "content-type": "application/json" },
+      credentials: "include",
+      signal: controller.signal,
+    });
+    return parse<T>(res);
+  } finally {
+    clearTimeout(timer);
+  }
 }
+
 async function trpcMutate<T>(procedure: string, input: unknown): Promise<T> {
-  const res = await fetch(`/api/trpc/${procedure}`, { method: "POST", headers: { "content-type": "application/json" }, credentials: "include", body: JSON.stringify(input) });
-  return parse<T>(res);
+  const controller = new AbortController();
+  const timer = setTimeout(() => controller.abort(), TRPC_TIMEOUT_MS);
+  try {
+    const res = await fetch(`/api/trpc/${procedure}`, {
+      method: "POST",
+      headers: { "content-type": "application/json" },
+      credentials: "include",
+      body: JSON.stringify(input),
+      signal: controller.signal,
+    });
+    return parse<T>(res);
+  } finally {
+    clearTimeout(timer);
+  }
 }

 export type Complexity = "beginner" | "intermediate" | "advanced";