Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR improves the resilience and maintainability of the HTTP/2 client implementation by adding comprehensive JSDoc type annotations and making defensive programming improvements.
- Adds JSDoc type definitions and function documentation for better type safety
- Improves error handling in GOAWAY frame processing with null checks
- Updates variable declarations and comparisons for better code clarity
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| // Specifically, we do not verify the "valid" stream id. | ||
|
|
||
| const err = this[kError] || new SocketError(`HTTP/2: "GOAWAY" frame received with code ${errorCode}`, util.getSocketInfo(this[kSocket])) | ||
| const err = this[kError] || new SocketError(`HTTP/2: "GOAWAY" frame received with code ${errorCode}`, this[kSocket] && util.getSocketInfo(this[kSocket])) |
There was a problem hiding this comment.
The null check this[kSocket] && will pass false to util.getSocketInfo() when socket is null/undefined, which may cause unexpected behavior. Consider using a ternary operator: this[kSocket] ? util.getSocketInfo(this[kSocket]) : null
| const err = this[kError] || new SocketError(`HTTP/2: "GOAWAY" frame received with code ${errorCode}`, this[kSocket] && util.getSocketInfo(this[kSocket])) | |
| const err = this[kError] || new SocketError(`HTTP/2: "GOAWAY" frame received with code ${errorCode}`, this[kSocket] ? util.getSocketInfo(this[kSocket]) : null) |
|
|
||
| /** @type {import('node:http2').ClientHttp2Stream} */ | ||
| let stream = null | ||
| let stream |
There was a problem hiding this comment.
[nitpick] Variable stream is declared without initialization. Consider initializing it explicitly as let stream = undefined for clarity, or use the original null initialization if that was intentional.
| let stream | |
| let stream = undefined |
This relates to...
Rationale
Changes
Features
Bug Fixes
Breaking Changes and Deprecations
Status