-
-
Notifications
You must be signed in to change notification settings - Fork 103
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support firebase #548
base: master
Are you sure you want to change the base?
Support firebase #548
Conversation
Signed-off-by: Thomas Kientz <[email protected]>
the tests are not happy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Decorate that new assignment to req please
Signed-off-by: Thomas Kientz <[email protected]>
Signed-off-by: Thomas Kientz <[email protected]>
@Uzlopak don't really understand why it's failing, looks good but return exit code 1 |
The test coverage is Not 100% |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is payload.rawBody
comes from?
I don't like the current idea to support one serverless environment and adding extra branching.
Instead one should provide a plugin
that dedicated for the serverless environment and add all the necessary patching to match how Node.js
works.
Well the problem is, that gcf is a glorified express router. So they preparse the body so that you dont have to. You get the body stream in rawBody. So maybe we need, like you suggest, a plugin which basically removes the default content type parsers and just assigns rawbody to body?! |
What's comes my mind is something like? function gcp(fastify) {
// add pass through content-type parser
fastify.addContentTypeParser('application/json', {}, (req, body, done) => {
done(null, body.body);
});
// add stream transform for multipart when neccessary
if (fastify.hasContentTypeParser('multipart/form-data')) {
fastify.addHook('preParsing', function(request, reply, payload, done) {
if(request.headers['content-type'].startsWith('multipart/form-data')) {
// we override the `.pipe` method and return rawBody as new payload
const pipe = request.raw.pipe
request.raw.pipe = function(stream) {
Readable.from([rawBody]).pipe(stream)
}
request.raw.originalPipe = pipe
done(null, payload.rawBody)
} else {
done(null, payload)
}
})
}
} |
Should I pursue the current implementation of this pull request and add test coverage? @Uzlopak |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 1 out of 1 changed files in this pull request and generated no suggestions.
Currently we can't use this plugin with serverless GCP (or Firebase).
As explained here fastify/fastify#946 (comment) and in the doc for firebase, we need to use
req.rawBody
in order to parse files from multipart.