feat: Add support for invoking Cloud Function with multipart/form-data protocol#10395
Conversation
|
🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review. Tip
Note Please respond to review comments from AI agents just like you would to comments from a human reviewer. Let the reviewer resolve their own comments, unless they have reviewed and accepted your commit, or agreed with your explanation for why the feedback was incorrect. Caution Pull requests must be written using an AI agent with human supervision. Pull requests written entirely by a human will likely be rejected, because of lower code quality, higher review effort and the higher risk of introducing bugs. Please note that AI review comments on this pull request alone do not satisfy this requirement. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds server-side multipart/form-data handling for Cloud Code function calls: JSON parser skips multipart requests, a new multipart middleware (using Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Express as "Express/Router"
participant JSONParser as "JSON Parser"
participant MultipartMW as "Multipart Middleware"
participant Busboy as "@fastify/busboy"
participant CloudFunc as "Cloud Function Handler"
participant Response
Client->>Express: POST /functions/:name (multipart/form-data)
Express->>JSONParser: should parse body?
JSONParser-->>Express: skip (is multipart)
Express->>MultipartMW: run multipartMiddleware
MultipartMW->>Busboy: stream request -> parse parts
Busboy-->>MultipartMW: emit field/file events
MultipartMW->>MultipartMW: accumulate bytes, enforce maxUploadSize
MultipartMW->>MultipartMW: construct req.body (text + files as Buffers)
MultipartMW-->>Express: next()
Express->>CloudFunc: handleCloudFunction(req)
CloudFunc->>CloudFunc: parse params (Buffer passthrough)
CloudFunc-->>Response: result
Response-->>Client: HTTP response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
multipart/form-data
multipart/form-datamultipart/form-data protocol
multipart/form-data protocol multipart/form-data protocol
There was a problem hiding this comment.
🧹 Nitpick comments (4)
package.json (1)
1-173: Consider adding scope to PR title for better changelog clarity.The current PR title follows Angular commit convention. To make it more specific for developers scanning the changelog, consider adding the
functionsscope:feat(functions): Add multipart/form-data support for Cloud Function invocationThis makes it immediately clear which subsystem is affected. Based on learnings, suggesting PR titles that make meaningful changelog entries for developers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 1 - 173, Update the PR title to include the "functions" scope so the changelog is clearer; change the current title to: "feat(functions): Add multipart/form-data support for Cloud Function invocation" and ensure any related commit messages or squash commit used for the merge also use this scoped title.src/Routers/FunctionsRouter.js (3)
230-235: Add braces to single-lineifstatement per linting rules.🔧 Proposed fix
busboy.on('finish', () => { - if (settled) return; + if (settled) { + return; + } settled = true; req.body = fields; resolve(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Routers/FunctionsRouter.js` around lines 230 - 235, The single-line conditional inside the busboy 'finish' handler uses an unbraced if (settled) return; which violates linting rules; update the handler in the busboy.on('finish', ...) callback (the code referencing settled, req.body = fields, and resolve()) to use a braced conditional (e.g., if (settled) { return; }) so the early-return is explicitly scoped and consistent with lint rules.
221-228: Add braces to single-lineifstatement per linting rules.🔧 Proposed fix
stream.on('end', () => { - if (settled) return; + if (settled) { + return; + } fields[name] = { filename, contentType: mimeType || 'application/octet-stream', data: Buffer.concat(chunks), }; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Routers/FunctionsRouter.js` around lines 221 - 228, The single-line if inside the stream.on('end', ...) callback violates lint rules; update the callback so the conditional uses braces: replace "if (settled) return;" with a block-form conditional (e.g., "if (settled) { return; }") within the stream.on('end' ...) handler where variables like settled, fields, name, filename, mimeType, chunks are used to populate fields[name].
188-193: Add braces to single-lineifstatement per linting rules.The static analysis tool flagged the missing braces on the early return.
🔧 Proposed fix
const safeReject = (err) => { - if (settled) return; + if (settled) { + return; + } settled = true; busboy.destroy(); reject(err); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Routers/FunctionsRouter.js` around lines 188 - 193, The single-line early-return inside the safeReject function lacks braces (if (settled) return;) which violates lint rules; update safeReject to use a braced if block (if (settled) { return; }) so the guard is clear and consistent, keeping the rest of the function (setting settled = true, busboy.destroy(), reject(err)) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@package.json`:
- Around line 1-173: Update the PR title to include the "functions" scope so the
changelog is clearer; change the current title to: "feat(functions): Add
multipart/form-data support for Cloud Function invocation" and ensure any
related commit messages or squash commit used for the merge also use this scoped
title.
In `@src/Routers/FunctionsRouter.js`:
- Around line 230-235: The single-line conditional inside the busboy 'finish'
handler uses an unbraced if (settled) return; which violates linting rules;
update the handler in the busboy.on('finish', ...) callback (the code
referencing settled, req.body = fields, and resolve()) to use a braced
conditional (e.g., if (settled) { return; }) so the early-return is explicitly
scoped and consistent with lint rules.
- Around line 221-228: The single-line if inside the stream.on('end', ...)
callback violates lint rules; update the callback so the conditional uses
braces: replace "if (settled) return;" with a block-form conditional (e.g., "if
(settled) { return; }") within the stream.on('end' ...) handler where variables
like settled, fields, name, filename, mimeType, chunks are used to populate
fields[name].
- Around line 188-193: The single-line early-return inside the safeReject
function lacks braces (if (settled) return;) which violates lint rules; update
safeReject to use a braced if block (if (settled) { return; }) so the guard is
clear and consistent, keeping the rest of the function (setting settled = true,
busboy.destroy(), reject(err)) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 770506d5-a803-4582-9641-a4ff9aee8506
📒 Files selected for processing (5)
package-lock.jsonpackage.jsonspec/CloudCodeMultipart.spec.jssrc/ParseServer.tssrc/Routers/FunctionsRouter.js
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## alpha #10395 +/- ##
==========================================
- Coverage 92.52% 92.50% -0.02%
==========================================
Files 192 192
Lines 16663 16721 +58
Branches 229 229
==========================================
+ Hits 15417 15468 +51
- Misses 1224 1231 +7
Partials 22 22 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/Routers/FunctionsRouter.js (1)
186-186: Normalize invalidmaxUploadSizevalues here.Line 186 is the first place the functions path parses
req.config.maxUploadSize, so a bad config bubbles out as the rawparseSizeToBytesfailure. Consider validating this once at startup or mirroring the guard already used on the file-upload path so multipart calls fail with a clearer error.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Routers/FunctionsRouter.js` at line 186, The code currently calls Utils.parseSizeToBytes(req.config.maxUploadSize) directly in FunctionsRouter (producing const maxBytes) which lets bad config values throw raw errors; update this to validate/normalize req.config.maxUploadSize before parsing (or catch parse errors) and mirror the existing file-upload guard behavior: if parse fails or yields a non-positive value, return a clear HTTP error (or fall back to a sensible default) with a descriptive message referencing req.config.maxUploadSize, and ensure Utils.parseSizeToBytes usage around maxBytes is wrapped in a try/validation branch so multipart function uploads fail with that clearer error instead of bubbling the raw exception.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Routers/FunctionsRouter.js`:
- Around line 193-218: The Busboy instance is not configured to respect Parse
Server's maxUploadSize and the field handler doesn't check truncation flags, so
text fields can be silently truncated; when creating Busboy (where you call
Busboy({ headers: req.headers })) pass limits that wire fieldSize (and
optionally fieldNameSize) to the same maxUploadSize value, and inside the
busboy.on('field', (name, value, info) => ...) handler check
info.fieldnameTruncated or info.valueTruncated (or valTruncated) and call
safeReject with a Parse.Error(Parse.Error.OBJECT_TOO_LARGE, 'Multipart request
exceeds maximum upload size.') if truncation occurred, preserving the existing
totalBytes check and use of safeReject/settled.
- Around line 34-35: parseParams currently returns Buffer objects verbatim
(Buffer.isBuffer(obj) branch), which lets JSON.stringify serialize full byte
arrays before truncateLogMessage runs; update parseParams in FunctionsRouter.js
to detect Buffer instances (and any nested Buffers) and replace them with a
safe/redacted placeholder (e.g., "[REDACTED_BUFFER]" or an object with only the
size/metadata) so that when params are later JSON.stringify'd (see where params
is logged around the previous params logging calls) large uploads from the
multipart path (the { filename, contentType, data: Buffer } objects handled
around the parseParams call) do not cause full byte-array serialization; ensure
the Buffer redaction is applied recursively in parseParams so nested Buffers are
also replaced.
---
Nitpick comments:
In `@src/Routers/FunctionsRouter.js`:
- Line 186: The code currently calls
Utils.parseSizeToBytes(req.config.maxUploadSize) directly in FunctionsRouter
(producing const maxBytes) which lets bad config values throw raw errors; update
this to validate/normalize req.config.maxUploadSize before parsing (or catch
parse errors) and mirror the existing file-upload guard behavior: if parse fails
or yields a non-positive value, return a clear HTTP error (or fall back to a
sensible default) with a descriptive message referencing
req.config.maxUploadSize, and ensure Utils.parseSizeToBytes usage around
maxBytes is wrapped in a try/validation branch so multipart function uploads
fail with that clearer error instead of bubbling the raw exception.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 588dd585-5d6e-499d-9d2e-15ae7440727a
📒 Files selected for processing (5)
package-lock.jsonpackage.jsonspec/CloudCodeMultipart.spec.jssrc/Routers/FunctionsRouter.jssrc/cloud-code/Parse.Cloud.js
✅ Files skipped from review due to trivial changes (3)
- package-lock.json
- package.json
- src/cloud-code/Parse.Cloud.js
# [9.8.0-alpha.5](9.8.0-alpha.4...9.8.0-alpha.5) (2026-04-04) ### Features * Add support for invoking Cloud Function with `multipart/form-data` protocol ([#10395](#10395)) ([a3f36a2](a3f36a2))
|
🎉 This change has been released in version 9.8.0-alpha.5 |
Pull Request
Issue
Cloud Code Functions only accept JSON payloads. Developers who need to send files to a Cloud Function must first upload the file separately, then call the function referencing the uploaded file.
Approach
Add
multipart/form-datasupport toPOST /functions/:functionName. A route-level middleware onFunctionsRouterdetects the content type and parses the body with@fastify/busboy. Text fields arrive as strings, file fields arrive as{ filename, contentType, data: Buffer }. All fields merge flat intorequest.params.Tasks
Summary by CodeRabbit
New Features
Bug Fixes / Privacy
Tests
Chores
Documentation