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
2 changes: 1 addition & 1 deletion packages/backend/src/billing/billing.constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,5 +53,5 @@ export function getStripePriceId(): string {
if (!priceId) {
throw new Error("STRIPE_PRICE_ID is not configured");
}
return priceId;
return priceId.trim();
}
33 changes: 33 additions & 0 deletions packages/backend/src/billing/billing.errors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import Stripe from "stripe";
import { Status } from "@core/errors/status.codes";

export class BillingHttpError extends Error {
readonly status: number;
readonly clientMessage: string;

constructor(status: number, clientMessage: string, cause?: unknown) {
const detail = cause instanceof Error ? cause.message : clientMessage;
super(detail);
this.name = "BillingHttpError";
this.status = status;
this.clientMessage = clientMessage;
if (cause !== undefined) {
this.cause = cause;
}
}
}

const BILLING_CLIENT_MESSAGE =
"Couldn't start billing. Please try again in a moment.";

export function wrapStripeFailure(e: unknown): never {
if (e instanceof Stripe.errors.StripeError) {
const stripeStatus = e.statusCode ?? 0;
const status =
stripeStatus === 429 || stripeStatus >= 500
? Status.BAD_GATEWAY
: Status.BAD_REQUEST;
throw new BillingHttpError(status, BILLING_CLIENT_MESSAGE, e);
}
throw e;
}
Original file line number Diff line number Diff line change
Expand Up @@ -81,4 +81,29 @@ describe("BillingController", () => {
);
expect(json).toHaveBeenCalledWith({ error: "Internal server error" });
});

it("maps BillingHttpError to its status and client message", async () => {
const { BillingHttpError } = await import(
"@backend/billing/billing.errors"
);
spyOn(billingService, "getStatus").mockRejectedValue(
new BillingHttpError(
502,
"Couldn't start billing. Please try again in a moment.",
),
);

const { res, json } = jsonRes();
await billingController.getStatus(
sessionReq("507f1f77bcf86cd799439011"),
res,
);

expect((res.status as ReturnType<typeof mock>).mock.calls[0]?.[0]).toBe(
502,
);
expect(json).toHaveBeenCalledWith({
error: "Couldn't start billing. Please try again in a moment.",
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,18 @@ import {
type BillingStatusResponse,
} from "@core/types/billing.types";
import { zObjectId } from "@core/types/type.utils";
import { BillingHttpError } from "@backend/billing/billing.errors";
import billingService from "@backend/billing/services/billing.service";
import stripeService from "@backend/billing/services/stripe.service";

const logger = Logger("app:billing");

const sendBillingError = (res: Response, e: unknown) => {
if (e instanceof BillingHttpError) {
logger.error(e.message, e.cause ?? e);
res.status(e.status).json({ error: e.clientMessage });
return;
}
const message = e instanceof Error ? e.message : "Unexpected error";
if (message === "User not found") {
res.status(Status.NOT_FOUND).json({ error: "User not found" });
Expand Down
46 changes: 45 additions & 1 deletion packages/backend/src/billing/services/stripe.service.db.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type Stripe from "stripe";
import Stripe from "stripe";
import {
cleanupCollections,
cleanupTestDb,
Expand Down Expand Up @@ -72,9 +72,16 @@ describe("StripeService", () => {
expect(sessionArgs.client_reference_id).toBe(userId.toString());
expect(sessionArgs.subscription_data.trial_period_days).toBe(7);
expect(sessionArgs.line_items[0]?.price).toBe("price_test");
expect(
(sessionArgs as { payment_method_collection?: string })
.payment_method_collection,
).toBe("always");
expect(
(sessionArgs as { payment_method_types?: unknown }).payment_method_types,
).toBeUndefined();
expect(sessionsCreate.mock.calls[0]?.[1]).toEqual({
idempotencyKey: `compass-checkout-v2-${userId.toString()}`,
});

const stored = await mongoService.user.findOne({ _id: userId });
expect(stored?.billing?.stripeCustomerId).toBe("cus_1");
Expand Down Expand Up @@ -248,4 +255,41 @@ describe("StripeService", () => {
return_url: "http://localhost:9080",
});
});

it("maps a Stripe invalid-request error to BillingHttpError", async () => {
using _env = mockEnv(stripeConfigured);
const userId = mongoService.objectId();
await mongoService.user.insertOne({
_id: userId,
email: "pay@example.com",
name: "Pay User",
firstName: "Pay",
lastName: "User",
locale: "en",
});

const stripeError = new Stripe.errors.StripeInvalidRequestError({
message: "No such price: 'price_test'",
type: "invalid_request_error",
statusCode: 400,
});
setStripeClientForTests({
customers: {
create: mock(() => Promise.resolve({ id: "cus_1" })),
},
checkout: {
sessions: {
create: mock(() => Promise.reject(stripeError)),
},
},
} as unknown as Stripe);

await expect(
stripeService.createCheckoutSession(userId.toString()),
).rejects.toMatchObject({
name: "BillingHttpError",
status: 400,
clientMessage: "Couldn't start billing. Please try again in a moment.",
});
});
});
74 changes: 47 additions & 27 deletions packages/backend/src/billing/services/stripe.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,15 @@ import {
BILLING_PLAN,
getStripePriceId,
} from "@backend/billing/billing.constants";
import { wrapStripeFailure } from "@backend/billing/billing.errors";
import { getStripeClient } from "@backend/billing/services/stripe.client";
import { CONFIG } from "@backend/common/constants/config.constants";
import { isStripeConfigured } from "@backend/common/constants/config.util";
import mongoService from "@backend/common/services/mongo.service";

/** Bump when Checkout Session create params change so Stripe does not replay a failed create. */
const CHECKOUT_IDEMPOTENCY_PREFIX = "compass-checkout-v2-";

const checkoutReturnUrl = (outcome: "success" | "cancel"): string => {
const url = new URL(CONFIG.FRONTEND_URL);
url.searchParams.set("checkout", outcome);
Expand Down Expand Up @@ -45,37 +49,51 @@ class StripeService {
const grantTrial = !user.billing?.stripeSubscriptionId;

if (!customerId) {
const customer = await stripe.customers.create(
{
email: user.email,
metadata: { compassUserId: userId },
},
{ idempotencyKey: `compass-customer-${userId}` },
);
const customer = await stripe.customers
.create(
{
email: user.email,
metadata: { compassUserId: userId },
},
{ idempotencyKey: `compass-customer-${userId}` },
)
.catch(wrapStripeFailure);
customerId = customer.id;
await mongoService.user.updateOne(
{ _id },
{ $set: { "billing.stripeCustomerId": customerId } },
);
}

const session = await stripe.checkout.sessions.create(
{
mode: "subscription",
customer: customerId,
client_reference_id: userId,
line_items: [{ price: getStripePriceId(), quantity: 1 }],
subscription_data: {
...(grantTrial
? { trial_period_days: BILLING_PLAN.TRIAL_LENGTH_DAYS }
: {}),
metadata: { compassUserId: userId },
const session = await stripe.checkout.sessions
.create(
{
mode: "subscription",
customer: customerId,
client_reference_id: userId,
line_items: [{ price: getStripePriceId(), quantity: 1 }],
// Card is required to start the trial. Default is `always`, but pin
// it so a Dashboard setting cannot silently skip collection.
payment_method_collection: "always",
subscription_data: {
...(grantTrial
? {
trial_period_days: BILLING_PLAN.TRIAL_LENGTH_DAYS,
trial_settings: {
end_behavior: { missing_payment_method: "cancel" },
},
}
: {}),
metadata: { compassUserId: userId },
},
success_url: checkoutReturnUrl("success"),
cancel_url: checkoutReturnUrl("cancel"),
},
success_url: checkoutReturnUrl("success"),
cancel_url: checkoutReturnUrl("cancel"),
},
grantTrial ? { idempotencyKey: `compass-checkout-${userId}` } : undefined,
);
grantTrial
? { idempotencyKey: `${CHECKOUT_IDEMPOTENCY_PREFIX}${userId}` }
: undefined,
)
.catch(wrapStripeFailure);

if (!session.url) {
throw new Error("Stripe Checkout did not return a URL");
Expand Down Expand Up @@ -103,10 +121,12 @@ class StripeService {
}

const stripe = getStripeClient();
const session = await stripe.billingPortal.sessions.create({
customer: customerId,
return_url: CONFIG.FRONTEND_URL,
});
const session = await stripe.billingPortal.sessions
.create({
customer: customerId,
return_url: CONFIG.FRONTEND_URL,
})
.catch(wrapStripeFailure);

return { url: session.url };
};
Expand Down
22 changes: 22 additions & 0 deletions packages/backend/src/common/constants/config.constants.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,28 @@ describe("config.constants", () => {
expect(isStripeConfigured(env)).toBe(true);
});

it("trims Stripe values from env and compass.yaml", () => {
const fromEnv = parseConfigFromEnv({
...validEnv,
STRIPE_SECRET_KEY: " rk_test_123\n",
STRIPE_WEBHOOK_SECRET: " whsec_test ",
STRIPE_PRICE_ID: "price_test\n",
});
expect(fromEnv.STRIPE_PRICE_ID).toBe("price_test");
expect(fromEnv.STRIPE_SECRET_KEY).toBe("rk_test_123");

const fromFile = parseRawConfig({
...baseRawConfig,
stripe: {
secretKey: " sk_test_abc ",
webhookSecret: "whsec_file",
priceId: " price_file\n",
},
});
expect(fromFile.STRIPE_PRICE_ID).toBe("price_file");
expect(fromFile.STRIPE_SECRET_KEY).toBe("sk_test_abc");
});

it("rejects partially configured Google credentials", () => {
expect(() =>
parseConfigFromEnv({
Expand Down
12 changes: 7 additions & 5 deletions packages/backend/src/common/constants/config.constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,10 @@ const toStr = (
value: string | number | null | undefined,
): string | undefined => (value != null ? String(value) : undefined);

const nonEmpty = (value: string | null | undefined): string | undefined =>
value?.trim() ? value : undefined;
const nonEmpty = (value: string | null | undefined): string | undefined => {
const trimmed = value?.trim();
return trimmed ? trimmed : undefined;
};

export function parseRawConfig(config: CompassConfig): Config {
const nodeEnv = config.runtime.nodeEnv as NodeEnv;
Expand Down Expand Up @@ -151,9 +153,9 @@ export function parseConfigFromEnv(
SYNC_EXECUTION: nonEmpty(rawEnv["SYNC_EXECUTION"]),
POSTHOG_KEY: rawEnv["POSTHOG_KEY"],
POSTHOG_HOST: rawEnv["POSTHOG_HOST"] || DEFAULT_POSTHOG_HOST,
STRIPE_SECRET_KEY: rawEnv["STRIPE_SECRET_KEY"],
STRIPE_WEBHOOK_SECRET: rawEnv["STRIPE_WEBHOOK_SECRET"],
STRIPE_PRICE_ID: rawEnv["STRIPE_PRICE_ID"],
STRIPE_SECRET_KEY: nonEmpty(rawEnv["STRIPE_SECRET_KEY"]),
STRIPE_WEBHOOK_SECRET: nonEmpty(rawEnv["STRIPE_WEBHOOK_SECRET"]),
STRIPE_PRICE_ID: nonEmpty(rawEnv["STRIPE_PRICE_ID"]),
});
}

Expand Down
9 changes: 9 additions & 0 deletions packages/backend/src/servers/express/express.server.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { initExpressServer } from "@backend/servers/express/express.server";
import { describe, expect, it } from "bun:test";

describe("initExpressServer", () => {
it("trusts one proxy hop so Caddy X-Forwarded-For does not collapse rate-limit keys", () => {
const app = initExpressServer();
expect(app.get("trust proxy")).toBe(1);
});
});
3 changes: 3 additions & 0 deletions packages/backend/src/servers/express/express.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ import { UserRoutes } from "@backend/user/user.routes.config";
export const initExpressServer = () => {
/* Express Configuration */
const app: Application = express();
// Caddy terminates TLS and proxies `/api/*` with X-Forwarded-For. One hop
// of trust keeps express-rate-limit from treating every visitor as Caddy.
app.set("trust proxy", 1);

initSupertokens();

Expand Down
19 changes: 19 additions & 0 deletions packages/web/src/api/util/api.util.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
import {
createApiError as buildApiError,
getApiErrorCode,
getApiErrorMessage,
getErrorStatus,
handleErrorResponse,
isSessionLevelError,
Expand Down Expand Up @@ -142,6 +143,24 @@ describe("shouldShowContextualLoadError", () => {
});
});

describe("getApiErrorMessage", () => {
it("returns the error string from a JSON body", () => {
const error = createApiError({
data: { error: "Couldn't start billing. Please try again in a moment." },
});
expect(getApiErrorMessage(error)).toBe(
"Couldn't start billing. Please try again in a moment.",
);
});

it("returns undefined when the body has no error string", () => {
expect(
getApiErrorMessage(createApiError({ data: { code: "X" } })),
).toBeUndefined();
expect(getApiErrorMessage(new Error("nope"))).toBeUndefined();
});
});

describe("getApiErrorCode", () => {
it("returns the code when response.data has a string code property", () => {
const error = createApiError({ data: { code: "GOOGLE_REVOKED" } });
Expand Down
9 changes: 9 additions & 0 deletions packages/web/src/api/util/api.util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,15 @@ export const getApiErrorCode = (error: ApiError): string | undefined => {
return typeof code === "string" ? code : undefined;
};

/** Safe `error` string from a JSON `{ error: string }` API body, if present. */
export const getApiErrorMessage = (error: unknown): string | undefined => {
if (!isApiError(error)) return undefined;
const data = getApiErrorData(error);
if (!data || typeof data !== "object" || !("error" in data)) return undefined;
const message = (data as { error?: unknown }).error;
return typeof message === "string" && message.trim() ? message : undefined;
};

export const parseApiError = <T>(
error: ApiError,
schema: ZodType<T>,
Expand Down
Loading