Skip to content

test: absorb h2 stream timeout resets#5383

Merged
mcollina merged 1 commit into
nodejs:mainfrom
marko1olo:fix-issue-5137-h2-reset-teardown
Jun 8, 2026
Merged

test: absorb h2 stream timeout resets#5383
mcollina merged 1 commit into
nodejs:mainfrom
marko1olo:fix-issue-5137-h2-reset-teardown

Conversation

@marko1olo

Copy link
Copy Markdown
Contributor

Summary

  • Swallow expected h2c stream/session/socket reset errors in the issue-5137 timeout regression test.
  • Close the RetryAgent inside the test body so teardown happens before node:test reports late asynchronous activity.
  • Await server close from the test cleanup hook.

Why

A macOS Node 25 CI run reported test/issue-5137.js generating asynchronous activity after the test ended with Error: read ECONNRESET. The test intentionally lets every HTTP/2 stream time out, so client-side resets are expected teardown noise rather than the regression under test.

Test

  • node --test test\issue-5137.js
  • 1..5 | ForEach-Object { node --test test\issue-5137.js }
  • npx eslint --no-cache test\issue-5137.js
  • git diff --check

AI-assisted contribution: implementation and verification were performed with OpenAI Codex under my direction.

Signed-off-by: marko1olo <barsukdana@gmail.com>
@codecov-commenter

codecov-commenter commented Jun 7, 2026

Copy link
Copy Markdown

Codecov Report

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

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5383      +/-   ##
==========================================
+ Coverage   93.25%   93.26%   +0.01%     
==========================================
  Files         110      110              
  Lines       36752    36752              
==========================================
+ Hits        34274    34278       +4     
+ Misses       2478     2474       -4     

☔ 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.

@AliMahmoudDev

Copy link
Copy Markdown
Contributor

Good call closing the RetryAgent inside the test body and awaiting server close from cleanup. Those late ECONNRESET errors from timed-out h2 streams are expected — swallowing them in the cleanup hook is the right approach since the test intentionally lets every stream time out.

@mcollina

mcollina commented Jun 8, 2026

Copy link
Copy Markdown
Member

@AliMahmoudDev @marko1olo it seems this is part of a Bot/automation campaign. Slow down and review all changes manually. Purely automated contributions are not welcomed.

@AliMahmoudDev

Copy link
Copy Markdown
Contributor

fair point @mcollina — i'll slow down on the reviews and make sure to test changes manually before commenting. no intention to come across as automated.

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mcollina mcollina merged commit e27be2a into nodejs:main Jun 8, 2026
38 checks passed
@github-actions github-actions Bot mentioned this pull request Jun 15, 2026
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.

4 participants