feat: Add Vercel serverless deployment for API v2 #26548
Conversation
|
Hey there and thank you for opening this pull request! 👋🏼 We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted. Details: |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 6 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/api/v2/src/main.ts">
<violation number="1" location="apps/api/v2/src/main.ts:21">
P1: Race condition: Multiple concurrent cold-start requests can initialize multiple Nest instances simultaneously. Use a promise-based singleton pattern to ensure only one initialization runs.</violation>
</file>
<file name="apps/api/v2/src/instrument.ts">
<violation number="1" location="apps/api/v2/src/instrument.ts:2">
P0: This import change will break Sentry initialization. The `@sentry/nestjs` package does not have a default export - it only exports named functions/integrations. Using `import Sentry from "@sentry/nestjs"` instead of `import * as Sentry from "@sentry/nestjs"` will cause `Sentry.init()` and `Sentry.prismaIntegration()` to fail at runtime.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
apps/api/v2/src/instrument.ts
Outdated
| @@ -1,5 +1,5 @@ | |||
| import { getEnv } from "@/env"; | |||
| import * as Sentry from "@sentry/nestjs"; | |||
| import Sentry from "@sentry/nestjs"; | |||
There was a problem hiding this comment.
P0: This import change will break Sentry initialization. The @sentry/nestjs package does not have a default export - it only exports named functions/integrations. Using import Sentry from "@sentry/nestjs" instead of import * as Sentry from "@sentry/nestjs" will cause Sentry.init() and Sentry.prismaIntegration() to fail at runtime.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/v2/src/instrument.ts, line 2:
<comment>This import change will break Sentry initialization. The `@sentry/nestjs` package does not have a default export - it only exports named functions/integrations. Using `import Sentry from "@sentry/nestjs"` instead of `import * as Sentry from "@sentry/nestjs"` will cause `Sentry.init()` and `Sentry.prismaIntegration()` to fail at runtime.</comment>
<file context>
@@ -1,5 +1,5 @@
import { getEnv } from "@/env";
-import * as Sentry from "@sentry/nestjs";
+import Sentry from "@sentry/nestjs";
import { nodeProfilingIntegration } from "@sentry/profiling-node";
</file context>
| import Sentry from "@sentry/nestjs"; | |
| import * as Sentry from "@sentry/nestjs"; |
✅ Addressed in 81d60ac
| * CACHE: This allows the Nest app to persist across multiple | ||
| * serverless "warm" invocations, significantly reducing latency. | ||
| */ | ||
| let cachedServer: any; |
There was a problem hiding this comment.
P1: Race condition: Multiple concurrent cold-start requests can initialize multiple Nest instances simultaneously. Use a promise-based singleton pattern to ensure only one initialization runs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/v2/src/main.ts, line 21:
<comment>Race condition: Multiple concurrent cold-start requests can initialize multiple Nest instances simultaneously. Use a promise-based singleton pattern to ensure only one initialization runs.</comment>
<file context>
@@ -5,42 +5,82 @@ import { NestFactory } from "@nestjs/core";
+ * CACHE: This allows the Nest app to persist across multiple
+ * serverless "warm" invocations, significantly reducing latency.
+ */
+let cachedServer: any;
+
+/**
</file context>
- Revert namespace imports (import * as X) for qs, Sentry, jwt, tzdata - Update vercel.json to use simpler configuration with buildCommand, outputDirectory, functions, and rewrites - Remove complex two-step build with @vercel/static-build Co-Authored-By: morgan@cal.com <morgan@cal.com>
devin-ai-integration
bot
changed the title
Co-Authored-By: morgan@cal.com <morgan@cal.com>
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/api/v2/vercel.json">
<violation number="1" location="apps/api/v2/vercel.json:6">
P1: Missing `buildCommand` will cause deployment failures. This NestJS monorepo app requires workspace dependencies to be built first (`yarn turbo run build --filter=@calcom/api-v2`). The `@vercel/node` builder alone cannot resolve workspace package dependencies like `@calcom/platform-libraries` or handle the complex build process. Consider restoring `buildCommand` or configuring Vercel root settings appropriately.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
apps/api/v2/vercel.json
Outdated
| "builds": [ | ||
| { | ||
| "src": "src/main.ts", | ||
| "use": "@vercel/node" |
There was a problem hiding this comment.
P1: Missing buildCommand will cause deployment failures. This NestJS monorepo app requires workspace dependencies to be built first (yarn turbo run build --filter=@calcom/api-v2). The @vercel/node builder alone cannot resolve workspace package dependencies like @calcom/platform-libraries or handle the complex build process. Consider restoring buildCommand or configuring Vercel root settings appropriately.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/v2/vercel.json, line 6:
<comment>Missing `buildCommand` will cause deployment failures. This NestJS monorepo app requires workspace dependencies to be built first (`yarn turbo run build --filter=@calcom/api-v2`). The `@vercel/node` builder alone cannot resolve workspace package dependencies like `@calcom/platform-libraries` or handle the complex build process. Consider restoring `buildCommand` or configuring Vercel root settings appropriately.</comment>
<file context>
@@ -1,18 +1,16 @@
+ "builds": [
+ {
+ "src": "src/main.ts",
+ "use": "@vercel/node"
}
- },
</file context>
✅ Addressed in d972d86
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/api/v2/src/modules/slots/slots-2024-04-15/controllers/slots.controller.ts">
<violation number="1" location="apps/api/v2/src/modules/slots/slots-2024-04-15/controllers/slots.controller.ts:168">
P1: Rule violated: **Avoid Logging Sensitive Information**
This console.log statement can expose PII (email addresses) in production logs. The `query` object contains `email` and `teamMemberEmail` fields. Remove this debug log or use a proper logging framework with PII redaction.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| @Req() req: ExpressRequest | ||
| ): Promise<ApiResponse<{ slots: TimeSlots["slots"] | RangeSlots["slots"] }>> { | ||
| try { | ||
| console.log("Get available slots query", query); |
There was a problem hiding this comment.
P1: Rule violated: Avoid Logging Sensitive Information
This console.log statement can expose PII (email addresses) in production logs. The query object contains email and teamMemberEmail fields. Remove this debug log or use a proper logging framework with PII redaction.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/v2/src/modules/slots/slots-2024-04-15/controllers/slots.controller.ts, line 168:
<comment>This console.log statement can expose PII (email addresses) in production logs. The `query` object contains `email` and `teamMemberEmail` fields. Remove this debug log or use a proper logging framework with PII redaction.</comment>
<file context>
@@ -165,6 +165,7 @@ export class SlotsController_2024_04_15 {
@Req() req: ExpressRequest
): Promise<ApiResponse<{ slots: TimeSlots["slots"] | RangeSlots["slots"] }>> {
try {
+ console.log("Get available slots query", query);
const isTeamEvent =
query.isTeamEvent === undefined
</file context>
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/platform/types/slots/slots-2024-04-15/inputs/index.ts">
<violation number="1" location="packages/platform/types/slots/slots-2024-04-15/inputs/index.ts:149">
P2: The filter `!isNaN(n)` does not filter out `null` values because `isNaN(null)` returns `false` (null coerces to 0). Consider using a more explicit check to handle null/undefined values.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| // 2. Map everything to a Number. class-validator @IsNumber | ||
| // needs the actual type to be 'number', not 'string'. | ||
| return array.map((val) => (typeof val === "string" ? parseInt(val, 10) : val)).filter((n) => !isNaN(n)); // Clean out any bad data |
There was a problem hiding this comment.
P2: The filter !isNaN(n) does not filter out null values because isNaN(null) returns false (null coerces to 0). Consider using a more explicit check to handle null/undefined values.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/platform/types/slots/slots-2024-04-15/inputs/index.ts, line 149:
<comment>The filter `!isNaN(n)` does not filter out `null` values because `isNaN(null)` returns `false` (null coerces to 0). Consider using a more explicit check to handle null/undefined values.</comment>
<file context>
@@ -141,10 +141,12 @@ export class GetAvailableSlotsInput_2024_04_15 {
+
+ // 2. Map everything to a Number. class-validator @IsNumber
+ // needs the actual type to be 'number', not 'string'.
+ return array.map((val) => (typeof val === "string" ? parseInt(val, 10) : val)).filter((n) => !isNaN(n)); // Clean out any bad data
})
@IsArray()
</file context>
| return array.map((val) => (typeof val === "string" ? parseInt(val, 10) : val)).filter((n) => !isNaN(n)); // Clean out any bad data | |
| return array.map((val) => (typeof val === "string" ? parseInt(val, 10) : val)).filter((n) => n != null && !isNaN(n)); // Clean out any bad data |
✅ Addressed in 8e5c2ca
There was a problem hiding this comment.
2 issues found across 2 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/platform/types/slots/slots-2024-04-15/inputs/index.ts">
<violation number="1">
P2: Missing NaN filter: Invalid values like `"abc"` will result in `NaN` being included in the array, whereas the original code filtered these out with `.filter((n) => !isNaN(n))`.</violation>
<violation number="2">
P1: Breaking change: Single values are no longer wrapped in arrays. Query params like `?routedTeamMemberIds=5` (common for single-element queries) will now fail the `@IsArray()` validation since the string `"5"` is returned as-is instead of being converted to `[5]`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
What does this PR do?
Adds Vercel serverless support for API v2 with a serverless entrypoint and a warm cache to reduce cold starts. Local development stays the same.
Changes
main.tsthat initializes the NestJS app and caches the instance across warm invocationsvercel.jsonwithbuildsusing@vercel/nodeandroutesto handle all HTTP methodsimport * as X) for qs, cookie-parser, etc. to maintain compatibilityUpdates since last revision
vercel.jsonto usebuildswith@vercel/nodepointing tosrc/main.tsandroutespattern (standard NestJS on Vercel approach)functions/rewritesconfiguration caused "pattern doesn't match any Serverless Functions inside theapidirectory" errorVisual Demo (For contributors especially)
N/A - Infrastructure/deployment change
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
yarn devHuman Review Checklist
@vercel/nodecan resolve workspace dependencies (@calcom/platform-*, @calcom/prisma, etc.)@/) resolve correctly when Vercel compiles TypeScriptanytypes for req/res - acceptable for serverless handler but worth notingChecklist
Link to Devin run: https://app.devin.ai/sessions/c453624347b247d5bfec8f091f657119
Requested by: morgan@cal.com (@ThyMinimalDev)