[sogav21]

Checklist

• npm test passes
• tests and/or benchmarks are included
• commit message follows commit guidelines

Affected core subsystem(s)

HTTP and HTTPS agent timeout initialization.

Description of change

With freeSocketTimeout: '10s' and no explicit timeout, calculating the default before parsing the duration produces NaN instead of 20000, disabling the intended active socket inactivity timeout. Normalize the effective free socket timeout after applying legacy aliases, then derive the default from milliseconds.

Add HTTP/HTTPS coverage for duration strings, the 8000ms minimum, both legacy aliases and their precedence, numeric/default values, and explicit timeout overrides including zero. Real local HTTP/HTTPS hanging-request tests verify ERR_SOCKET_TIMEOUT.

Validation: the new constructor regressions fail against the original code (14 failures); npm test passes with 90 passing and 1 existing pending test on Node.js v22.23.1 / Windows.

Closes #125

Summary by CodeRabbit

• Bug Fixes
  • Corrected how default request timeouts are calculated when free-socket timeouts are provided as human-readable strings or numbers.
  • Preserved explicitly configured timeouts and legacy timeout-option precedence, including when values are zero or undefined.
  • Added coverage for HTTP and HTTPS agents, including verification that timed-out requests report the expected timeout value.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The constructor now normalizes freeSocketTimeout before deriving the default timeout. Tests cover HTTP and HTTPS agents, legacy timeout aliases, explicit timeout values, and active request timeout errors.

Changes: Agent timeout defaults

Layer / File(s)	Summary
Normalize timeout before deriving the default lib/agent.js, test/default_timeout.test.js	The constructor converts freeSocketTimeout before calculating the default timeout. Tests cover human-readable, numeric, zero, and undefined values; legacy option precedence; explicit timeouts; and HTTP and HTTPS active request timeout errors.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Possibly related PRs

• node-modules/agentkeepalive#124: Clones supplied options before constructor defaults and normalization, which relates to the constructor option processing changed here.

Suggested reviewers: fengmk2

Merge Risk: Minimal · up to c2538

The change addresses duration-string defaults; no merge-blocking issue is evident from the supplied evidence.

Pre-merge checks: 5

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: deriving the active timeout from the normalized free socket timeout.
Linked Issues check		Issue #125 requires normalization of freeSocketTimeout before deriving the default active timeout. lib/agent.js applies ms() after legacy alias resolution and derives `Math.max(options.freeSocke…
Out of Scope Changes check		The changes are limited to the timeout initialization in lib/agent.js and tests for the behavior required by issue #125. The HTTP and HTTPS hanging-request tests directly verify the reported runtime…
Docstring Coverage		No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…

Full details: Linked Issues check

Explanation

Issue #125 requires normalization of freeSocketTimeout before deriving the default active timeout. lib/agent.js applies ms() after legacy alias resolution and derives Math.max(options.freeSocketTimeout * 2, 8000). The change covers keepAliveTimeout and freeSocketKeepAliveTimeout. test/default_timeout.test.js covers HTTP and HTTPS, duration strings, the 8000 ms minimum, alias precedence, numeric and default values, explicit timeouts including zero, and runtime ERR_SOCKET_TIMEOUT behavior.

Full details: Out of Scope Changes check

Explanation

The changes are limited to the timeout initialization in lib/agent.js and tests for the behavior required by issue #125. The HTTP and HTTPS hanging-request tests directly verify the reported runtime effect. No unrelated product behavior or unrelated files are changed.

Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 2 files.

Finishing Touches

Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 9f674d40-18fc-4d00-9df2-1ccd370ecef8

Commits

Reviewing files that changed from the base of the PR and between 422e9fc and c25386b.

Files selected for processing: 2

• lib/agent.js
• test/default_timeout.test.js

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.