feat: add transformRequest option for custom request streams#604
feat: add transformRequest option for custom request streams#604ZawaPaP wants to merge 7 commits intofastify:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a new transformRequest option to @fastify/multipart that allows users to provide a custom stream for multipart parsing. This addresses issue #547 to support serverless environments like Google Cloud Functions and Firebase where the request body is pre-consumed, making it unavailable for standard stream processing.
Changes:
- Added
transformRequestoption that accepts a function to transform the request stream before piping to Busboy - Added TypeScript type definitions for the new option
- Added documentation and examples showing usage for GCP/Firebase scenarios
- Added test coverage for the basic functionality
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| index.js | Implements transformRequest logic by allowing a custom stream to be piped to Busboy parser instead of the raw request |
| types/index.d.ts | Adds TypeScript type definition for transformRequest option |
| types/index.test-d.ts | Adds type test to verify transformRequest parameter and return types |
| test/multipart-transform-request.test.js | Adds tests verifying transformRequest is called and can provide custom stream data |
| README.md | Documents the transformRequest option with a GCP-specific example |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }) | ||
|
|
||
| request.pipe(bb) | ||
| const stream = transformRequest(request) |
There was a problem hiding this comment.
When transformRequest returns a different stream than the original request, event listeners for 'close' and 'error' are only attached to the original request object (lines 275-276, outside this diff). This means errors or close events from the transformed stream won't trigger cleanup, potentially leading to resource leaks.
Consider also attaching event listeners to the transformed stream when it differs from the original request, similar to how event listeners are attached to the request object.
| const stream = transformRequest(request) | |
| const stream = transformRequest(request) | |
| if (stream !== request && typeof stream?.on === 'function') { | |
| stream.on('close', cleanup) | |
| stream.on('error', cleanup) | |
| } |
There was a problem hiding this comment.
this is correct to fix, but possibly with a different implementation
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Matteo Collina <matteo.collina@gmail.com>
…ify-multipart into feature/support-gcf
|
Thanks for looking into this PR, @mcollina. I have added the following based on Copilot's suggestions:
Could you please take another look when you have a chance? Let me know if I should take any further action. |
index.js
Outdated
|
|
||
| request.pipe(bb) | ||
| const stream = transformRequest(request) | ||
| if (stream instanceof Error) { |
There was a problem hiding this comment.
unfortunately this can't work because users can throw whatever. you should probably let the throw flow out and handle it in that way.
There was a problem hiding this comment.
Thank you for taking a look!
I've simplified it to let the error flow out and only check if the transformed data is a stream before passing it to Busboy.
Summary
I implemented a new option,
transformRequest, allowing users to explicitly provide a custom stream to be piped into Busboy.Background
As discussed in #547, environment-specific behaviors (such as Google Cloud Functions) pre-consume the request body, leaving the original request stream empty. This makes multipart parsing impossible with the standard streaming approach.
How to use
Checklist
npm run test && npm run benchmark --if-presentand the Code of conduct