diff --git a/apps/web/Dockerfile b/apps/web/Dockerfile index 9f16dde98538..4146393d0dbe 100644 --- a/apps/web/Dockerfile +++ b/apps/web/Dockerfile @@ -299,7 +299,9 @@ RUN chmod -R 755 \ ./node_modules/@opentelemetry/exporter-logs-otlp-proto \ && OTEL_EXPORTER_OTLP_ENDPOINT=http://127.0.0.1:4318 node -e "\ const pino = require('pino'); \ - const target = process.cwd() + '/node_modules/pino-opentelemetry-transport/lib/pino-opentelemetry-transport.js'; \ + process.chdir('/home/nextjs/apps/web'); \ + const runtimeRoot = process.cwd().replace(/[\/\\\\]apps[\/\\\\]web$/, ''); \ + const target = runtimeRoot + '/node_modules/pino-opentelemetry-transport/lib/pino-opentelemetry-transport.js'; \ process.on('uncaughtException', (error) => { console.error(error); process.exit(1); }); \ process.on('unhandledRejection', (error) => { console.error(error); process.exit(1); }); \ const logger = pino({ \ diff --git a/apps/web/modules/ee/unify-feedback/topics-subtopics/components/topics-subtopics-container.tsx b/apps/web/modules/ee/unify-feedback/topics-subtopics/components/topics-subtopics-container.tsx index fc36ff554e59..3904474c03e8 100644 --- a/apps/web/modules/ee/unify-feedback/topics-subtopics/components/topics-subtopics-container.tsx +++ b/apps/web/modules/ee/unify-feedback/topics-subtopics/components/topics-subtopics-container.tsx @@ -16,7 +16,7 @@ import { useRenameTaxonomyNode } from "../hooks/use-rename-taxonomy-node"; import { useTaxonomyFields } from "../hooks/use-taxonomy-fields"; import { NODE_RECORD_LIMIT, useTaxonomyNodeRecords } from "../hooks/use-taxonomy-node-records"; import { useTaxonomyRecordCounts } from "../hooks/use-taxonomy-record-counts"; -import { useTaxonomyRun } from "../hooks/use-taxonomy-run"; +import { isTaxonomyRunGone, useTaxonomyRun } from "../hooks/use-taxonomy-run"; import { useTaxonomyState } from "../hooks/use-taxonomy-state"; import { useTriggerTaxonomyRun } from "../hooks/use-trigger-taxonomy-run"; import { MIN_OPEN_TEXT_RECORDS, computeGate } from "../lib/gate"; @@ -144,7 +144,18 @@ export const TopicsSubtopicsContainer = ({ } }, [runStatus, queryClient, workspaceId, scope]); - const isRunning = runningRunId !== null || triggerMutation.isPending; + /** + * A run the service no longer has is not in flight, whatever the cached run list still says. `runningRunId` + * is derived from the *state* query, and a gone run never yields a terminal status — so without this the + * hand-off effect above never fires, `isRunning` stays true, Generate stays disabled, and the retry below + * only refetches the same missing run. The page would be stuck until a reload. + * + * Dropping it from `isRunning` is enough: the stale entry stops being read as running, the warning below + * hides with it, and the next generate refreshes the list. Invalidating the state here instead would risk + * a refetch loop whenever the run list and the run endpoint disagree. + */ + const runIsGone = isTaxonomyRunGone(runQuery.error); + const isRunning = (runningRunId !== null && !runIsGone) || triggerMutation.isPending; // A failed run keeps generation enabled (that is the retry path); only block generation while the // taxonomy service itself is unreachable, since a new run can't succeed then anyway. const serviceUnavailable = fieldsUnavailable || stateUnavailable; @@ -294,7 +305,8 @@ export const TopicsSubtopicsContainer = ({ )} {/* A run is in progress but its status poll is failing. Polling self-recovers (see - * useTaxonomyRun), so surface a soft warning + manual retry rather than a hard error. */} + * useTaxonomyRun), so surface a soft warning + manual retry rather than a hard error. A *gone* run + * clears `isRunning` above, so this does not offer a retry that can only 404 again. */} {isRunning && runQuery.isError && ( {t("common.something_went_wrong_please_try_again")} diff --git a/apps/web/modules/ee/unify-feedback/topics-subtopics/hooks/use-taxonomy-run.test.ts b/apps/web/modules/ee/unify-feedback/topics-subtopics/hooks/use-taxonomy-run.test.ts index 47f066816853..e5935f7a9543 100644 --- a/apps/web/modules/ee/unify-feedback/topics-subtopics/hooks/use-taxonomy-run.test.ts +++ b/apps/web/modules/ee/unify-feedback/topics-subtopics/hooks/use-taxonomy-run.test.ts @@ -5,7 +5,8 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { act, renderHook, waitFor } from "@testing-library/react"; import { type ReactNode, createElement } from "react"; import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; -import { useTaxonomyRun } from "./use-taxonomy-run"; +import { V3ApiError } from "@/modules/api/lib/v3-client"; +import { isTaxonomyRunGone, useTaxonomyRun } from "./use-taxonomy-run"; function createWrapper(queryClient: QueryClient) { const Wrapper = ({ children }: { children: ReactNode }) => @@ -82,6 +83,53 @@ describe("useTaxonomyRun", () => { } }); + /** + * A run reaped by the orphan cleaner, or a stale id after a regenerate. Both were treated as "status + * unknown" before, so the interval kept firing at ~12 requests a minute while the UI showed "generating". + * + * Run under both retry policies on purpose: the suite's default is `retry: false`, but the provider that + * actually ships (`topics-subtopics/query-client-provider.tsx`) uses `retry: 1`, and React Query only + * commits the error once retries are exhausted — so only the second case proves the poll terminates in the + * configuration users get. + * + * The request count is asserted as an equality rather than an upper bound: "no more than N" would also + * hold at zero requests, so a hook that never fetched at all — a broken fixture, `enabled` off — would + * pass while proving nothing. + */ + test.each([ + { policy: "with retries disabled", retry: false as const, expected: 1 }, + { policy: "under the app's retry: 1 policy", retry: 1, expected: 2 }, + ])("stops polling once the run is gone (404), $policy", async ({ retry, expected }) => { + vi.useFakeTimers(); + try { + const fetchMock = vi.mocked(global.fetch); + fetchMock.mockResolvedValue( + new Response(JSON.stringify({ title: "Not Found", status: 404, code: "not_found" }), { + status: 404, + headers: { "Content-Type": "application/problem+json" }, + }) + ); + + renderHook(() => useTaxonomyRun({ workspaceId: "w", directoryId: "d", runId: "run-1" }), { + wrapper: createWrapper(new QueryClient({ defaultOptions: { queries: { retry } } })), + }); + + // Long enough for any retry to have been made (React Query's first retry delay is 1s). + await act(async () => { + await vi.advanceTimersByTimeAsync(2000); + }); + expect(fetchMock).toHaveBeenCalledTimes(expected); + + // Six intervals' worth of time later, still nothing: the poll is stopped, not merely slowed. + await act(async () => { + await vi.advanceTimersByTimeAsync(30000); + }); + expect(fetchMock).toHaveBeenCalledTimes(expected); + } finally { + vi.useRealTimers(); + } + }); + test("stays idle (no fetch) when runId is null", () => { const fetchMock = vi.mocked(global.fetch); @@ -93,3 +141,24 @@ describe("useTaxonomyRun", () => { expect(fetchMock).not.toHaveBeenCalled(); }); }); + +/** + * Exercised directly because both consumers must agree: the hook stops polling on it, and + * `topics-subtopics-container` stops treating the run as in-flight. The container is a `.tsx` and so is not + * unit-tested per AGENTS.md, which is precisely why the shared predicate carries the coverage. + */ +describe("isTaxonomyRunGone", () => { + test.each([ + { + label: "a 404 from this endpoint", + error: new V3ApiError({ status: 404, detail: "gone" }), + expected: true, + }, + { label: "a 403", error: new V3ApiError({ status: 403, detail: "no" }), expected: false }, + { label: "a 502", error: new V3ApiError({ status: 502, detail: "upstream" }), expected: false }, + { label: "a plain Error (network failure)", error: new Error("fetch failed"), expected: false }, + { label: "no error at all", error: null, expected: false }, + ])("$label -> $expected", ({ error, expected }) => { + expect(isTaxonomyRunGone(error)).toBe(expected); + }); +}); diff --git a/apps/web/modules/ee/unify-feedback/topics-subtopics/hooks/use-taxonomy-run.ts b/apps/web/modules/ee/unify-feedback/topics-subtopics/hooks/use-taxonomy-run.ts index cd1512663b7a..8fa433669a9d 100644 --- a/apps/web/modules/ee/unify-feedback/topics-subtopics/hooks/use-taxonomy-run.ts +++ b/apps/web/modules/ee/unify-feedback/topics-subtopics/hooks/use-taxonomy-run.ts @@ -2,14 +2,30 @@ import { useQuery } from "@tanstack/react-query"; import { InvalidInputError } from "@formbricks/types/errors"; +import { V3ApiError } from "@/modules/api/lib/v3-client"; import { getTaxonomyRun } from "../lib/api-client"; import { taxonomyKeys } from "../lib/query"; const RUN_POLL_INTERVAL_MS = 5000; -/** Poll a taxonomy run until it reaches a terminal state (succeeded/failed/canceled). Keeps polling - * while the status is unknown too — e.g. a poll that errored before any success — so a transient Hub - * blip self-recovers instead of leaving the caller stuck showing "generating" forever. */ +/** + * The run no longer exists — reaped by the orphan cleaner, or a stale id after a regenerate. The endpoint + * answers 404 for both "no such run" and "not this directory's run", so the status alone decides. + * + * Shared rather than inlined because two things have to agree about it: this hook stops polling, and the + * container stops treating the run as in-flight. If they disagreed, the UI would sit on "generating…" with + * the Generate button disabled and nothing left to poll — stuck until a page reload. + */ +export const isTaxonomyRunGone = (error: unknown): boolean => + error instanceof V3ApiError && error.status === 404; + +/** Poll a taxonomy run until it reaches a terminal state (succeeded/failed/canceled). Keeps polling while + * the status is unknown too — e.g. a poll that errored before any success — so a transient Hub blip + * self-recovers instead of leaving the caller stuck showing "generating" forever. + * + * A 404 is the exception, and the reason this stops rather than self-recovering: the run is gone — reaped, + * or a stale id after a regenerate — so no amount of polling brings it back, and "keep trying" means + * "generating…" forever at one request every five seconds. */ export const useTaxonomyRun = ({ workspaceId, directoryId, @@ -27,9 +43,14 @@ export const useTaxonomyRun = ({ }, staleTime: 0, refetchInterval: (query) => { - // Stop only once the run has genuinely finished. Any other state — pending/running, or unknown - // because the last poll errored (data undefined) — keeps the interval alive so polling resumes - // on its own when the Hub recovers. + // A run that no longer exists is terminal however many times we ask. + if (isTaxonomyRunGone(query.state.error)) { + return false; + } + + // Otherwise stop only once the run has genuinely finished. Any other state — pending/running, or + // unknown because the last poll errored (data undefined) — keeps the interval alive so polling + // resumes on its own when the Hub recovers. const status = query.state.data?.status; const isTerminal = status === "succeeded" || status === "failed" || status === "canceled"; return isTerminal ? false : RUN_POLL_INTERVAL_MS; diff --git a/docs/api-v3-reference/openapi.yml b/docs/api-v3-reference/openapi.yml index 6b0144db7416..d351d5d25dff 100644 --- a/docs/api-v3-reference/openapi.yml +++ b/docs/api-v3-reference/openapi.yml @@ -2404,6 +2404,7 @@ components: - not_authenticated - not_found - payload_too_large + - service_unavailable - too_many_requests - unprocessable_content - workflow_not_executable diff --git a/docs/api-v3-reference/src/components/schemas/Problem.yml b/docs/api-v3-reference/src/components/schemas/Problem.yml index 022442ef2dc0..ab49a5a6c44e 100644 --- a/docs/api-v3-reference/src/components/schemas/Problem.yml +++ b/docs/api-v3-reference/src/components/schemas/Problem.yml @@ -37,6 +37,7 @@ properties: - not_authenticated - not_found - payload_too_large + - service_unavailable - too_many_requests - unprocessable_content - workflow_not_executable diff --git a/packages/logger/src/logger.test.ts b/packages/logger/src/logger.test.ts index 506f709c76e4..5dc5c2a01c0d 100644 --- a/packages/logger/src/logger.test.ts +++ b/packages/logger/src/logger.test.ts @@ -165,22 +165,25 @@ describe("Logger", () => { expect(logger).toBeDefined(); }); - test("production OTEL logs use the absolute pino-opentelemetry transport target when enabled", async () => { + test("production OTEL logs use the runtime-root pino-opentelemetry transport target when enabled", async () => { process.env.NODE_ENV = "production"; process.env.NEXT_RUNTIME = "nodejs"; process.env.OTEL_EXPORTER_OTLP_ENDPOINT = "http://signoz-otel-collector.signoz:4318"; process.env.OTEL_LOGS_ENABLED = "1"; process.env.OTEL_SERVICE_NAME = "formbricks-web"; process.env.npm_package_version = "5.1.2"; + const cwdSpy = vi.spyOn(process, "cwd").mockReturnValue("/home/nextjs/apps/web"); const { logger } = await import("./logger"); const loggerConfig = vi.mocked(Pino).mock.calls.at(-1)?.[0] as Pino.LoggerOptions; + cwdSpy.mockRestore(); expect(loggerConfig.transport).toEqual( expect.objectContaining({ targets: expect.arrayContaining([ expect.objectContaining({ - target: `${process.cwd()}/node_modules/pino-opentelemetry-transport/lib/pino-opentelemetry-transport.js`, + target: + "/home/nextjs/node_modules/pino-opentelemetry-transport/lib/pino-opentelemetry-transport.js", options: expect.objectContaining({ loggerName: "formbricks-web", serviceVersion: "5.1.2", diff --git a/packages/logger/src/logger.ts b/packages/logger/src/logger.ts index 74e2a99eaa22..651d96b258f3 100644 --- a/packages/logger/src/logger.ts +++ b/packages/logger/src/logger.ts @@ -4,6 +4,9 @@ import { type TLogLevel, ZLogLevel } from "../types/logger"; const IS_PRODUCTION = !process.env.NODE_ENV || process.env.NODE_ENV === "production"; const IS_BUILD = process.env.NEXT_PHASE === "phase-production-build"; const PROCESS_GLOBAL_KEY = "process"; +const NEXT_STANDALONE_APP_DIR_PATTERN = /[/\\]apps[/\\]web$/; +const OTEL_TRANSPORT_PACKAGE_PATH = + "node_modules/pino-opentelemetry-transport/lib/pino-opentelemetry-transport.js"; interface TransportStream { on?: (event: "error", listener: (error: unknown) => void) => void; @@ -11,8 +14,10 @@ interface TransportStream { const getNodeProcess = (): typeof process => globalThis[PROCESS_GLOBAL_KEY]; -const getOtelTransportTarget = (): string => - `${getNodeProcess().cwd()}/node_modules/pino-opentelemetry-transport/lib/pino-opentelemetry-transport.js`; +const getOtelTransportTarget = (): string => { + const runtimeRoot = getNodeProcess().cwd().replace(NEXT_STANDALONE_APP_DIR_PATTERN, ""); + return `${runtimeRoot}/${OTEL_TRANSPORT_PACKAGE_PATH}`; +}; const getLogLevel = (): TLogLevel => { let logLevel: TLogLevel = "info";