refactor(http): extract Http2Sessions into its own module by carladams1299-lab · Pull Request #10861 · axios/axios

carladams1299-lab · 2026-05-06T23:08:09Z

Summary

Extract the Http2Sessions class from lib/adapters/http.js into its own
module at lib/helpers/Http2Sessions.js and add direct unit tests.

The class is self-contained — it only depends on http2 and util, with no
closure over per-request state — so the move requires no parameter threading
and no behavior change. The module-level http2Sessions singleton stays in
http.js as a one-line instantiation. Pulling the class out gives a real
unit-test seam for session-pooling, timeout, and cleanup logic that was
previously only exercised end-to-end through the adapter.

Aligns with AGENTS.md guidance that lib/helpers/ should hold pieces that
are generic and reusable outside axios — Http2Sessions qualifies; nothing
in it knows about axios.

Linked issue

Closes #10860

Changes

New file lib/helpers/Http2Sessions.js — class moved verbatim, with its own http2/util imports and a default export.
lib/adapters/http.js: added import Http2Sessions from '../helpers/Http2Sessions.js', removed the inline class definition (~102 lines).
New file tests/unit/helpers/Http2Sessions.test.js — 12 unit tests covering cache reuse, options-mismatch invalidation, skipping destroyed/closed cached sessions, removal on session close, sessionTimeout close-after-idle, timeout cancellation when a new stream opens, multi-stream lifetime, default 1000ms timeout, and the sessionTimeout: null no-wrapper path.

Checklist

Tests added or updated (or N/A with reason) — 12 new direct unit tests for Http2Sessions; npm run test:vitest:unit reports 49 files / 709 tests passing.
Docs / types updated if public API changed (index.d.ts and index.d.cts) — N/A, internal refactor only; no public API, type, or error-code changes.
No breaking changes (or called out explicitly above) — pure code move; http2Sessions singleton, behavior, and call sites unchanged.

Summary by cubic

Extracted the Http2Sessions class into lib/helpers/Http2Sessions.js for direct unit testing and cleaned up session idle timers on close; no public API changes.

Description

Summary of changes
- Moved Http2Sessions to lib/helpers/Http2Sessions.js and imported it in lib/adapters/http.js; the adapter-level singleton remains.
- Fixed: cancel pending idle timer when a session close event fires.
- Added 'use strict' and a Node-only note; relies on http2 and util.
Reasoning
- Improves testability and structure.
- Ensures proper cleanup and clarifies runtime boundaries.
Additional context
- Behavior for consumers is unchanged; internal refactor plus a small robustness fix.

Docs

No user-facing changes. Optionally add a maintainer note in /docs/ that Http2Sessions lives under lib/helpers/.

Testing

Added tests/unit/helpers/Http2Sessions.test.js with 14 unit tests covering cache reuse, option mismatches, destroyed/closed handling, removal on close, idle timeout defaults and behavior, cancel on new stream, multi-stream lifecycle, the sessionTimeout: null path, and clearing the idle timer when the session closes.

Semantic version impact

Patch: internal refactor and non-breaking bug fix.

^{Written for commit b827625. Summary will update on new commits. Review in cubic}

…tests

cubic-dev-ai

2 issues found across 3 files

Confidence score: 3/5

There is a concrete runtime risk in lib/helpers/Http2Sessions.js: the idle-session timer is not cleared on session close, so a ref'ed Timeout may keep Node’s event loop alive longer than expected.
The test coverage concern in tests/unit/helpers/Http2Sessions.test.js means a wrapper added during the first getSession() call could be missed, reducing confidence that this behavior is fully guarded against regression.
Given the medium severity (5/10) with high confidence, this looks like a manageable but real risk rather than a merge blocker.
Pay close attention to lib/helpers/Http2Sessions.js, tests/unit/helpers/Http2Sessions.test.js - timer cleanup on close and first-call assertion coverage are the key risk areas.

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="lib/helpers/Http2Sessions.js">

<violation number="1" location="lib/helpers/Http2Sessions.js:95">
P2: Idle-session timer is never cleared when the session closes, so the ref'ed Timeout can keep the event loop alive until it expires.</violation>
</file>

<file name="tests/unit/helpers/Http2Sessions.test.js">

<violation number="1" location="tests/unit/helpers/Http2Sessions.test.js:165">
P2: The null-timeout test only compares `session.request` after a second lookup, so it would miss a wrapper installed during the first `getSession()` call.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

carladams1299-lab · 2026-05-06T23:24:47Z

@cubic-dev-ai

cubic-dev-ai · 2026-05-06T23:25:02Z

@cubic-dev-ai

@carladams1299-lab I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai

No issues found across 3 files

Confidence score: 5/5

Automated review surfaced no issues in the provided summaries.
No files require special attention.

jasonsaayman

Thanks for the PR @carladams1299-lab, just the two issues

…timer test

carladams1299-lab · 2026-05-07T18:24:46Z

Hi, @jasonsaayman ,
Thank you for your time.
I've just fixed issues.
I am a new contributor here.
I think there are some refactoring issues in this project.
Can I create refactoring issues?
Or else, could you please let me know your current contributing direction?

jasonsaayman

Looks good to me 🔥

* refactor(http): extract Http2Sessions to lib/helpers for direct unit tests * fix(http2): clear pending idle timer when session closes * fix(http2): address review — strict mode, Node-only note, observable timer test --------- Co-authored-by: Jay <[email protected]>

refactor(http): extract Http2Sessions to lib/helpers for direct unit …

7d98187

…tests

carladams1299-lab requested a review from jasonsaayman as a code owner May 6, 2026 23:08

cubic-dev-ai Bot reviewed May 6, 2026

View reviewed changes

Comment thread lib/helpers/Http2Sessions.js

Comment thread tests/unit/helpers/Http2Sessions.test.js Outdated

fix(http2): clear pending idle timer when session closes

7ffdb33

cubic-dev-ai Bot reviewed May 6, 2026

View reviewed changes

jasonsaayman requested changes May 7, 2026

View reviewed changes

Comment thread lib/helpers/Http2Sessions.js

Comment thread tests/unit/helpers/Http2Sessions.test.js Outdated

jasonsaayman assigned carladams1299-lab May 7, 2026

jasonsaayman added priority::medium A medium priority commit::refactor The PR is related to refactoring status::changes-requested A reviewer requested changes to the PR labels May 7, 2026

fix(http2): address review — strict mode, Node-only note, observable …

2de099d

…timer test

carladams1299-lab requested a review from jasonsaayman May 7, 2026 18:45

jasonsaayman changed the title ~~refactor(http): extract Http2Sessions to lib/helpers for direct unit …~~ refactor(http): extract Http2Sessions into its own module May 22, 2026

Merge branch 'v1.x' into refactor/extract-http2sessions

b827625

jasonsaayman approved these changes May 22, 2026

View reviewed changes

jasonsaayman merged commit 315ec44 into axios:v1.x May 22, 2026
26 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor(http): extract Http2Sessions into its own module#10861

refactor(http): extract Http2Sessions into its own module#10861
jasonsaayman merged 4 commits into
axios:v1.xfrom
carladams1299-lab:refactor/extract-http2sessions

carladams1299-lab commented May 6, 2026 •

edited by cubic-dev-ai Bot

Loading

Uh oh!

cubic-dev-ai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

carladams1299-lab commented May 6, 2026

Uh oh!

cubic-dev-ai Bot commented May 6, 2026

Uh oh!

cubic-dev-ai Bot left a comment

Uh oh!

jasonsaayman left a comment

Uh oh!

Uh oh!

Uh oh!

carladams1299-lab commented May 7, 2026 •

edited

Loading

Uh oh!

jasonsaayman left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

carladams1299-lab commented May 6, 2026 • edited by cubic-dev-ai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Linked issue

Changes

Checklist

Summary by cubic

Description

Docs

Testing

Semantic version impact

Uh oh!

cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

carladams1299-lab commented May 6, 2026

Uh oh!

cubic-dev-ai Bot commented May 6, 2026

Uh oh!

cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

jasonsaayman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

carladams1299-lab commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jasonsaayman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

carladams1299-lab commented May 6, 2026 •

edited by cubic-dev-ai Bot

Loading

carladams1299-lab commented May 7, 2026 •

edited

Loading