From b95ba75ac84c669abf300de4e1ec17f70b4658db Mon Sep 17 00:00:00 2001 From: Light1YT Date: Sat, 13 Jun 2026 18:10:21 +0500 Subject: [PATCH] =?UTF-8?q?perf(site-finder):=20honour=20AbortSignal=20?= =?UTF-8?q?=D0=B2=20poll-=D1=86=D0=B8=D0=BA=D0=BB=D0=B5=20useParcelAnalyze?= =?UTF-8?q?Query=20(#1248)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit queryFn игнорировал TanStack v5 AbortSignal: first POST /analyze, poll GET /fetch-status и re-POST не передавали signal, цикл спал на non-abortable setTimeout без проверки signal.aborted. При unmount / смене cad|horizon цикл жил до 2-мин кэпа, дёргая GET /fetch-status каждые 2с + POST /analyze на каждый ready — зомби-поллинг. Проброс signal во все 3 fetch, sleep через shared abortableSleep (extracted из useSiteAnalysis #1242), throw AbortError перед итерацией. +3 vitest (abort + happy-path). Closes #1248 --- frontend/src/hooks/useSiteAnalysis.ts | 26 +- .../__tests__/useParcelAnalyzeQuery.test.ts | 247 ++++++++++++++++++ frontend/src/lib/abortableSleep.ts | 32 +++ frontend/src/lib/site-finder-api.ts | 22 +- 4 files changed, 297 insertions(+), 30 deletions(-) create mode 100644 frontend/src/lib/__tests__/useParcelAnalyzeQuery.test.ts create mode 100644 frontend/src/lib/abortableSleep.ts diff --git a/frontend/src/hooks/useSiteAnalysis.ts b/frontend/src/hooks/useSiteAnalysis.ts index f06ee217..7792b0af 100644 --- a/frontend/src/hooks/useSiteAnalysis.ts +++ b/frontend/src/hooks/useSiteAnalysis.ts @@ -4,6 +4,7 @@ import { useMutation } from "@tanstack/react-query"; import { useCallback, useRef, useState } from "react"; import { HTTPError, apiFetch, apiFetchWithStatus } from "@/lib/api"; +import { abortableSleep } from "@/lib/abortableSleep"; import type { ParcelAnalysis } from "@/types/site-finder"; // #93 — on-demand cadastre fetch flow. @@ -51,31 +52,6 @@ export interface AnalyzeOptions { weights?: Record | null; } -/** - * Sleep, but resolve early if the AbortSignal fires. - * - * Используем чтобы цикл polling'а пробуждался сразу при cancel(), - * а не ждал полные POLL_INTERVAL_MS до следующей проверки. - */ -function abortableSleep(ms: number, signal: AbortSignal): Promise { - return new Promise((resolve) => { - if (signal.aborted) { - resolve(); - return; - } - const timer = setTimeout(() => { - signal.removeEventListener("abort", onAbort); - resolve(); - }, ms); - const onAbort = () => { - clearTimeout(timer); - signal.removeEventListener("abort", onAbort); - resolve(); - }; - signal.addEventListener("abort", onAbort, { once: true }); - }); -} - /** * Custom hook для analyze flow с graceful on-demand fetch fallback. * diff --git a/frontend/src/lib/__tests__/useParcelAnalyzeQuery.test.ts b/frontend/src/lib/__tests__/useParcelAnalyzeQuery.test.ts new file mode 100644 index 00000000..f84f2033 --- /dev/null +++ b/frontend/src/lib/__tests__/useParcelAnalyzeQuery.test.ts @@ -0,0 +1,247 @@ +/** + * Regression coverage for issue #1248 (P3 performance). + * + * Bug: `useParcelAnalyzeQuery`'s queryFn polled GET /fetch-status (+ re-POST + * /analyze) in a `for (i < 60)` loop with a non-abortable `setTimeout` and never + * read the TanStack Query v5 AbortSignal. On unmount / смена cad|horizon the + * loop kept firing requests up to the 2-min hard cap — zombie-polling. + * + * Fix: queryFn destructures `{ signal }`, forwards it to every apiFetch call, + * sleeps via `abortableSleep`, and throws AbortError before each iteration once + * `signal.aborted`. The sibling mutation flow (useSiteAnalysis #1242) already + * did this; both now share `@/lib/abortableSleep`. + * + * Strategy: `@tanstack/react-query` is mocked so the hook's `useQuery(options)` + * call simply captures the options object. We then invoke `options.queryFn` + * directly with a real AbortSignal and a per-URL `fetch` stub, under fake + * timers, and assert on abort behaviour + the happy path. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +// ── Capture the options passed to useQuery ─────────────────────────────────── +// The captured options carry the polling queryFn we want to drive directly. +interface CapturedQueryOptions { + queryKey: readonly unknown[]; + queryFn: (ctx: { signal: AbortSignal }) => Promise; +} + +// vi.mock is hoisted; keep the captured ref in a hoisted holder so both the +// factory and the tests can reach it. +const captured = vi.hoisted<{ options: CapturedQueryOptions | null }>(() => ({ + options: null, +})); + +vi.mock("@tanstack/react-query", () => ({ + keepPreviousData: Symbol("keepPreviousData"), + useQuery: (options: CapturedQueryOptions) => { + captured.options = options; + return {}; + }, +})); + +import { useParcelAnalyzeQuery } from "../site-finder-api"; + +// ── fetch router ───────────────────────────────────────────────────────────── + +interface MockSpec { + status: number; + body?: unknown; +} + +/** Minimal Response stub — only the fields apiFetch/apiFetchWithStatus read. */ +function makeResponse({ status, body }: MockSpec): Response { + const text = body == null ? "" : JSON.stringify(body); + return { + ok: status >= 200 && status < 300, + status, + text: () => Promise.resolve(text), + } as unknown as Response; +} + +/** + * Routes the stubbed fetch by URL + method and records every call so we can + * assert that nothing fires after abort. `statusSeq` is the queue of + * /fetch-status responses, consumed in order. + */ +function installFetchRouter(opts: { + firstAnalyze: MockSpec; + statusSeq: MockSpec[]; + secondAnalyze?: MockSpec; +}) { + const calls: Array<{ url: string; method: string }> = []; + let analyzeCount = 0; + let statusIdx = 0; + + const fn = vi.fn((input: RequestInfo | URL, init?: RequestInit) => { + const url = String(input); + const method = (init?.method ?? "GET").toUpperCase(); + calls.push({ url, method }); + + // Mirror the browser: a pre-aborted signal makes fetch reject immediately. + if (init?.signal?.aborted) { + return Promise.reject(new DOMException("Aborted", "AbortError")); + } + + if (url.includes("/analyze")) { + analyzeCount += 1; + const spec = + analyzeCount === 1 + ? opts.firstAnalyze + : (opts.secondAnalyze ?? { status: 200, body: {} }); + return Promise.resolve(makeResponse(spec)); + } + if (url.includes("/fetch-status")) { + const spec = opts.statusSeq[statusIdx] ?? { + status: 200, + body: { status: "fetching" }, + }; + statusIdx += 1; + return Promise.resolve(makeResponse(spec)); + } + return Promise.reject(new Error(`unexpected fetch: ${method} ${url}`)); + }); + + vi.stubGlobal("fetch", fn); + return { + calls, + statusCalls: () => + calls.filter((c) => c.url.includes("/fetch-status")).length, + analyzeCalls: () => calls.filter((c) => c.url.includes("/analyze")).length, + }; +} + +const CAD = "66:41:0701045:42"; + +/** + * Run the hook so the mocked `useQuery` captures its options, then return the + * polling queryFn. Reads `captured.options` via a fresh binding so TS control- + * flow doesn't pin it (the hook mutates it opaquely through the mock). + * + * `useQuery` is fully mocked (it just records its options, no React state), so + * the rules-of-hooks invariant does not apply to this call — disable locally. + */ +function getQueryFn(): CapturedQueryOptions["queryFn"] { + // eslint-disable-next-line react-hooks/rules-of-hooks + useParcelAnalyzeQuery(CAD, 12); + const options = captured.options; + if (options === null) throw new Error("useQuery options not captured"); + return options.queryFn; +} + +function runQueryFn(signal: AbortSignal): Promise { + return getQueryFn()({ signal }); +} + +describe("useParcelAnalyzeQuery — poll loop honours AbortSignal (#1248)", () => { + beforeEach(() => { + captured.options = null; + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + }); + + it("aborts the poll loop and stops issuing requests after abort", async () => { + const controller = new AbortController(); + const router = installFetchRouter({ + firstAnalyze: { + status: 202, + body: { status: "fetching", job_id: 1, eta_seconds: 30 }, + }, + // Always "fetching" → the loop would otherwise run to the 60-iter cap. + statusSeq: Array.from({ length: 60 }, () => ({ + status: 200, + body: { + status: "fetching", + job_id: 1, + error_msg: null, + eta_seconds: 10, + }, + })), + }); + + const promise = runQueryFn(controller.signal).catch((e: unknown) => e); + + // First POST /analyze (202) resolves and the loop enters its first sleep. + await vi.advanceTimersByTimeAsync(0); + expect(router.analyzeCalls()).toBe(1); + + // One poll cycle: sleep 2s → GET /fetch-status ("fetching"). + await vi.advanceTimersByTimeAsync(2000); + expect(router.statusCalls()).toBeGreaterThanOrEqual(1); + + // Abort mid-flight (как unmount / смена cad). abortableSleep wakes at once. + controller.abort(); + await vi.advanceTimersByTimeAsync(0); + + const result = await promise; + expect(result).toBeInstanceOf(DOMException); + expect((result as DOMException).name).toBe("AbortError"); + + // No further requests after abort — even if the full 2-min cap elapses. + const callsAtAbort = router.calls.length; + await vi.advanceTimersByTimeAsync(60 * 2000); + expect(router.calls.length).toBe(callsAtAbort); + }); + + it("does not poll at all when the signal is already aborted", async () => { + const controller = new AbortController(); + controller.abort(); + const router = installFetchRouter({ + firstAnalyze: { + status: 202, + body: { status: "fetching", job_id: 1, eta_seconds: 30 }, + }, + statusSeq: [], + }); + + const result = await runQueryFn(controller.signal).catch((e: unknown) => e); + + expect(result).toBeInstanceOf(DOMException); + expect((result as DOMException).name).toBe("AbortError"); + // First POST /analyze sees the aborted signal → rejects, never polls. + expect(router.statusCalls()).toBe(0); + }); + + it("happy path: polls until ready then resolves with the 200 analysis", async () => { + const controller = new AbortController(); + const payload = { cad_num: CAD, score: 72, source: "nspd" }; + const router = installFetchRouter({ + firstAnalyze: { + status: 202, + body: { status: "fetching", job_id: 1, eta_seconds: 30 }, + }, + statusSeq: [ + { + status: 200, + body: { + status: "fetching", + job_id: 1, + error_msg: null, + eta_seconds: 10, + }, + }, + { + status: 200, + body: { status: "ready", job_id: 1, error_msg: null, eta_seconds: 0 }, + }, + ], + secondAnalyze: { status: 200, body: payload }, + }); + + const promise = runQueryFn(controller.signal); + + // first POST 202 → sleep → status(fetching) → sleep → status(ready) → re-POST 200 + await vi.advanceTimersByTimeAsync(0); + await vi.advanceTimersByTimeAsync(2000); + await vi.advanceTimersByTimeAsync(2000); + await vi.advanceTimersByTimeAsync(0); + + await expect(promise).resolves.toEqual(payload); + expect(router.analyzeCalls()).toBe(2); // first + re-trigger + expect(router.statusCalls()).toBe(2); + }); +}); diff --git a/frontend/src/lib/abortableSleep.ts b/frontend/src/lib/abortableSleep.ts new file mode 100644 index 00000000..458ef6c6 --- /dev/null +++ b/frontend/src/lib/abortableSleep.ts @@ -0,0 +1,32 @@ +/** + * Sleep for `ms`, but resolve early if the AbortSignal fires. + * + * Используем в poll-циклах (analyze / fetch-status) чтобы цикл пробуждался + * сразу при abort (unmount, смена cad/horizon, cancel), а не ждал полные `ms` + * до следующей проверки `signal.aborted`. Без этого осиротевший цикл живёт до + * hard-cap (60 × 2s = 2 мин), сыпя GET /fetch-status + POST /analyze — зомби- + * поллинг (#1248, sibling #1242). + * + * Резолвится (не реджектится) — вызывающий сам решает бросать ли AbortError, + * проверив `signal.aborted` после await. + */ +export function abortableSleep( + ms: number, + signal?: AbortSignal, +): Promise { + return new Promise((resolve) => { + if (signal?.aborted) { + resolve(); + return; + } + const onAbort = () => { + clearTimeout(timer); + resolve(); + }; + const timer = setTimeout(() => { + signal?.removeEventListener("abort", onAbort); + resolve(); + }, ms); + signal?.addEventListener("abort", onAbort, { once: true }); + }); +} diff --git a/frontend/src/lib/site-finder-api.ts b/frontend/src/lib/site-finder-api.ts index 97ca8cb7..6de20144 100644 --- a/frontend/src/lib/site-finder-api.ts +++ b/frontend/src/lib/site-finder-api.ts @@ -10,6 +10,7 @@ import { keepPreviousData, useQuery } from "@tanstack/react-query"; import { HTTPError, apiFetch, apiFetchWithStatus } from "@/lib/api"; +import { abortableSleep } from "@/lib/abortableSleep"; import type { AnalyzeAcceptedResponse, FetchStatusResponse, @@ -414,13 +415,18 @@ const ANALYZE_POLL_MAX_ITERATIONS = 60; // 60 × 2s = 2 min hard cap export function useParcelAnalyzeQuery(cad: string, horizon: number = 12) { return useQuery({ queryKey: ["parcel-analyze", cad, horizon], - queryFn: async (): Promise => { + // TanStack Query v5 passes an AbortSignal in the queryFn context; it aborts + // on unmount and whenever the queryKey changes (смена cad/horizon). Thread + // it through the POST/GET fetches and check it before each poll iteration so + // the loop dies immediately instead of zombie-polling /fetch-status + re- + // POSTing /analyze up to the 2-min hard cap (#1248). + queryFn: async ({ signal }): Promise => { if (MOCK_ANALYZE) { // Return fixture data regardless of cad/horizon — for dev only return fixtureAnalyze as ParcelAnalyzeResponse; } - // Backend accepts ?horizon= ∈ {6,12,18,24} (default 12; 422 otherwise, #995). + // Backend accepts ?horizon= ∈ {6,12,18} (default 12; 422 otherwise, #995). const analyzeUrl = `/api/v1/parcels/${encodeURIComponent( cad, )}/analyze?horizon=${horizon}`; @@ -429,7 +435,7 @@ export function useParcelAnalyzeQuery(cad: string, horizon: number = 12) { // Accepted code instead of treating it as a successful payload. const first = await apiFetchWithStatus< ParcelAnalyzeResponse | AnalyzeAcceptedResponse - >(analyzeUrl, { method: "POST" }); + >(analyzeUrl, { method: "POST", signal }); // 200 → geometry was cached, full analysis is ready. if (first.status === 200) { @@ -441,16 +447,22 @@ export function useParcelAnalyzeQuery(cad: string, horizon: number = 12) { // pending throughout, so the page's "Анализируем участок…" screen shows. if (first.status === 202) { for (let i = 0; i < ANALYZE_POLL_MAX_ITERATIONS; i++) { - await new Promise((r) => setTimeout(r, ANALYZE_POLL_INTERVAL_MS)); + // abortableSleep wakes immediately on abort; then bail before issuing + // any further requests (signal also aborts the in-flight fetches). + await abortableSleep(ANALYZE_POLL_INTERVAL_MS, signal); + if (signal.aborted) { + throw new DOMException("Анализ отменён", "AbortError"); + } const status = await apiFetch( `/api/v1/parcels/${encodeURIComponent(cad)}/fetch-status`, + { signal }, ); if (status.status === "ready") { // Re-POST should now be 200. If it races back to 202, keep polling // rather than returning the stub (symmetry with the first request). const second = await apiFetchWithStatus< ParcelAnalyzeResponse | AnalyzeAcceptedResponse - >(analyzeUrl, { method: "POST" }); + >(analyzeUrl, { method: "POST", signal }); if (second.status === 200) { return second.body as ParcelAnalyzeResponse; } -- 2.45.3