Conversation
| handlerPath: | ||
| rawCreateServerlessFunctionInput.handlerPath ?? DEFAULT_HANDLER_PATH, | ||
| handlerName: | ||
| rawCreateServerlessFunctionInput.handlerName ?? DEFAULT_HANDLER_NAME, |
There was a problem hiding this comment.
Bug: Empty strings for handlerPath or handlerName bypass default fallbacks, leading to esbuild failures during serverless function execution.
Severity: CRITICAL | Confidence: 0.98
🔍 Detailed Analysis
The system allows empty strings for handlerPath or handlerName fields because @IsString() validation does not prevent them, and the nullish coalescing operator ?? does not trigger fallbacks for empty strings. When a client provides an empty string for handlerPath, it is persisted to the database. During serverless function execution, buildServerlessFunctionInMemory uses this empty handlerPath, causing join(sourceTemporaryDir, handlerPath) to resolve to a directory. esbuild then fails to build the function because the entry point is a directory instead of a file.
💡 Suggested Fix
Add @MinLength(1) to the DTO fields for handlerPath and handlerName, or modify the utility to use a more robust fallback pattern like rawCreateServerlessFunctionInput.handlerPath?.trim() || DEFAULT_HANDLER_PATH.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location:
packages/twenty-server/src/engine/metadata-modules/serverless-function/utils/from-create-serverless-function-input-to-flat-serverless-function.util.ts#L33-L36
Potential issue: The system allows empty strings for `handlerPath` or `handlerName`
fields because `@IsString()` validation does not prevent them, and the nullish
coalescing operator `??` does not trigger fallbacks for empty strings. When a client
provides an empty string for `handlerPath`, it is persisted to the database. During
serverless function execution, `buildServerlessFunctionInMemory` uses this empty
`handlerPath`, causing `join(sourceTemporaryDir, handlerPath)` to resolve to a
directory. `esbuild` then fails to build the function because the entry point is a
directory instead of a file.
Did we get this right? 👍 / 👎 to inform future reviews.
e6a5fc6 to
70bd6a5
Compare
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
Added optional handlerPath and handlerName fields to serverless function creation, allowing users to override default handler configuration while maintaining backward compatibility.
Key changes:
- Added
handlerPathandhandlerNameas optional string fields inCreateServerlessFunctionInputDTO with proper validation decorators - Updated transformation utility to use provided values or fall back to
DEFAULT_HANDLER_PATH(src/index.ts) andDEFAULT_HANDLER_NAME(main) - Maintains consistency with existing update functionality where these fields are already supported
Confidence Score: 5/5
- This PR is safe to merge with minimal risk
- The changes are straightforward, well-scoped, and maintain backward compatibility. Optional fields with proper defaults ensure existing functionality is unaffected. The implementation pattern matches the existing update DTO, demonstrating consistency across the codebase.
- No files require special attention
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| packages/twenty-server/src/engine/metadata-modules/serverless-function/dtos/create-serverless-function.input.ts | 5/5 | Added optional handlerName and handlerPath fields to allow custom handler configuration during serverless function creation |
| packages/twenty-server/src/engine/metadata-modules/serverless-function/utils/from-create-serverless-function-input-to-flat-serverless-function.util.ts | 5/5 | Updated transformation logic to use custom handlerPath and handlerName from input, falling back to defaults when not provided |
Sequence Diagram
sequenceDiagram
participant Client
participant CreateServerlessFunctionInput
participant TransformUtil
participant FlatServerlessFunction
participant Database
Client->>CreateServerlessFunctionInput: Create with optional handlerPath & handlerName
Note over CreateServerlessFunctionInput: Validates string fields<br/>handlerPath (optional)<br/>handlerName (optional)
CreateServerlessFunctionInput->>TransformUtil: fromCreateServerlessFunctionInputToFlatServerlessFunction()
Note over TransformUtil: handlerPath = input.handlerPath ?? DEFAULT_HANDLER_PATH<br/>handlerName = input.handlerName ?? DEFAULT_HANDLER_NAME
TransformUtil->>FlatServerlessFunction: Create flat object
FlatServerlessFunction->>Database: Persist with handler values
2 files reviewed, no comments
Weiko
left a comment
There was a problem hiding this comment.
You probably want some kind of validation otherwise LGTM
|
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:6189 This environment will automatically shut down when the PR is closed or after 5 hours. |
Fixes serverlessFucntion are created with default handlerPath and handlerName values