Skip to content

Comments

feat(http): add HTTP2 support; (#7150)#1

Open
MitchLewis930 wants to merge 1 commit intopr_021_beforefrom
pr_021_after
Open

feat(http): add HTTP2 support; (#7150)#1
MitchLewis930 wants to merge 1 commit intopr_021_beforefrom
pr_021_after

Conversation

@MitchLewis930
Copy link

@MitchLewis930 MitchLewis930 commented Jan 30, 2026

PR_021


Note

Medium Risk
Touches the core Node transport (lib/adapters/http.js) and introduces new protocol/session behavior, which could impact request lifecycle/cancellation and connection reuse; changes are mitigated by extensive new tests but still affect a critical code path.

Overview
Adds experimental HTTP/2 support to the Node http adapter via a new httpVersion config (1 or 2) that switches request execution to a http2-based transport.

Introduces http2Options (including a sessionTimeout defaulting to 1000ms) and implements per-authority HTTP/2 session pooling/reuse keyed by resolved options, plus adapter changes to better unify abort/timeout/cancel handling across HTTP/1 and HTTP/2 (e.g., using req.close() when available and ensuring cleanup for stream responses).

Updates TypeScript definitions, README documentation, test infrastructure (adds HTTP/2 secure server support with self-signed certs), and adds a comprehensive HTTP/2 test suite covering payloads, response types, timeouts/cancellation, and session reuse; also tweaks rate-limit test tolerances.

Written by Cursor Bugbot for commit d676df7. This will update automatically on new commits. Configure here.

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 3 potential issues.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.


const http2Transport = {
request(options, cb) {
const authority = options.protocol + '//' + options.hostname + ':' + (options.port || 80);
Copy link

Choose a reason for hiding this comment

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

HTTP2 uses wrong default port for HTTPS connections

High Severity

When constructing the HTTP/2 authority, options.port || 80 always defaults to port 80, regardless of protocol. For HTTPS URLs without explicit ports (e.g., https://httpbin.org/post), this constructs an authority like https://httpbin.org:80 instead of https://httpbin.org:443. Since HTTPS services listen on port 443 by default, this causes HTTP/2 connections to fail. The README documentation example at line 1718 would not work due to this bug.

Fix in Cursor Fix in Web


response.statusCode = +status;

cb(response);
Copy link

Choose a reason for hiding this comment

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

HTTP2 responses have undefined statusText

Medium Severity

The http2Transport response handler sets statusCode and headers on the response object but never sets statusMessage. When the response is built at line 696 using statusText: res.statusMessage, HTTP/2 responses will have statusText: undefined. This violates the TypeScript type definition (statusText: string) and could break user code that assumes statusText is always a string.

Additional Locations (1)

Fix in Cursor Fix in Web

}
}
}
});
Copy link

Choose a reason for hiding this comment

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

Missing error handler on HTTP/2 session causes crashes

High Severity

The HTTP/2 session created via connect() only has a 'close' event handler but no 'error' handler. In Node.js, when an EventEmitter emits 'error' without a listener, it throws an uncaught exception. HTTP/2 sessions can emit errors for connection failures, TLS/SSL issues, protocol violations, and GOAWAY frames. Without an error handler, these scenarios would crash the process rather than being caught and properly rejected.

Fix in Cursor Fix in Web

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