Skip to content

http2: emit close for aborted HEAD compat responses - #66310

Open
ronag wants to merge 1 commit into
nodejs:mainfrom
nxtedition:http2-compat-head-close
Open

ronag wants to merge 1 commit into
nodejs:mainfrom
nxtedition:http2-compat-head-close

Conversation

@ronag

@ronag ronag commented Sep 26, 2026

Copy link
Copy Markdown
Member

The compat response defers 'finish' and 'close' for a HEAD request until response.end(), because the stream of a headers-only response closes as soon as the headers are sent. The same deferral also applied to a HEAD stream that closed before any response was sent, for example when the client cancelled it or the session was destroyed. Nothing was left to call end(), so the response never emitted 'close' and the abort could not be observed on it.

Defer only once the headers were sent, and otherwise close the response as for any other method. The writable side of a HEAD stream is finished from the start, so 'finish' is emitted only after the headers were sent, and an aborted HEAD response does not report success.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http2
  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added http2 Issues and PRs related to the http2 subsystem. needs-ci PRs that need a full CI run. labels Sep 26, 2026
@ronag
ronag requested a review from mcollina September 26, 2026 10:27
@ronag
ronag force-pushed the http2-compat-head-close branch from 6f1a425 to 3e43327 Compare September 26, 2026 10:53
The compat response defers 'finish' and 'close' for a HEAD request
until response.end(), because the stream of a headers-only response
closes as soon as the headers are sent. The same deferral also applied
to a HEAD stream that closed before any response was sent, for example
when the client cancelled it or the session was destroyed. Nothing was
left to call end(), so the response never emitted 'close' and the abort
could not be observed on it.

Defer only once the headers were sent, and otherwise close the response
as for any other method. The writable side of a HEAD stream is finished
from the start, so 'finish' is emitted only after the headers were
sent, and an aborted HEAD response does not report success.

Assisted-by: Opus 5.5
Signed-off-by: Robert Nagy <ronagy@icloud.com>
@ronag
ronag force-pushed the http2-compat-head-close branch from 3e43327 to 8aed10d Compare September 26, 2026 10:53
@codecov

codecov Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.38%. Comparing base (45fef37) to head (8aed10d).
⚠️ Report is 30 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #66310   +/-   ##
=======================================
  Coverage   90.37%   90.38%           
=======================================
  Files         790      790           
  Lines      274497   274505    +8     
  Branches    52558    52555    -3     
=======================================
+ Hits       248065   248099   +34     
+ Misses      16922    16883   -39     
- Partials     9510     9523   +13     
Files with missing lines Coverage Δ
lib/internal/http2/compat.js 97.23% <100.00%> (+0.02%) ⬆️

... and 34 files with indirect coverage changes

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

@ronag
ronag requested a review from jasnell September 26, 2026 13:05
@ronag ronag added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 26, 2026

@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

@ronag ronag added the commit-queue PRs queued for automated landing through the Commit Queue. label Sep 26, 2026
@panva panva added author ready PRs with CI started, the required approvals, and no outstanding review comments. request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. and removed request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. commit-queue PRs queued for automated landing through the Commit Queue. labels Sep 26, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 26, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@panva panva added the commit-queue PRs queued for automated landing through the Commit Queue. label Sep 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. commit-queue PRs queued for automated landing through the Commit Queue. http2 Issues and PRs related to the http2 subsystem. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants