Skip to content

util: fix inspect indentation of detached DataView - #66421

Open
Kjubikstronk wants to merge 1 commit into
nodejs:mainfrom
Kjubikstronk:util-inspect-detached-dataview-indentation
Open

Kjubikstronk wants to merge 1 commit into
nodejs:mainfrom
Kjubikstronk:util-inspect-detached-dataview-indentation

Conversation

@Kjubikstronk

Copy link
Copy Markdown

util.inspect() of a DataView whose buffer was detached comes out indented too far, and so does everything after it in the same output:

const util = require('node:util');
const ab = new ArrayBuffer(4);
const dv = new DataView(ab);
structuredClone(ab, { transfer: [ab] });
console.log(util.inspect(dv));
DataView {
      [byteLength]: 0,
      [byteOffset]: undefined,
      [buffer]: ArrayBuffer { (detached), [byteLength]: 0 }
    }

The cause was formatExtraProperties(): it raised ctx.indentationLvl before reading value[key]. On a detached DataView the byteLength and byteOffset getters throw, so the -= 2 never runs and 2 spaces leak per key, 4 in total.

The bug came in with #60131, and the test added there asserted the leaked output.

Fix: read the value before indenting.

I built Node on Windows and tested it: the changed test fails without the fix and passes with it, as do 428 related tests.

I used Claude Code to find the bug and write the fix. I reviewed the diff and the changed assertion myself.

Refs: #60131

formatExtraProperties() raised ctx.indentationLvl before reading the
property. The getters of a detached DataView throw, so the level was
never lowered again, and the DataView and everything inspected after it
in the same call ended up indented too far. Read the value first.

Signed-off-by: Miodrag Obradovic <mck097@gmail.com>
Assisted-by: a closed-source coding agent
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. util Issues and PRs related to the built-in util module. labels Sep 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Welcome to Node.js, and thank you for your first contribution!

Before review, please take a moment to read:

Please make sure every commit is signed off. For a first pull request, GitHub Actions require collaborator approval and Jenkins CI must be started by a collaborator or triager, so an initial wait is normal.

@codecov

codecov Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.37%. Comparing base (d7ea02d) to head (c90d878).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #66421   +/-   ##
=======================================
  Coverage   90.37%   90.37%           
=======================================
  Files         792      792           
  Lines      275683   275686    +3     
  Branches    52854    52854           
=======================================
+ Hits       249147   249159   +12     
+ Misses      16947    16935   -12     
- Partials     9589     9592    +3     
Files with missing lines Coverage Δ
lib/internal/util/inspect.js 97.04% <100.00%> (+<0.01%) ⬆️

... and 21 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.

@MikeMcC399 MikeMcC399 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 Oct 1, 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 Oct 1, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. util Issues and PRs related to the built-in util module.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants