feat(http): add HTTP2 support; (#7150)#1
Conversation
|
@CodeRabbit full review |
✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughThis pull request introduces HTTP/2 support to axios's HTTP adapter. It adds Changes
Sequence DiagramsequenceDiagram
participant Client as Axios Client
participant Adapter as HTTP Adapter
participant Sessions as Http2Sessions
participant Session as HTTP/2 Session
participant Server as HTTP/2 Server
Client->>Adapter: POST /api (httpVersion: 2, http2Options)
Adapter->>Sessions: getSession(authority, http2Options)
alt Session Exists
Sessions-->>Session: return cached session
else Session Missing
Sessions->>Session: createSecureConnection(authority)
Session-->>Sessions: session created
Sessions-->>Session: return session
end
Adapter->>Session: request(http2Headers, options)
Session->>Server: send HTTP/2 request
Server-->>Session: HTTP/2 response stream
Session-->>Adapter: response headers + stream
Adapter->>Adapter: map headers to standard format
Adapter-->>Client: resolve with response data
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@lib/adapters/http.js`:
- Around line 702-705: The code currently calls settle(resolve, abort, response)
when responseType === 'stream', which uses abort as the reject handler and
destroys the response stream; change this to settle(resolve, reject, response)
so status validation rejects without aborting the stream (keep abort usage only
for actual cancellation paths); update the call in the block handling
responseType === 'stream' (where response.data = responseStream) to use the
reject handler instead of abort.
- Around line 238-274: The authority string built in request() is incorrect: it
always uses port 80 and does not bracket IPv6 hostnames, which breaks https and
IPv6; update the authority construction (used by http2Sessions.getSession) to
choose the default port based on options.protocol ('https:' -> 443, otherwise 80
or options.port when provided) and ensure IPv6 hostnames (options.hostname
containing ':') are wrapped in square brackets before appending the port; keep
the rest of the logic (http2Options, headers, session.request) unchanged.
In `@package.json`:
- Line 149: The package.json dependency "selfsigned" is pinned to "^3.0.1" which
blocks newer releases; update the dependency entry for "selfsigned" to the
desired newer version (e.g., "^5.5.0"), run your package manager (npm install or
yarn install) to refresh lockfile, review the selfsigned changelog for breaking
changes between v3 and v5, adjust any call sites that use its API (search for
usages of "selfsigned" import/require) and run the test-suite and local
cert-generation flows to verify nothing breaks.
In `@README.md`:
- Line 90: The TOC entry for "🔥 HTTP2" is indented, violating MD007; remove the
leading spaces so the top-level list item is unindented (e.g., change the line
containing "- [🔥 HTTP2](`#-http2`)" to start at column 0) to satisfy
markdownlint's top-level list rule.
In `@test/unit/adapters/http.js`:
- Around line 2393-2496: The test "should support request cancellation"
currently only checks typeof AbortSignal !== 'function' but still calls
AbortSignal.timeout; update the guard to detect timeout support (e.g., check
typeof AbortSignal?.timeout === 'function') and if unavailable, use a fallback:
create an AbortController, call setTimeout(() => controller.abort(), 500) and
pass controller.signal to http2Axios.get; adjust the assertion to expect the
same canceled error and keep the existing stream-aborted checks (references: the
test block name, AbortSignal.timeout usage, AbortController fallback, and
http2Axios.get call).
🧹 Nitpick comments (2)
lib/adapters/http.js (1)
65-124: Separate pool-onlysessionTimeoutfromhttp2.connectoptions.
sessionTimeoutis used for HTTP/2 session idle timeout (viaHttp2Sessions.setTimeout) but currently passes through tohttp2.connect, where it's interpreted as a TLS session resumption timeout (different semantics and units). StripsessionTimeoutbefore passing tohttp2.connectwhile keeping it for pool comparisons and idle timeout management.♻️ Suggested adjustment
getSession(authority, options) { options = Object.assign({ sessionTimeout: 1000 }, options); + const connectOptions = {...options}; + delete connectOptions.sessionTimeout; ... - const session = connect(authority, options); + const session = connect(authority, connectOptions);test/unit/adapters/http.js (1)
2099-2131: Avoid stream.pipeline array form unless supported; consider restoring test timeout.Some Node versions don’t accept the array signature for
stream.pipeline, and removing the timeout can make this test flaky under slower CI. Please verify supported Node versions and consider the safer varargs form plus an explicit timeout.🔧 Suggested update
- //this.timeout(5000); + this.timeout(5000); ... - await assert.rejects(() => pipelineAsync([data, devNull()])); + await assert.rejects(() => pipelineAsync(data, devNull()));
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
README.mdindex.d.ctsindex.d.tslib/adapters/http.jspackage.jsontest/helpers/server.jstest/unit/adapters/http.js
🧰 Additional context used
🧬 Code graph analysis (1)
test/helpers/server.js (3)
lib/adapters/http.js (5)
options(241-241)options(566-577)req(255-255)req(285-285)session(243-243)test/unit/adapters/fetch.js (3)
res(273-273)res(355-355)server(23-23)test/unit/adapters/http.js (1)
server(13-13)
🪛 markdownlint-cli2 (0.20.0)
README.md
90-90: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
🔇 Additional comments (22)
lib/adapters/http.js (7)
1-44: No concerns spotted in the new import/constants block.
280-363: Validation and abort lifecycle look solid.
Nice to gate unsupported protocol versions and clean up listeners after stream completion.
565-610: Options wiring + transport selection are consistent.
Forwardinghttp2Optionsand switching transports based onhttpVersionis clear.
629-629: Safer content-length parsing.
UsingtoFiniteNumberavoids NaN propagation.
718-773: Unified abort cleanup for size breaches/cancel looks good.
The abort path now consistently tears down response/request streams.
795-823: Timeout paths now funnel through abort—nice consistency.
This keeps cancellation semantics uniform across failures.
849-851: Write + end flow for non-stream payloads is fine.README.md (1)
1706-1732: HTTP/2 documentation section is clear and actionable.
The example and option descriptions are easy to follow.test/helpers/server.js (2)
16-83: Nice refactor to centralize the server handler and session tracking.
This makes the test server setup cleaner and improves lifecycle management.
98-100: Good to close sessions during teardown.
Prevents lingering sessions from keeping tests alive.index.d.ts (1)
372-375: HTTP/2 configuration typings are correctly aligned with runtime implementation.The runtime code validates
httpVersionmust be 1 or 2 (matching the type definition), useshttpVersion = 1as the default, and properly handleshttp2Optionsin session management. Both fields are optional, preserving backward compatibility.index.d.cts (1)
439-442: CJS typings properly mirror the new HTTP/2 options.Both index.d.cts (lines 439-443) and index.d.ts (lines 372-376) contain identical declarations for
httpVersionandhttp2Options, including thesessionTimeoutproperty. The CJS declaration file is correctly aligned with the ESM types.test/unit/adapters/http.js (10)
13-13: Track the secondary server instance for cleanup.Nice addition to keep server2 in scope for teardown.
25-33: Shared test helpers import looks good.Centralizing server utilities and stream helpers improves consistency.
35-37: Dedicated port constants are clear.This makes the multi-server setup easier to reason about.
57-63: Tolerance helper change is fine.The updated range calculation reads cleanly.
73-77: Teardown now covers server2.Good cleanup hygiene for multi-server tests.
883-883: Stream test now uses the shared readable helper.Consistent with the new helper-driven workflow.
1999-2013: Rate-limit test tuning looks reasonable.Sampling and tolerance adjustments are clear.
2059-2061: Download rate-limit sampling mirrors upload nicely.The same tolerance pattern keeps tests consistent.
2275-2389: HTTP/2 baseline coverage and response-type matrix look solid.Good breadth across payloads and response types.
2498-2662: Session pooling tests are comprehensive.Nice coverage for authority and option-based session reuse.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| request(options, cb) { | ||
| const authority = options.protocol + '//' + options.hostname + ':' + (options.port || 80); | ||
|
|
||
| const {http2Options, headers} = options; | ||
|
|
||
| const session = http2Sessions.getSession(authority, http2Options); | ||
|
|
||
| const http2Headers = { | ||
| [HTTP2_HEADER_SCHEME]: options.protocol.replace(':', ''), | ||
| [HTTP2_HEADER_METHOD]: options.method, | ||
| [HTTP2_HEADER_PATH]: options.path, | ||
| } | ||
|
|
||
| utils.forEach(headers, (header, name) => { | ||
| name.charAt(0) !== ':' && (http2Headers[name] = header); | ||
| }); | ||
|
|
||
| const req = session.request(http2Headers); | ||
|
|
||
| req.once('response', (responseHeaders) => { | ||
| const response = req; //duplex | ||
|
|
||
| responseHeaders = Object.assign({}, responseHeaders); | ||
|
|
||
| const status = responseHeaders[HTTP2_HEADER_STATUS]; | ||
|
|
||
| delete responseHeaders[HTTP2_HEADER_STATUS]; | ||
|
|
||
| response.headers = responseHeaders; | ||
|
|
||
| response.statusCode = +status; | ||
|
|
||
| cb(response); | ||
| }) | ||
|
|
||
| return req; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Explore repository structure
git ls-files | head -20Repository: Signal65/axios-CodeRabbit
Length of output: 676
🏁 Script executed:
#!/bin/bash
# Understand the context of the http.js file
head -50 lib/adapters/http.jsRepository: Signal65/axios-CodeRabbit
Length of output: 1937
🏁 Script executed:
#!/bin/bash
# Check if there are tests for this http adapter
find . -type f -name "*.test.js" -o -name "*.spec.js" -o -name "test-*.js" | grep -i http | head -20Repository: Signal65/axios-CodeRabbit
Length of output: 51
🏁 Script executed:
#!/bin/bash
# Search for http2Sessions and getSession usage
rg "http2Sessions|getSession" -A 3 -B 3Repository: Signal65/axios-CodeRabbit
Length of output: 944
🏁 Script executed:
#!/bin/bash
# Search for how authority is used with http2
rg "session\.request\|http2\.connect" -A 5 -B 5Repository: Signal65/axios-CodeRabbit
Length of output: 51
🏁 Script executed:
#!/bin/bash
# Check the full context of the request method and surrounding code
sed -n '230,280p' lib/adapters/http.jsRepository: Signal65/axios-CodeRabbit
Length of output: 1571
🏁 Script executed:
#!/bin/bash
# Find the getSession method implementation
rg "getSession\(authority" -A 15Repository: Signal65/axios-CodeRabbit
Length of output: 1788
🏁 Script executed:
#!/bin/bash
# Search for http2.connect or where connect is called with authority
rg "connect\(" lib/adapters/http.js -A 5 -B 2Repository: Signal65/axios-CodeRabbit
Length of output: 250
🏁 Script executed:
#!/bin/bash
# Find the full Http2Sessions class implementation
sed -n '1,350p' lib/adapters/http.js | tail -100Repository: Signal65/axios-CodeRabbit
Length of output: 2967
🏁 Script executed:
#!/bin/bash
# Get more context from the Http2Sessions class
rg "class Http2Sessions" -A 40Repository: Signal65/axios-CodeRabbit
Length of output: 1978
🌐 Web query:
Node.js http2.connect() authority string format default port IPv6 brackets
💡 Result:
-
http2.connect(authority)expects a URL, not a rawhost:port: theauthorityargument must be a minimal, valid URL starting withhttp://orhttps://(any userinfo/path/query/fragment is ignored). (nodejs.org)- Examples:
http2.connect('https://example.com'),http2.connect('http://localhost:8080'). (nodejs.org)
- Examples:
-
Default port behavior:
- If you omit the port, it uses the scheme’s default: 80 for
http:, 443 forhttps:. (nodejs.org) - If you include a port that equals the default for that scheme, the URL model normalizes it away (the URL’s
.portbecomes''), i.e. “default port implied”. (nodejs.org)
- If you omit the port, it uses the scheme’s default: 80 for
-
IPv6 address formatting (brackets are required):
- In the URL authority/host component, an IPv6 literal must be enclosed in
[]to disambiguate the colons from the:portseparator. (greenbytes.de) - Examples:
http2.connect('https://[::1]')(implies port 443)http2.connect('http://[2001:db8::1]:8080')(greenbytes.de)
- In the URL authority/host component, an IPv6 literal must be enclosed in
Citations:
- 1: https://nodejs.org/download/release/v22.16.0/docs/api/http2.html?utm_source=openai
- 2: https://nodejs.org/download/release/v22.16.0/docs/api/http2.html?utm_source=openai
- 3: https://nodejs.org/api/url.html?utm_source=openai
- 4: https://nodejs.org/api/url.html?utm_source=openai
- 5: https://www.greenbytes.de/tech/webdav/draft-fielding-uri-rfc2396bis-02.html?utm_source=openai
- 6: https://www.greenbytes.de/tech/webdav/draft-fielding-uri-rfc2396bis-02.html?utm_source=openai
Fix authority construction for HTTPS defaults and IPv6 hosts.
Node.js http2.connect() infers the default port from the scheme (80 for http:, 443 for https:), but the current code hardcodes port 80 regardless of protocol. This breaks HTTPS when the port is omitted. Additionally, IPv6 hostnames must be enclosed in brackets in the URL authority component to disambiguate the colons from the port separator; without them, the authority string is malformed.
🐛 Proposed fix
- const authority = options.protocol + '//' + options.hostname + ':' + (options.port || 80);
+ const defaultPort = options.protocol === 'https:' ? 443 : 80;
+ const port = options.port || defaultPort;
+ const host = options.hostname.includes(':') ? `[${options.hostname}]` : options.hostname;
+ const authority = `${options.protocol}//${host}:${port}`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| request(options, cb) { | |
| const authority = options.protocol + '//' + options.hostname + ':' + (options.port || 80); | |
| const {http2Options, headers} = options; | |
| const session = http2Sessions.getSession(authority, http2Options); | |
| const http2Headers = { | |
| [HTTP2_HEADER_SCHEME]: options.protocol.replace(':', ''), | |
| [HTTP2_HEADER_METHOD]: options.method, | |
| [HTTP2_HEADER_PATH]: options.path, | |
| } | |
| utils.forEach(headers, (header, name) => { | |
| name.charAt(0) !== ':' && (http2Headers[name] = header); | |
| }); | |
| const req = session.request(http2Headers); | |
| req.once('response', (responseHeaders) => { | |
| const response = req; //duplex | |
| responseHeaders = Object.assign({}, responseHeaders); | |
| const status = responseHeaders[HTTP2_HEADER_STATUS]; | |
| delete responseHeaders[HTTP2_HEADER_STATUS]; | |
| response.headers = responseHeaders; | |
| response.statusCode = +status; | |
| cb(response); | |
| }) | |
| return req; | |
| } | |
| request(options, cb) { | |
| const defaultPort = options.protocol === 'https:' ? 443 : 80; | |
| const port = options.port || defaultPort; | |
| const host = options.hostname.includes(':') ? `[${options.hostname}]` : options.hostname; | |
| const authority = `${options.protocol}//${host}:${port}`; | |
| const {http2Options, headers} = options; | |
| const session = http2Sessions.getSession(authority, http2Options); | |
| const http2Headers = { | |
| [HTTP2_HEADER_SCHEME]: options.protocol.replace(':', ''), | |
| [HTTP2_HEADER_METHOD]: options.method, | |
| [HTTP2_HEADER_PATH]: options.path, | |
| } | |
| utils.forEach(headers, (header, name) => { | |
| name.charAt(0) !== ':' && (http2Headers[name] = header); | |
| }); | |
| const req = session.request(http2Headers); | |
| req.once('response', (responseHeaders) => { | |
| const response = req; //duplex | |
| responseHeaders = Object.assign({}, responseHeaders); | |
| const status = responseHeaders[HTTP2_HEADER_STATUS]; | |
| delete responseHeaders[HTTP2_HEADER_STATUS]; | |
| response.headers = responseHeaders; | |
| response.statusCode = +status; | |
| cb(response); | |
| }) | |
| return req; | |
| } |
🤖 Prompt for AI Agents
In `@lib/adapters/http.js` around lines 238 - 274, The authority string built in
request() is incorrect: it always uses port 80 and does not bracket IPv6
hostnames, which breaks https and IPv6; update the authority construction (used
by http2Sessions.getSession) to choose the default port based on
options.protocol ('https:' -> 443, otherwise 80 or options.port when provided)
and ensure IPv6 hostnames (options.hostname containing ':') are wrapped in
square brackets before appending the port; keep the rest of the logic
(http2Options, headers, session.request) unchanged.
| if (responseType === 'stream') { | ||
| response.data = responseStream; | ||
| settle(resolve, reject, response); | ||
| settle(resolve, abort, response); | ||
| } else { |
There was a problem hiding this comment.
Avoid aborting streams on non-2xx status.
Using abort as the reject handler destroys the response stream, preventing consumers from reading error bodies. Keep reject for status validation and reserve abort for cancellations.
🐛 Proposed fix
- settle(resolve, abort, response);
+ settle(resolve, reject, response);🤖 Prompt for AI Agents
In `@lib/adapters/http.js` around lines 702 - 705, The code currently calls
settle(resolve, abort, response) when responseType === 'stream', which uses
abort as the reject handler and destroys the response stream; change this to
settle(resolve, reject, response) so status validation rejects without aborting
the stream (keep abort usage only for actual cancellation paths); update the
call in the block handling responseType === 'stream' (where response.data =
responseStream) to use the reject handler instead of abort.
| "rollup-plugin-auto-external": "^2.0.0", | ||
| "rollup-plugin-bundle-size": "^1.0.3", | ||
| "rollup-plugin-terser": "^7.0.2", | ||
| "selfsigned": "^3.0.1", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
selfsigned npm latest version
💡 Result:
The latest version of the selfsigned npm package is 5.5.0 (as of January 29, 2026). [1] [2]
To verify locally:
npm view selfsigned versionSources: Snyk package page [1], jsDelivr package page [2].
🌐 Web query:
selfsigned npm security advisories CVE
💡 Result:
selfsigned(the npm package): as of the latest listings, there are no known/direct security advisories or CVEs recorded forselfsigneditself in major vulnerability databases (e.g., Snyk shows “No known security issues” forselfsigned, latest listed version 5.5.0). (security.snyk.io)- What people often hit in practice is vulnerabilities in transitive deps historically used under
selfsigned, especiallynode-forge(commonly pulled in via older stacks likewebpack-dev-server→selfsigned→node-forge). One example: CVE-2025-66031 (ASN.1 unbounded recursion DoS) affectsnode-forge< 1.3.2 and is fixed in 1.3.2. (github.com) - Separate but commonly-confused topic: npm’s historical “SELF_SIGNED_CERT_IN_CHAIN” install/publish errors relate to npm registry certificate handling changes (2014 blog post) and are not a CVE for the
selfsignedpackage. (blog.npmjs.org)
Citations:
- 1: https://security.snyk.io/package/npm/selfsigned?utm_source=openai
- 2: https://github.com/advisories/GHSA-554w-wpv2-vw27?utm_source=openai
- 3: https://blog.npmjs.org/post/78085451721/npms-self-signed-certificate-is-no-more?utm_source=openai
🌐 Web query:
selfsigned 3.0.1 release date changelog
💡 Result:
- Release date (selfsigned v3.0.1): July 23, 2025 (Git tag
v3.0.1, commita6bf3f0). [1] - Changelog for v3.0.1: The project’s
CHANGELOG.mddoes not list changes for3.0.1; it only documents5.0.0in detail and notes for4.x and earlier(including3.0.1) to “see git history.” [2] - Where to see actual changes: Compare tags/commits around
v3.0.0(Jul 21, 2025) →v3.0.1(Jul 23, 2025) in the repo history/tags. [1]
Sources: [1] [2]
Update selfsigned to a current version.
Version 3.0.1 (released July 2025) is significantly outdated; latest is 5.5.0. While no direct security advisories exist for selfsigned itself, the version constraint using ^3.0.1 prevents access to newer releases. Review the changelog for major version differences and test the upgrade path.
🤖 Prompt for AI Agents
In `@package.json` at line 149, The package.json dependency "selfsigned" is pinned
to "^3.0.1" which blocks newer releases; update the dependency entry for
"selfsigned" to the desired newer version (e.g., "^5.5.0"), run your package
manager (npm install or yarn install) to refresh lockfile, review the selfsigned
changelog for breaking changes between v3 and v5, adjust any call sites that use
its API (search for usages of "selfsigned" import/require) and run the
test-suite and local cert-generation flows to verify nothing breaks.
| - [🔥 Custom fetch](#-custom-fetch) | ||
| - [🔥 Using with Tauri](#-using-with-tauri) | ||
| - [🔥 Using with SvelteKit](#-using-with-sveltekit-) | ||
| - [🔥 HTTP2](#-http2) |
There was a problem hiding this comment.
Fix markdownlint TOC indentation.
MD007 expects top‑level list items to be unindented.
✏️ Suggested fix
- - [🔥 HTTP2](`#-http2`)
+ - [🔥 HTTP2](`#-http2`)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - [🔥 HTTP2](#-http2) | |
| - [🔥 HTTP2](`#-http2`) |
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
90-90: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
🤖 Prompt for AI Agents
In `@README.md` at line 90, The TOC entry for "🔥 HTTP2" is indented, violating
MD007; remove the leading spaces so the top-level list item is unindented (e.g.,
change the line containing "- [🔥 HTTP2](`#-http2`)" to start at column 0) to
satisfy markdownlint's top-level list rule.
| it('should support request timeout', async () => { | ||
| let isAborted= false; | ||
|
|
||
| let aborted; | ||
| const promise = new Promise(resolve => aborted = resolve); | ||
|
|
||
| server = await startHTTPServer((req, res) => { | ||
| setTimeout(() => { | ||
| res.end('OK'); | ||
| }, 15000); | ||
| }, { | ||
| useHTTP2: true | ||
| }); | ||
|
|
||
| server.on('stream', (stream) => { | ||
| stream.once('aborted', () => { | ||
| isAborted = true; | ||
| aborted(); | ||
| }); | ||
| }); | ||
|
|
||
| await assert.rejects(async () => { | ||
| await http2Axios.get(LOCAL_SERVER_URL, { | ||
| timeout: 500 | ||
| }); | ||
| }, /timeout/); | ||
|
|
||
| await promise; | ||
|
|
||
| assert.ok(isAborted); | ||
| }); | ||
|
|
||
| it('should support request cancellation', async function (){ | ||
| if (typeof AbortSignal !== 'function') { | ||
| this.skip(); | ||
| } | ||
|
|
||
| let isAborted= false; | ||
|
|
||
| let aborted; | ||
| const promise = new Promise(resolve => aborted = resolve); | ||
|
|
||
| server = await startHTTPServer((req, res) => { | ||
| setTimeout(() => { | ||
| res.end('OK'); | ||
| }, 15000); | ||
| }, { | ||
| useHTTP2: true | ||
| }); | ||
|
|
||
| server.on('stream', (stream) => { | ||
| stream.once('aborted', () => { | ||
| isAborted = true; | ||
| aborted(); | ||
| }); | ||
| }); | ||
|
|
||
| await assert.rejects(async () => { | ||
| await http2Axios.get(LOCAL_SERVER_URL, { | ||
| signal: AbortSignal.timeout(500) | ||
| }); | ||
| }, /CanceledError: canceled/); | ||
|
|
||
| await promise; | ||
|
|
||
| assert.ok(isAborted); | ||
| }); | ||
|
|
||
| it('should support stream response cancellation', async () => { | ||
| let isAborted= false; | ||
| var source = axios.CancelToken.source(); | ||
|
|
||
| let aborted; | ||
| const promise = new Promise(resolve => aborted = resolve); | ||
|
|
||
| server = await startHTTPServer((req, res) => { | ||
| generateReadable(10000, 100, 100).pipe(res); | ||
| }, { | ||
| useHTTP2: true | ||
| }); | ||
|
|
||
| server.on('stream', (stream) => { | ||
| stream.once('aborted', () => { | ||
| isAborted = true; | ||
| aborted(); | ||
| }); | ||
| }); | ||
|
|
||
| const {data} = await http2Axios.get(LOCAL_SERVER_URL, { | ||
| cancelToken: source.token, | ||
| responseType: 'stream' | ||
| }); | ||
|
|
||
| setTimeout(() => source.cancel()); | ||
|
|
||
| await assert.rejects( | ||
| () => pipelineAsync([data, devNull()]), | ||
| /CanceledError: canceled/ | ||
| ) | ||
|
|
||
| await promise; | ||
|
|
||
| assert.ok(isAborted); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the test file to see the actual implementation
head -c 5000 test/unit/adapters/http.js | tail -c 2000Repository: Signal65/axios-CodeRabbit
Length of output: 2069
🏁 Script executed:
# Check Node version requirements in package.json
cat package.json | grep -A 5 "engines"Repository: Signal65/axios-CodeRabbit
Length of output: 51
🏁 Script executed:
# Search for any existing AbortSignal.timeout usage in codebase
rg "AbortSignal.timeout" -A 3 -B 3Repository: Signal65/axios-CodeRabbit
Length of output: 441
🏁 Script executed:
# Check for any existing version guards or fallbacks in the test file
rg "AbortSignal" test/unit/adapters/http.js -B 5 -A 5Repository: Signal65/axios-CodeRabbit
Length of output: 508
🏁 Script executed:
# Search for other version guards or conditional checks in the test suite
rg "typeof.*function|AbortController|abortSignal" test/ --type js -A 2 -B 2 | head -100Repository: Signal65/axios-CodeRabbit
Length of output: 5266
🏁 Script executed:
cat package.json | head -50Repository: Signal65/axios-CodeRabbit
Length of output: 2091
🏁 Script executed:
# Also check for any existing patterns of guarding AbortSignal.timeout in other files
rg "AbortSignal.*timeout|AbortController.*setTimeout" --type js -B 3 -A 3Repository: Signal65/axios-CodeRabbit
Length of output: 808
🏁 Script executed:
# Check what Node versions are being tested
cat .github/workflows/*.yml 2>/dev/null | grep -i "node" | head -20Repository: Signal65/axios-CodeRabbit
Length of output: 1002
🌐 Web query:
AbortSignal.timeout Node.js minimum version support 17.3.0
💡 Result:
AbortSignal.timeout() is supported in Node.js >= 16.14.0 (backported) and >= 17.3.0 (first introduced on the 17.x line). The minimum Node.js version that supports it is 16.14.0. [1]
🏁 Script executed:
# Let's also check the actual test file to see the current structure around line 2430
sed -n '2420,2450p' test/unit/adapters/http.jsRepository: Signal65/axios-CodeRabbit
Length of output: 744
Guard AbortSignal.timeout for supported Node versions.
AbortSignal.timeout is only supported in Node.js 16.14.0+. The current guard only checks typeof AbortSignal !== 'function', which will pass on Node 16.0–16.13.x where AbortSignal exists but AbortSignal.timeout does not. This will cause the test to fail with "AbortSignal.timeout is not a function" on those versions.
Add a guard like typeof AbortSignal?.timeout === 'function' or use the suggested fallback with AbortController and setTimeout.
🤖 Prompt for AI Agents
In `@test/unit/adapters/http.js` around lines 2393 - 2496, The test "should
support request cancellation" currently only checks typeof AbortSignal !==
'function' but still calls AbortSignal.timeout; update the guard to detect
timeout support (e.g., check typeof AbortSignal?.timeout === 'function') and if
unavailable, use a fallback: create an AbortController, call setTimeout(() =>
controller.abort(), 500) and pass controller.signal to http2Axios.get; adjust
the assertion to expect the same canceled error and keep the existing
stream-aborted checks (references: the test block name, AbortSignal.timeout
usage, AbortController fallback, and http2Axios.get call).
2 participants
PR_021
Summary by CodeRabbit
New Features
httpVersionconfiguration option to select HTTP/1 or HTTP/2http2Optionsto customize HTTP/2 session behavior and timeout settingsDocumentation
✏️ Tip: You can customize this high-level summary in your review settings.