Skip to content
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

feat: Add configurable request body size limit option #380

Merged
merged 4 commits into from
Feb 17, 2025

Conversation

contactjittu
Copy link
Contributor

feat: Add configurable request body size limit option

Description

Adds a new --bodyLimit CLI option and configuration parameter to control the maximum allowed HTTP request payload size in the faas-js-runtime.

Changes

  • Added --bodyLimit CLI option to set maximum request payload size
  • Added FUNC_BODY_LIMIT environment variable support
  • Default body limit set to 1MB (1048576 bytes)
  • Updated README.md with new option

Example Usage

# Set 2MB limit via CLI
faas-js-runtime function.js --bodyLimit 2097152

# Set limit via environment variable
FUNC_BODY_LIMIT=2097152 faas-js-runtime function.js

Technical Details

  • Implements body size limiting using Fastify's built-in bodyLimit option

@contactjittu contactjittu marked this pull request as ready for review February 17, 2025 09:10
@lholmquist
Copy link
Member

thanks for the PR, mind adding a test for this new feature?

package.json Outdated
@@ -1,6 +1,6 @@
{
"name": "faas-js-runtime",
"version": "2.4.1",
"version": "2.5.0",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need for this in the PR, we have some automation that will update the version when we do the release

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed version bump

@lholmquist
Copy link
Member

looks like the tests are fine, but it's failing on uploading codecoverage. I fixed that in this issue #381 , would you mind doing a quick rebase?

Copy link
Member

@lholmquist lholmquist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks for the PR

@lholmquist lholmquist merged commit 6c8055f into nodeshift:main Feb 17, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants