Skip to content

fix(dispatcher): remove unreachable assert in writeBlob#5231

Merged
mcollina merged 1 commit intonodejs:mainfrom
SAY-5:sayassh/issue-5153-deadcode-writeblob-assert
May 6, 2026
Merged

fix(dispatcher): remove unreachable assert in writeBlob#5231
mcollina merged 1 commit intonodejs:mainfrom
SAY-5:sayassh/issue-5153-deadcode-writeblob-assert

Conversation

@SAY-5
Copy link
Copy Markdown
Contributor

@SAY-5 SAY-5 commented May 6, 2026

This relates to...

Closes #5153

Rationale

The assert(contentLength === body.size, ...) at the top of writeBlob (in both client-h1.js and client-h2.js) can never fire: writeH1/writeH2 always overwrite contentLength with body.size via util.bodyLength before invoking writeBlob for blob-like bodies. The follow-up if (contentLength != null && contentLength !== body.size) guard is also unreachable today, but is left in place as a defensive net for future refactors of the upstream contentLength computation, matching option 2 in the issue.

Changes

Features

N/A

Bug Fixes

  • Drop the unreachable assert(contentLength === body.size, ...) from writeBlob in lib/dispatcher/client-h1.js and lib/dispatcher/client-h2.js.

Breaking Changes and Deprecations

N/A

Status

The contentLength === body.size assert is always true because writeH1
and writeH2 overwrite contentLength with body.size (via util.bodyLength)
before invoking writeBlob for blob bodies. Drop the dead assert and
keep the if-guard as a defensive net.

Refs nodejs#5153

Signed-off-by: SAY-5 <saiasish.cnp@gmail.com>
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.28%. Comparing base (e58fff7) to head (7904eb2).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5231      +/-   ##
==========================================
+ Coverage   93.27%   93.28%   +0.01%     
==========================================
  Files         110      110              
  Lines       36353    36349       -4     
==========================================
  Hits        33909    33909              
+ Misses       2444     2440       -4     

☔ View full report in Codecov by Sentry.
📢 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.

Copy link
Copy Markdown
Member

@mcollina mcollina left a comment

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 ef4d1d6 into nodejs:main May 6, 2026
34 of 35 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.

Dead code in writeBlob: assert and RequestContentLengthMismatchError are both unreachable in practice

3 participants