Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions .github/workflows/e2e.yml
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,10 @@ jobs:

- name: Apply Prisma Migrations
run: |
# pnpm prisma migrate deploy
pnpm db:migrate:dev
# @formbricks/database is already built by the "Build App" step above,
# so run the migration runner directly instead of db:migrate:dev
# (which would rebuild + re-generate the package unnecessarily).
pnpm --filter=@formbricks/database db:migrate:ci

- name: Run Rate Limiter Load Tests
run: |
Expand Down
5 changes: 4 additions & 1 deletion .github/workflows/integration-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,10 @@ jobs:

- name: Apply schema + data migrations to the test database
if: steps.harness.outputs.present == 'true'
run: pnpm db:migrate:dev
# @formbricks/database is already built by the "Build workspace package
# dependencies" step above, so run the migration runner directly instead
# of db:migrate:dev (which would rebuild + re-generate the package).
run: pnpm --filter=@formbricks/database db:migrate:ci
shell: bash

# Shared with apps/web/integration/global-setup.ts (the local harness applies the same file) so
Expand Down
29 changes: 17 additions & 12 deletions apps/web/app/api/v1/auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -278,43 +278,48 @@ describe("authenticateRequest", () => {

describe("handleErrorResponse", () => {
test("returns 401 notAuthenticated for 'NotAuthenticated' message", async () => {
const response = handleErrorResponse(new Error("NotAuthenticated"));
const { response } = handleErrorResponse(new Error("NotAuthenticated"));
expect(response.status).toBe(401);
const body = await response.json();
expect(body.code).toBe("not_authenticated");
});

test("returns 401 unauthorized for 'Unauthorized' message", async () => {
const response = handleErrorResponse(new Error("Unauthorized"));
const { response } = handleErrorResponse(new Error("Unauthorized"));
expect(response.status).toBe(401);
const body = await response.json();
expect(body.code).toBe("unauthorized");
});

test("returns 409 conflict for UniqueConstraintError", async () => {
const response = handleErrorResponse(new UniqueConstraintError("Action with name foo already exists"));
const { response } = handleErrorResponse(new UniqueConstraintError("Action with name foo already exists"));
expect(response.status).toBe(409);
const body = await response.json();
expect(body.code).toBe("conflict");
expect(body.message).toBe("Action with name foo already exists");
});

test("returns 400 badRequest for DatabaseError", async () => {
const response = handleErrorResponse(new DatabaseError("db boom"));
expect(response.status).toBe(400);
test("returns a generic 500 for DatabaseError, threads the real error, and doesn't leak the message", async () => {
const error = new DatabaseError("db boom");
const { response, error: reportedError } = handleErrorResponse(error);
// The real error must reach the wrapper's reportApiError (not a synthetic one), so 5xx errors
// from routes still on handleErrorResponse keep their Sentry signal.
expect(reportedError).toBe(error);
expect(response.status).toBe(500);
const body = await response.json();
expect(body.message).toBe("db boom");
expect(body.message).toBe("Something went wrong. Please try again.");
expect(JSON.stringify(body)).not.toContain("db boom");
});

test("returns 400 badRequest for InvalidInputError", async () => {
const response = handleErrorResponse(new InvalidInputError("bad input"));
const { response } = handleErrorResponse(new InvalidInputError("bad input"));
expect(response.status).toBe(400);
const body = await response.json();
expect(body.message).toBe("bad input");
});

test("returns 404 notFound for ResourceNotFoundError", async () => {
const response = handleErrorResponse(new ResourceNotFoundError("Survey", "id-1"));
const { response } = handleErrorResponse(new ResourceNotFoundError("Survey", "id-1"));
expect(response.status).toBe(404);
const body = await response.json();
expect(body).toEqual({
Expand All @@ -327,10 +332,10 @@ describe("handleErrorResponse", () => {
});
});

test("returns 500 internalServerError for unknown errors", async () => {
const response = handleErrorResponse(new Error("something else"));
test("returns a generic 500 for unknown errors", async () => {
const { response } = handleErrorResponse(new Error("something else"));
expect(response.status).toBe(500);
const body = await response.json();
expect(body.message).toBe("Some error occurred");
expect(body.message).toBe("Something went wrong. Please try again.");
});
});
30 changes: 10 additions & 20 deletions apps/web/app/api/v1/auth.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
import { NextRequest } from "next/server";
import { TAuthenticationApiKey } from "@formbricks/types/auth";
import {
DatabaseError,
InvalidInputError,
ResourceNotFoundError,
UniqueConstraintError,
} from "@formbricks/types/errors";
import { type ApiErrorResult, handleApiError } from "@/app/lib/api/handle-api-error";
import { responses } from "@/app/lib/api/response";
import {
type AuthenticateApiKeyOptions,
Expand All @@ -19,22 +14,17 @@ export const authenticateRequest = async (
return await authenticateApiKeyFromHeaders(request.headers, options);
};

export const handleErrorResponse = (error: any): Response => {
switch (error.message) {
export const handleErrorResponse = (error: unknown): ApiErrorResult => {
const message = error instanceof Error ? error.message : undefined;
switch (message) {
case "NotAuthenticated":
return responses.notAuthenticatedResponse();
return { response: responses.notAuthenticatedResponse() };
case "Unauthorized":
return responses.unauthorizedResponse();
return { response: responses.unauthorizedResponse() };
default:
if (error instanceof UniqueConstraintError) {
return responses.conflictResponse(error.message);
}
if (error instanceof ResourceNotFoundError) {
return responses.notFoundResponse(error.resourceType, error.resourceId);
}
if (error instanceof DatabaseError || error instanceof InvalidInputError) {
return responses.badRequestResponse(error.message);
}
return responses.internalServerErrorResponse("Some error occurred");
// Delegate to the shared boundary and return its full { response, error } result — not just
// `.response` — so the wrapper's reportApiError receives the real 5xx error (e.g. a
// DatabaseError) instead of a synthetic one. Expected 4xx errors keep their status/message.
return handleApiError(error);
}
};
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { logger } from "@formbricks/logger";
import { ResourceNotFoundError } from "@formbricks/types/errors";
import { handleApiError } from "@/app/lib/api/handle-api-error";
import { responses } from "@/app/lib/api/response";
import { THandlerParams, withV1ApiWrapper } from "@/app/lib/api/with-api-logging";
import { resolveClientApiIds } from "@/lib/utils/resolve-client-id";
Expand All @@ -11,7 +11,6 @@ export const OPTIONS = async (): Promise<Response> => {

export const GET = withV1ApiWrapper({
handler: async ({
req,
props,
}: THandlerParams<{ params: Promise<{ workspaceId: string; displayId: string }> }>) => {
const params = await props.params;
Expand All @@ -36,14 +35,7 @@ export const GET = withV1ApiWrapper({
response: responses.notFoundResponse("Display", params.displayId, true),
};
}

logger.error(
{ error, url: req.url, workspaceId, displayId: params.displayId },
"Error in GET /api/v1/client/[workspaceId]/displays/[displayId]/response"
);
return {
response: responses.internalServerErrorResponse("Something went wrong. Please try again."),
};
return handleApiError(error, { cors: true });
}
},
});
13 changes: 5 additions & 8 deletions apps/web/app/api/v1/client/[workspaceId]/displays/route.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { logger } from "@formbricks/logger";
import { ZDisplayCreateInput } from "@formbricks/types/displays";
import { InvalidInputError, ResourceNotFoundError } from "@formbricks/types/errors";
import { handleApiError } from "@/app/lib/api/handle-api-error";
import { RequestBodyTooLargeError, parseJsonBodyWithLimit } from "@/app/lib/api/request-body";
import { responses } from "@/app/lib/api/response";
import { transformErrorToDetails } from "@/app/lib/api/validator";
Expand Down Expand Up @@ -89,20 +89,17 @@ export const POST = withV1ApiWrapper({
} catch (error) {
if (error instanceof ResourceNotFoundError) {
return {
response: responses.notFoundResponse("Survey", inputValidation.data.surveyId),
response: responses.notFoundResponse("Survey", inputValidation.data.surveyId, true),
};
} else if (error instanceof InvalidInputError) {
}
if (error instanceof InvalidInputError) {
return {
response: responses.forbiddenResponse(error.message, true, {
surveyId: inputValidation.data.surveyId,
}),
};
} else {
logger.error({ error, url: req.url }, "Error in POST /api/v1/client/[workspaceId]/displays");
return {
response: responses.internalServerErrorResponse("Something went wrong. Please try again."),
};
}
return handleApiError(error, { cors: true });
}
},
});
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ const mocks = vi.hoisted(() => ({
getResponse: vi.fn(),
getSurvey: vi.fn(),
getValidatedResponseUpdateInput: vi.fn(),
loggerError: vi.fn(),
resolveClientApiIds: vi.fn(),
sendToPipeline: vi.fn(),
updateResponseWithQuotaEvaluation: vi.fn(),
Expand All @@ -23,12 +22,6 @@ const mocks = vi.hoisted(() => ({
verifyLinkSurveyPinToken: vi.fn(),
}));

vi.mock("@formbricks/logger", () => ({
logger: {
error: mocks.loggerError,
},
}));

vi.mock("@/app/lib/pipelines", () => ({
sendToPipeline: mocks.sendToPipeline,
}));
Expand Down Expand Up @@ -220,26 +213,20 @@ describe("putResponseHandler", () => {
});
});

test("maps database lookup errors to a reported internal server error", async () => {
test("maps database lookup errors to an internal server error without leaking the message", async () => {
const error = new DatabaseError("Lookup failed");
mocks.getResponse.mockRejectedValue(error);

const result = await putResponseHandler(createHandlerParams());

// The real error is threaded back so the wrapper logs/reports it; the client sees a generic message.
expect(result.error).toBe(error);
expect(result.response.status).toBe(500);
await expect(result.response.json()).resolves.toEqual({
code: "internal_server_error",
message: "Lookup failed",
message: "Something went wrong. Please try again.",
details: {},
});
expect(mocks.loggerError).toHaveBeenCalledWith(
{
error,
url: createRequest().url,
},
"Error in PUT /api/v1/client/[workspaceId]/responses/[responseId]"
);
});

test("maps unknown lookup failures to a generic internal server error", async () => {
Expand All @@ -252,7 +239,7 @@ describe("putResponseHandler", () => {
expect(result.response.status).toBe(500);
await expect(result.response.json()).resolves.toEqual({
code: "internal_server_error",
message: "Unknown error occurred",
message: "Something went wrong. Please try again.",
details: {},
});
});
Expand Down Expand Up @@ -470,7 +457,7 @@ describe("putResponseHandler", () => {
});
});

test("returns a reported internal server error for database update failures", async () => {
test("returns an internal server error for database update failures without leaking the message", async () => {
const error = new DatabaseError("Update failed");
mocks.updateResponseWithQuotaEvaluation.mockRejectedValue(error);

Expand All @@ -480,16 +467,9 @@ describe("putResponseHandler", () => {
expect(result.response.status).toBe(500);
await expect(result.response.json()).resolves.toEqual({
code: "internal_server_error",
message: "Update failed",
message: "Something went wrong. Please try again.",
details: {},
});
expect(mocks.loggerError).toHaveBeenCalledWith(
{
error,
url: createRequest().url,
},
"Error in PUT /api/v1/client/[workspaceId]/responses/[responseId]"
);
});

test("returns a generic internal server error for unexpected update failures", async () => {
Expand All @@ -502,16 +482,9 @@ describe("putResponseHandler", () => {
expect(result.response.status).toBe(500);
await expect(result.response.json()).resolves.toEqual({
code: "internal_server_error",
message: "Something went wrong",
message: "Something went wrong. Please try again.",
details: {},
});
expect(mocks.loggerError).toHaveBeenCalledWith(
{
error,
url: createRequest().url,
},
"Error in PUT /api/v1/client/[workspaceId]/responses/[responseId]"
);
});

test("returns a success payload and emits a responseUpdated pipeline event", async () => {
Expand Down
Loading
Loading