diff --git a/apps/loopover-ui/src/lib/api/request-notify-retry.test.ts b/apps/loopover-ui/src/lib/api/request-notify-retry.test.ts new file mode 100644 index 0000000000..75143cf046 --- /dev/null +++ b/apps/loopover-ui/src/lib/api/request-notify-retry.test.ts @@ -0,0 +1,61 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +// #8676: notifyApiFailure shows a sonner toast whose "Retry" action drives runRetryWithProgress' in-flight +// re-entrancy guard. Mock sonner to capture each toast.error's action.onClick, and stub ./status so +// pingHealth/getApiStatus don't touch the network. +const { toast } = vi.hoisted(() => ({ + toast: Object.assign(vi.fn(), { + error: vi.fn(), + loading: vi.fn(), + success: vi.fn(), + dismiss: vi.fn(), + }), +})); +vi.mock("sonner", () => ({ toast })); +vi.mock("./status", () => ({ + beginRequest: vi.fn(), + endRequest: vi.fn(), + reportApiFailure: vi.fn(), + reportApiOk: vi.fn(), + describeApiStatus: () => "", + getApiStatus: () => ({ status: "ok" }), + pingHealth: vi.fn(), +})); + +import { notifyApiFailure } from "./request"; + +function lastRetryAction(): () => void { + const calls = (toast.error as ReturnType).mock.calls; + const opts = calls.at(-1)?.[1] as { action?: { onClick: () => void } } | undefined; + return opts?.action?.onClick as () => void; +} + +describe("notifyApiFailure re-entrancy guard (#8676)", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("preserves an in-flight retrying flag so a mid-retry notifyApiFailure can't double-fire the retry", async () => { + // A retry that never settles keeps `retrying` true for the whole test (the .finally that clears it never runs). + const retry = vi.fn(() => new Promise(() => {})); + notifyApiFailure({ label: "widget", kind: "http", status: 500, retry }); + lastRetryAction()(); // click Retry -> runRetryWithProgress sets retrying:true, calls retry() (next microtask) + await Promise.resolve(); // flush the `Promise.resolve().then(() => retry())` hop + expect(retry).toHaveBeenCalledTimes(1); + + // A second failure notification for the SAME label lands while the first retry is still in flight. Before + // #8676 this full-object-replaced the entry, resetting retrying:false and defeating the guard. + notifyApiFailure({ label: "widget", kind: "http", status: 500, retry }); + lastRetryAction()(); // click Retry again -> must be blocked by the still-true retrying guard + await Promise.resolve(); + expect(retry).toHaveBeenCalledTimes(1); // still once, not a concurrent second run + }); + + it("still updates repeatCount on repeat failures when no retry is in flight (regression)", () => { + const retry = vi.fn(); + notifyApiFailure({ label: "poll", kind: "network", retry }); + notifyApiFailure({ label: "poll", kind: "network", retry }); // same kind within 5s -> repeatCount 2 + const opts = (toast.error as ReturnType).mock.calls.at(-1)?.[1] as { description?: unknown }; + expect(String(opts?.description)).toContain("2× in a row"); + }); +}); diff --git a/apps/loopover-ui/src/lib/api/request.ts b/apps/loopover-ui/src/lib/api/request.ts index beab6500ba..422f3c3194 100644 --- a/apps/loopover-ui/src/lib/api/request.ts +++ b/apps/loopover-ui/src/lib/api/request.ts @@ -125,7 +125,11 @@ export function notifyApiFailure(args: { const recent = prev && now - prev.lastNotifiedAt < 5000; const repeatCount = sameKind && recent ? prev.repeatCount + 1 : 1; - notifierState.set(id, { kind, status, lastNotifiedAt: now, repeatCount, retrying: false }); + // #8676: MERGE, don't replace -- a full-object replacement here reset `retrying` to false on the shared + // entry even while runRetryWithProgress had a retry in flight, silently defeating its `if (entry.retrying) + // return` re-entrancy guard and allowing a concurrent second retry. Preserve an in-flight `true`; all other + // fields still update as before. + notifierState.set(id, { kind, status, lastNotifiedAt: now, repeatCount, retrying: prev?.retrying === true }); const statusLabel = apiStatus !== "ok" && apiStatus !== "idle" ? describeApiStatus(apiStatus) : null;