[pull] main from formbricks:main

apps/web/app/api/client/[workspaceId]/responses/lib/response-error.test.ts

@@ -24,6 +24,30 @@ describe("handleClientResponseCreateError", () => {
     );
   });
 
+  // ENG-2251: adapter-pg parses the column list out of the Postgres error DETAIL; when that line is
+  // absent or unparseable it emits `constraint: undefined`, so no field check can match. A P2002
+  // must still be a conflict (409), never fall through to DatabaseError (500).
+  test("maps a P2002 without recoverable fields to UniqueConstraintError", () => {
+    const error = new Prisma.PrismaClientKnownRequestError("Unique constraint failed", {
+      code: "P2002",
+      clientVersion: "test",
+      meta: { driverAdapterError: { cause: { kind: "UniqueConstraintViolation" } } },
+    });
+    expect(() => handleClientResponseCreateError(error)).toThrow(
+      new UniqueConstraintError("Response already exists")
+    );
+  });
+
+  test("maps a P2002 without any meta to UniqueConstraintError", () => {
+    const error = new Prisma.PrismaClientKnownRequestError("Unique constraint failed", {
+      code: "P2002",
+      clientVersion: "test",
+    });
+    expect(() => handleClientResponseCreateError(error)).toThrow(
+      new UniqueConstraintError("Response already exists")
+    );
+  });
+
   test("maps any other known Prisma error to DatabaseError carrying its message", () => {
     const error = new Prisma.PrismaClientKnownRequestError("boom", { code: "P2025", clientVersion: "test" });
     expect(() => handleClientResponseCreateError(error)).toThrow(new DatabaseError("boom"));

apps/web/app/api/client/[workspaceId]/responses/lib/response-error.ts

@@ -19,13 +19,21 @@ export const isDisplayIdUniqueConstraintError = (error: PrismaClientKnownRequest
  */
 export const handleClientResponseCreateError = (error: unknown, displayId?: string | null): never => {
   if (isPrismaKnownRequestError(error)) {
+    // Captured before the P2002 guards: their negative branches narrow `error` to `never`.
+    const { message } = error;
     if (isDisplayIdUniqueConstraintError(error)) {
       throw new InvalidInputError(`Display ${displayId} is already linked to a response`);
     }
     if (isSingleUseIdUniqueConstraintError(error)) {
       throw new UniqueConstraintError("Response already submitted for this single-use link");
     }
-    throw new DatabaseError(error.message);
+    if (isUniqueConstraintError(error)) {
+      // The adapter recovers the column list by parsing the Postgres error DETAIL, which can be
+      // absent or unparseable (redacted detail, non-English lc_messages). A P2002 is still a
+      // duplicate either way, so it must stay a conflict — never fall through to a 500.
+      throw new UniqueConstraintError("Response already exists");
+    }
+    throw new DatabaseError(message);
   }
   throw error;
 };

apps/web/app/api/v1/client/[workspaceId]/responses/lib/response.test.ts

@@ -163,7 +163,7 @@ describe("createResponse", () => {
 
   test("should throw DatabaseError on Prisma known request error", async () => {
     const prismaError = new Prisma.PrismaClientKnownRequestError("Test Prisma Error", {
-      code: "P2002",
+      code: "P2025",
       clientVersion: "test",
     });
     vi.mocked(prisma.response.create).mockRejectedValue(prismaError);

apps/web/app/api/v2/client/[workspaceId]/responses/lib/response.test.ts

@@ -202,7 +202,9 @@ describe("createResponse V2", () => {
     ).rejects.toThrow(UniqueConstraintError);
   });
 
-  test("should throw DatabaseError on P2002 without singleUseId or displayId target", async () => {
+  // ENG-2251: a P2002 on this create can only be one of the response table's unique constraints,
+  // so an unmatched (or unrecoverable) target is still a duplicate — a 409, never a 500.
+  test("should throw UniqueConstraintError on P2002 without singleUseId or displayId target", async () => {
     const prismaError = new Prisma.PrismaClientKnownRequestError("Unique constraint failed", {
       code: "P2002",
       clientVersion: "test",
@@ -211,7 +213,7 @@ describe("createResponse V2", () => {
     vi.mocked(mockTx.response.create).mockRejectedValue(prismaError);
     await expect(
       createResponse(mockResponseInput, mockTx as unknown as Prisma.TransactionClient)
-    ).rejects.toThrow(DatabaseError);
+    ).rejects.toThrow(UniqueConstraintError);
   });
 
   test("should throw InvalidInputError on P2002 with displayId target (race condition)", async () => {

apps/web/app/api/v3/surveys/create.test.ts

@@ -5,7 +5,7 @@ import { getActionClasses } from "@/lib/actionClass/service";
 import { getOrganizationByWorkspaceId } from "@/lib/organization/service";
 import { createSurvey, getSurvey } from "@/lib/survey/service";
 import { getExternalUrlsPermission } from "@/modules/survey/lib/permission";
-import { V3SurveyCreatePermissionError, createV3Survey } from "./create";
+import { V3SurveyCreatePermissionError, V3SurveyInputValidationError, createV3Survey } from "./create";
 import { V3SurveyReferenceValidationError } from "./reference-validation";
 import { ZV3CreateSurveyBody } from "./schemas";
 import { resolveV3ContactsEntitlement } from "./targeting";
@@ -306,6 +306,171 @@ describe("createV3Survey", () => {
     expect(createSurvey).not.toHaveBeenCalled();
   });
 
+  // ENG-2587. Driven through the real `ZSurveyCreateInput`/`surveyRefinement` rather than a
+  // synthetic schema, so it also fails if the refinement's issue path moves, or if the CTA branch
+  // stops firing (it is gated on `buttonExternal`). `createSurvey` is mocked, so no DB is involved.
+  test("rejects a CTA buttonUrl scheme the request schema admits but the write schema does not", async () => {
+    vi.mocked(getExternalUrlsPermission).mockResolvedValue(true);
+    const body = ZV3CreateSurveyBody.parse({
+      ...rawCreateBody,
+      blocks: [
+        {
+          ...rawCreateBody.blocks[0],
+          elements: [
+            {
+              id: "tel_cta",
+              type: "cta",
+              headline: { "en-US": "Call us", "de-DE": "Ruf uns an" },
+              required: false,
+              buttonExternal: true,
+              buttonUrl: "tel:+123456789",
+              ctaButtonLabel: { "en-US": "Call", "de-DE": "Anrufen" },
+            },
+          ],
+        },
+      ],
+    });
+
+    const error = await createV3Survey(body, null, "req_eng2587").catch((err: unknown) => err);
+
+    expect(error).toBeInstanceOf(V3SurveyInputValidationError);
+    expect((error as V3SurveyInputValidationError).invalidParams).toEqual(
+      expect.arrayContaining([expect.objectContaining({ name: "blocks.0.elements.0.buttonUrl" })])
+    );
+    // The whole point of pre-write validation: nothing was written.
+    expect(createSurvey).not.toHaveBeenCalled();
+  });
+
+  // Found in review on #8991: a second case the request schema admits and the write schema rejects, so
+  // it used to be a 500 too. Same path as the CTA case above, different refinement — worth pinning
+  // because the description claims this case is covered, and a live check alone would not keep it so.
+  // The other case the request schema admits and the write schema rejects: `isSafeLinkUrl` allows
+  // `http:`, `safeUrlRefinement` allows only `https://` and `http://localhost`. Per review this is the
+  // most common of the three in practice, so it gets its own assertion rather than riding on `tel:`.
+  test("rejects a plain http CTA buttonUrl the request schema admits", async () => {
+    vi.mocked(getExternalUrlsPermission).mockResolvedValue(true);
+    const body = ZV3CreateSurveyBody.parse({
+      ...rawCreateBody,
+      blocks: [
+        {
+          ...rawCreateBody.blocks[0],
+          elements: [
+            {
+              id: "http_cta",
+              type: "cta",
+              headline: { "en-US": "Docs", "de-DE": "Doku" },
+              required: false,
+              buttonExternal: true,
+              buttonUrl: "http://example.com",
+              ctaButtonLabel: { "en-US": "Open", "de-DE": "Oeffnen" },
+            },
+          ],
+        },
+      ],
+    });
+
+    const error = await createV3Survey(body, null, "req_http_cta").catch((err: unknown) => err);
+
+    expect(error).toBeInstanceOf(V3SurveyInputValidationError);
+    expect((error as V3SurveyInputValidationError).invalidParams).toEqual(
+      expect.arrayContaining([expect.objectContaining({ name: "blocks.0.elements.0.buttonUrl" })])
+    );
+    expect(createSurvey).not.toHaveBeenCalled();
+  });
+
+  // A blank headline is rejected either side of the new parse, and which side depends on the language
+  // set rather than on the blankness. What `prepareV3SurveyCreate` catches here is the *absent* `de-DE`
+  // key, since `rawCreateBody` declares `de-DE` while this headline carries only `en-US`.
+  test("routes a blank headline with a missing declared locale to the earlier preparation guard", async () => {
+    const body = ZV3CreateSurveyBody.parse({
+      ...rawCreateBody,
+      blocks: [
+        {
+          ...rawCreateBody.blocks[0],
+          elements: [
+            {
+              id: "blank_headline_missing_locale",
+              type: "openText",
+              headline: { "en-US": "   " },
+              required: true,
+            },
+          ],
+        },
+      ],
+    });
+
+    const error = await createV3Survey(body, null, "req_blank_missing_locale").catch((err: unknown) => err);
+
+    expect(error).toBeInstanceOf(V3SurveyReferenceValidationError);
+    expect(error).not.toBeInstanceOf(V3SurveyInputValidationError);
+    expect(createSurvey).not.toHaveBeenCalled();
+  });
+
+  // ...and once every declared locale key is present, the blank headline reaches the new pre-write parse
+  // instead. These two shapes were 500s on `main`, so they are part of what this PR fixes. Found in
+  // review after an earlier version of this suite pinned only the shape above and over-generalised
+  // from it.
+  test.each([
+    ["no languages declared", undefined, { "en-US": "   " }, { "en-US": "Product Feedback" }],
+    [
+      "every declared locale present and blank",
+      [{ code: "de-DE", enabled: true }],
+      { "en-US": "   ", "de-DE": "   " },
+      { "en-US": "Product Feedback", "de-DE": "Produktfeedback" },
+    ],
+  ])(
+    "rejects a blank headline with %s through the new pre-write parse",
+    async (_label, languages, headline, title) => {
+      const body = ZV3CreateSurveyBody.parse({
+        ...rawCreateBody,
+        languages,
+        // The metadata title has to carry the same declared locales, or preparation rejects *that*
+        // missing key first and the element never reaches the parse under test.
+        metadata: { cx_operation: "enterprise_onboarding", title },
+        blocks: [
+          {
+            ...rawCreateBody.blocks[0],
+            elements: [{ id: "blank_headline", type: "openText", headline, required: true }],
+          },
+        ],
+      });
+
+      const error = await createV3Survey(body, null, "req_blank_headline").catch((err: unknown) => err);
+
+      expect(error).toBeInstanceOf(V3SurveyInputValidationError);
+      expect((error as V3SurveyInputValidationError).invalidParams).toEqual(
+        expect.arrayContaining([expect.objectContaining({ name: "blocks.0.elements.0.headline" })])
+      );
+      expect(createSurvey).not.toHaveBeenCalled();
+    }
+  );
+
+  test("accepts an https CTA buttonUrl through the same path", async () => {
+    vi.mocked(getExternalUrlsPermission).mockResolvedValue(true);
+    const body = ZV3CreateSurveyBody.parse({
+      ...rawCreateBody,
+      blocks: [
+        {
+          ...rawCreateBody.blocks[0],
+          elements: [
+            {
+              id: "https_cta",
+              type: "cta",
+              headline: { "en-US": "Continue", "de-DE": "Weiter" },
+              required: false,
+              buttonExternal: true,
+              buttonUrl: "https://example.com",
+              ctaButtonLabel: { "en-US": "Open", "de-DE": "\u00d6ffnen" },
+            },
+          ],
+        },
+      ],
+    });
+
+    await expect(createV3Survey(body, null, "req_eng2587_ok")).resolves.toBeDefined();
+    expect(createSurvey).toHaveBeenCalled();
+  });
+
   test("rejects external CTA buttons for API-key creates without external URL permission", async () => {
     vi.mocked(getExternalUrlsPermission).mockResolvedValue(false);
     const body = ZV3CreateSurveyBody.parse({

apps/web/app/api/v3/surveys/create.ts

@@ -1,5 +1,7 @@
 import "server-only";
+import { ZSurveyCreateInput } from "@formbricks/types/surveys/types";
 import type { TSurvey, TSurveyCreateInput } from "@formbricks/types/surveys/types";
+import type { InvalidParam } from "@/app/api/v3/lib/response";
 import type { TV3Authentication } from "@/app/api/v3/lib/types";
 import { getActionClasses } from "@/lib/actionClass/service";
 import { getOrganizationByWorkspaceId } from "@/lib/organization/service";
@@ -10,7 +12,7 @@ import { v3DistributionToScalars } from "./distribution";
 import { type TV3SurveyLanguageRequest, ensureV3WorkspaceLanguages } from "./languages";
 import { prepareV3SurveyCreate } from "./prepare";
 import { V3SurveyReferenceValidationError } from "./reference-validation";
-import type { TV3CreateSurveyBody } from "./schemas";
+import { type TV3CreateSurveyBody, formatV3ZodInvalidParams } from "./schemas";
 import {
   V3_CONTACTS_NOT_ENABLED_MESSAGE,
   assertV3SurveyTargetingFilterReferences,
@@ -31,6 +33,29 @@ export class V3SurveyCreatePermissionError extends Error {
   }
 }
 
+/**
+ * The create document passed this route's request schema but failed the stricter schema the survey
+ * service applies on write (`ZSurveyCreateInput` + `surveyRefinement`) — e.g. a CTA `buttonUrl`
+ * scheme the element schema allows but the refinement rejects.
+ *
+ * This is raised from an explicit pre-write parse rather than by catching `ValidationError` in the
+ * response mapper, and the distinction matters: `createSurvey` runs `validateInputs` again itself,
+ * and it also calls out to code *after* its transaction commits (`subscribeOrganizationMembers-
+ * ToSurveyResponses` → `updateUser`, which re-validates the user's stored `notificationSettings`
+ * JSON). A `ValidationError` from there means the survey row already exists, so answering it with a
+ * 4xx would tell the caller nothing was written and invite a duplicate retry. Anything that is not
+ * this error keeps its 500.
+ */
+export class V3SurveyInputValidationError extends Error {
+  invalidParams: InvalidParam[];
+
+  constructor(invalidParams: InvalidParam[]) {
+    super("Survey document failed validation");
+    this.name = "V3SurveyInputValidationError";
+    this.invalidParams = invalidParams;
+  }
+}
+
 function getCreatedBy(authentication: TV3Authentication): string | null {
   if (authentication && "user" in authentication && authentication.user?.id) {
     return authentication.user.id;
@@ -184,6 +209,16 @@ export async function executeV3SurveyCreate(params: {
     ...surveyCreateInputOverrides,
   };
 
+  // Run the survey service's own write schema here, before any DB work, so a document this route's
+  // request schema admits but `surveyRefinement` rejects (a CTA `buttonUrl` of `tel:…` being the
+  // known case) fails with a typed error the route can answer 422. `createSurvey` validates this
+  // same input again; catching its `ValidationError` instead would be wrong, because it also throws
+  // one from post-commit work where the survey row already exists. See V3SurveyInputValidationError.
+  const parsedCreateInput = ZSurveyCreateInput.safeParse(surveyCreateInput);
+  if (!parsedCreateInput.success) {
+    throw new V3SurveyInputValidationError(formatV3ZodInvalidParams(parsedCreateInput.error, "body"));
+  }
+
   // App targeting filters are created atomically with the survey's private segment inside
   // `createSurvey` (a single transaction), so a failed targeting write can't leave a partial survey.
   const privateSegmentFilters = input.type === "app" ? (input.targeting?.filters ?? []) : [];

apps/web/app/api/v3/surveys/lib/operations.test.ts

@@ -1,13 +1,13 @@
 import { beforeEach, describe, expect, test, vi } from "vitest";
-import { DatabaseError, ResourceNotFoundError } from "@formbricks/types/errors";
+import { DatabaseError, ResourceNotFoundError, ValidationError } from "@formbricks/types/errors";
 import { requireV3WorkspaceAccess } from "@/app/api/v3/lib/auth";
 import { problemForbidden } from "@/app/api/v3/lib/response";
 import { capturePostHogEvent } from "@/lib/posthog";
 import { archiveSurvey, deleteSurvey, restoreSurvey } from "@/modules/survey/lib/surveys";
 import { getSurveyCount, hasArchivedSurveys } from "@/modules/survey/list/lib/survey";
 import { getSurveyListPage } from "@/modules/survey/list/lib/survey-page";
 import { getAuthorizedV3Survey } from "../authorization";
-import { V3SurveyCreatePermissionError, createV3Survey } from "../create";
+import { V3SurveyCreatePermissionError, V3SurveyInputValidationError, createV3Survey } from "../create";
 import { parseV3SurveysListQuery } from "../parse-v3-surveys-list-query";
 import { patchV3Survey } from "../patch";
 import { prepareV3SurveyCreateInput, prepareV3SurveyPatchInput } from "../prepare";
@@ -483,6 +483,48 @@ describe("createV3SurveyResponse", () => {
       ).status
     ).toBe(500);
   });
+
+  // ENG-2587. The document passes `ZV3CreateSurveyBody` but fails the survey service's stricter
+  // write schema; `executeV3SurveyCreate` catches that with a pre-write parse and throws this typed
+  // error. Before the fix there was no branch for it and it landed on the generic 500.
+  test("maps V3SurveyInputValidationError to 422 naming the offending path", async () => {
+    vi.mocked(createV3Survey).mockRejectedValueOnce(
+      new V3SurveyInputValidationError([{ name: "blocks.0.elements.0.buttonUrl", reason: "Invalid url" }])
+    );
+
+    const response = await createV3SurveyResponse({
+      body: parsedCreateBody,
+      authentication,
+      requestId,
+      instance,
+    });
+
+    expect(response.status).toBe(422);
+    expect(await readJson(response)).toMatchObject({
+      invalid_params: [expect.objectContaining({ name: "blocks.0.elements.0.buttonUrl" })],
+    });
+  });
+
+  // The reason the branch above is keyed on the typed error and not on `ValidationError`:
+  // `createSurvey` also throws `ValidationError` from work that runs *after* its transaction
+  // commits (`subscribeOrganizationMembersToSurveyResponses` -> `updateUser` re-validates the
+  // user's stored `notificationSettings` JSON). The survey row exists by then, so answering 4xx
+  // would tell the caller nothing was written and invite a duplicate retry — and it would drop a
+  // genuine server fault out of 5xx alerting. Those must keep the 500.
+  test("keeps a bare ValidationError on the 500 path, so post-commit faults are not reported as 4xx", async () => {
+    vi.mocked(createV3Survey).mockRejectedValueOnce(
+      new ValidationError("Validation failed: notificationSettings.alertExpected boolean")
+    );
+
+    const response = await createV3SurveyResponse({
+      body: parsedCreateBody,
+      authentication,
+      requestId,
+      instance,
+    });
+
+    expect(response.status).toBe(500);
+  });
 });
 
 describe("getV3Survey", () => {