Skip to content

fix(h1): complete paused parser on socket end instead of crashing#5474

Merged
mcollina merged 1 commit into
nodejs:mainfrom
nxtedition:claude/finish-paused-parser-5364
Jul 1, 2026
Merged

fix(h1): complete paused parser on socket end instead of crashing#5474
mcollina merged 1 commit into
nodejs:mainfrom
nxtedition:claude/finish-paused-parser-5364

Conversation

@ronag

@ronag ronag commented Jul 1, 2026

Copy link
Copy Markdown
Member

When a response body applies backpressure, the HTTP/1 llhttp parser is
left paused. If the peer then closes the socket, onHttpSocketEnd calls
parser.finish(), which asserted !this.paused and crashed the process
with an uncatchable AssertionError from the socket 'end' handler.

finish() now resumes a paused parser and drains the socket through it so
the response completes correctly across all body framings:

  • Content-Length / chunked bodies reach on_message_complete during
    execute() (driving the parser past the body is required; calling
    llhttp_finish() alone leaves them hanging).
  • EOF-delimited bodies (no length) can't complete via execute() and
    re-pause, so we resume once more and let llhttp_finish() deliver the
    EOF completion.
  • Backpressure is advisory here (onData keeps buffering delivered bytes),
    so we resume across pauses and loop until the socket buffer is empty,
    rather than parsing only a single read().

Truncated responses still surface as errors, never a false completion:
short Content-Length -> UND_ERR_RES_CONTENT_LENGTH_MISMATCH, unterminated
chunked -> HTTPParserError.

Fixes #5360. Supersedes #5364, whose fix hangs
Content-Length bodies (resume without execute) and whose test passes
without the fix (the 'data' listener switches the stream to flowing mode
so the parser never pauses). The tests here keep the body unconsumed
until after FIN and cover Content-Length/EOF completion plus
Content-Length/chunked truncation; all four fail against the unpatched
parser.

🤖 Generated with Claude Code

When a response body applies backpressure, the HTTP/1 llhttp parser is
left paused. If the peer then closes the socket, onHttpSocketEnd calls
parser.finish(), which asserted `!this.paused` and crashed the process
with an uncatchable AssertionError from the socket 'end' handler.

finish() now resumes a paused parser and drains the socket through it so
the response completes correctly across all body framings:

- Content-Length / chunked bodies reach on_message_complete during
  execute() (driving the parser past the body is required; calling
  llhttp_finish() alone leaves them hanging).
- EOF-delimited bodies (no length) can't complete via execute() and
  re-pause, so we resume once more and let llhttp_finish() deliver the
  EOF completion.
- Backpressure is advisory here (onData keeps buffering delivered bytes),
  so we resume across pauses and loop until the socket buffer is empty,
  rather than parsing only a single read().

Truncated responses still surface as errors, never a false completion:
short Content-Length -> UND_ERR_RES_CONTENT_LENGTH_MISMATCH, unterminated
chunked -> HTTPParserError.

Fixes nodejs#5360. Supersedes nodejs#5364, whose fix hangs
Content-Length bodies (resume without execute) and whose test passes
without the fix (the 'data' listener switches the stream to flowing mode
so the parser never pauses). The tests here keep the body unconsumed
until after FIN and cover Content-Length/EOF completion plus
Content-Length/chunked truncation; all four fail against the unpatched
parser.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.45%. Comparing base (ce7bf4f) to head (c52fa90).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5474      +/-   ##
==========================================
- Coverage   93.46%   93.45%   -0.01%     
==========================================
  Files         110      110              
  Lines       37124    37147      +23     
==========================================
+ Hits        34698    34717      +19     
- Misses       2426     2430       +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.

@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 7d78f7d into nodejs:main Jul 1, 2026
38 checks passed
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.

Uncatchable AssertionError: assert(!this.paused) on socket end

3 participants