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

fix(node): do not buffer first chunk #2989

Merged
merged 3 commits into from
Dec 8, 2022

Conversation

kt3k
Copy link
Member

@kt3k kt3k commented Dec 7, 2022

This PR updates http ServerResponse class.

Currently the first chunk of server responses are buffered in the private field #firstChunk to add content-type: text/plain;charset=UTF-8 automatically when the only chunk is string (only on non flash-backed implementation). This behavior is not tested anywhere and it seems only causing the issue (denoland/deno#16929). This PR removes this buffering behavior, and now the server always responds the first chunk immediately.

closes denoland/deno#16929

  • write tests

@kt3k kt3k marked this pull request as ready for review December 8, 2022 15:03
@kt3k kt3k requested a review from bartlomieju December 8, 2022 15:04
Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for fixing it Yoshiya!

@kt3k kt3k merged commit eca4dbf into denoland:main Dec 8, 2022
@kt3k kt3k deleted the fix-node-http-first-chunk branch December 8, 2022 15:13
bartlomieju added a commit to bartlomieju/deno_std that referenced this pull request Dec 16, 2022
bartlomieju added a commit that referenced this pull request Dec 19, 2022
This reverts commit eca4dbf.

This commit caused performance regression in express server from
25k rps to 20k rps.
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.

Node: Sending chunked responses
2 participants