Skip to content

test: harden http2-request-never-settles against late session errors - #5674

Open
pacocartones wants to merge 1 commit into
nodejs:mainfrom
pacocartones:fix/harden-http2-never-settles
Open

test: harden http2-request-never-settles against late session errors#5674
pacocartones wants to merge 1 commit into
nodejs:mainfrom
pacocartones:fix/harden-http2-never-settles

Conversation

@pacocartones

@pacocartones pacocartones commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

This relates to...

The flaky test/http2-request-never-settles.js (added in #5603), currently failing intermittently on main and painting unrelated PRs red. Update 2026-08-09: root cause identified upstream — this is nodejs/node#64841, a V8 Maglev SIGSEGV regression in Node ≥ v24.15.0, not an undici bug. This PR no longer claims to fix the red CI; see the verification section.

Rationale

Since #5603 merged, this test fails intermittently in CI with a distinctive signature: ✖ test/http2-request-never-settles.js 'test failed' with no TAP output, no subtests, no stack, the file finishing in 3–9 s so the 15 s watchdog never runs. That signature is the test process dying abruptly, with the TAP stream discarded.

The original hypothesis of this PR was a late unhandled 'error' on the churn server (the only emitter without a handler). That hypothesis has now been falsified as the cause of the red CI (see below): the killer is a native crash, not a JS exception. The handlers remain correct hygiene for a churn server — an unhandled 'error' would kill the process (with a visible stack, unlike the flake) — but they do not cure the flake.

Changes

Test-only (+9/−0), copying the established pattern from test/http2-abort.js:

  • server.on('error', …), session.on('error', …) and session.socket?.on('error', …) (tracked per session), and server.on('secureConnection', …) — late errors are swallowed instead of killing the process.
  • A comment explaining why, so the next reader doesn't "clean them up".

No library code touched; the settle invariant under test is unchanged.

Verification

Red→green falsification: NEGATIVE — this hardening does not fix the flake. Reproduced locally on WSL2 (Ubuntu 24.04) using the exact recipe from nodejs/node#64841 (batches of 6 processes, taskset -c 0,1), on undici 86b62998:

Config Result
v24.18.1, original test 1/72 and 4/72 SIGSEGV (exit 139) in two independent run sets
v24.18.1, test with this PR 1/72 SIGSEGV — same rate, same signature
v24.19.0 (current LTS), original test 4/72 SIGSEGV — the V8 backport in 24.19.0 does not cure it
v24.18.1 --no-maglev 0/48 SIGSEGV, but 1/48 SIGABRT (glibc corrupted size vs. prev_size) — not a clean stopgap
Controls: v24.14.1 (336 runs incl. ~30k amplified requests), v24.18.1 on Windows (72 runs) 0 failures — matches the issue being Linux-only, Node ≥ 24.15.0

The crash log is byte-identical to the CI signature: a single TAP line (✔ every h2 request settles under connection churn (seed 7)) and then sudden death — no error, no stack, exit 139. A probe test dying by an unhandled EventEmitter 'error' does show its stack under the test runner, confirming the CI killer is a signal death (SIGSEGV in Maglev concurrent compilation), not a JS exception.

What this means, honestly:

Status

The churn server is the only emitter in the setup without an 'error'
handler; a late ECONNRESET (client aborts, GOAWAYs, destroyed sessions,
agent.destroy()) after the test body resolves kills the process, and CI
reports a bare 'test failed' with no TAP. Swallow them like
test/http2-abort.js already does.

Signed-off-by: pacocartones <manusanchezhl@gmail.com>
@codecov-commenter

codecov-commenter commented Aug 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.44%. Comparing base (dd85997) to head (d8de0c4).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5674   +/-   ##
=======================================
  Coverage   93.43%   93.44%           
=======================================
  Files         110      110           
  Lines       38733    38776   +43     
=======================================
+ Hits        36190    36234   +44     
+ Misses       2543     2542    -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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