Skip to content

feat: implement h2c client #4118

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

Merged
merged 14 commits into from
Apr 2, 2025
Merged

feat: implement h2c client #4118

merged 14 commits into from
Apr 2, 2025

Conversation

metcoder95
Copy link
Member

@metcoder95 metcoder95 commented Mar 24, 2025

This relates to...

Closes #2868

Rationale

Changes

Features

Bug Fixes

Breaking Changes and Deprecations

Status

@metcoder95 metcoder95 marked this pull request as ready for review March 25, 2025 10:09
- **keepAliveTimeoutThreshold** `number | null` (optional) - Default: `2e3` - A number of milliseconds subtracted from server _keep-alive_ hints when overriding `keepAliveTimeout` to account for timing inaccuracies caused by e.g. transport latency. Defaults to 2 seconds.
- **maxHeaderSize** `number | null` (optional) - Default: `--max-http-header-size` or `16384` - The maximum length of request headers in bytes. Defaults to Node.js' --max-http-header-size or 16KiB.
- **maxResponseSize** `number | null` (optional) - Default: `-1` - The maximum length of response body in bytes. Set to `-1` to disable.
- **pipelining** `number | null` (optional) - Default: `1` - The amount of concurrent requests to be sent over the single TCP/TLS connection according to [RFC7230](https://tools.ietf.org/html/rfc7230#section-6.3.2). Carefully consider your workload and environment before enabling concurrent requests as pipelining may reduce performance if used incorrectly. Pipelining is sensitive to network stack settings as well as head of line blocking caused by e.g. long running requests. Set to `0` to disable keep-alive connections.
Copy link
Member

Choose a reason for hiding this comment

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

I have a rough feeling this description doesn't match h2c.

Can I send more than one request in parallel right?

Copy link
Member Author

Choose a reason for hiding this comment

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

We are under the same constraints as a normal Client with allowH2 enabled, then in theory we should be able to send and manage parallel requests as per H2 streams multiplexing concept

Copy link
Member

Choose a reason for hiding this comment

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

Yes exactly. Then what does this piece of doc mean?

Copy link
Member Author

Choose a reason for hiding this comment

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

See what mean; let me rephrase it, taking it from Client leave it too ambiguous

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@metcoder95 metcoder95 requested review from mcollina and ronag March 28, 2025 10:52
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@metcoder95 metcoder95 merged commit 1749556 into main Apr 2, 2025
34 of 37 checks passed
@github-actions github-actions bot mentioned this pull request Apr 2, 2025
@github-actions github-actions bot mentioned this pull request May 12, 2025
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.

HTTP/2 over unix domain sockets returns ERR_SSL_WRONG_VERSION_NUMBER
2 participants