golem15/msd-core
4
0
Fork 0
Code Issues 1 Pull Requests Actions 2 Packages Projects Releases Wiki Activity

fix(#4306): extend fault-injection fd-swallow fix across the whole suite (#4308)

Browse Source 
* fix(#4306): forward real bytes through io.test.cjs's fault-injection mocks

The bug #1008 fault-injection tests mock fs.writeSync scoped only by file
descriptor. On their "success" arms (the retry-after-EAGAIN/EINTR call, and
the short-write simulation) they fabricated a return byte count without ever
calling the real writeSync -- the bytes went into a local array and nowhere
else.

node:test's process-isolation runner (default on Node >= 22) reads each test
file's own stdout to parse its child-to-parent result protocol. If the
runner's own reporter write for an adjacent test lands on fd 1 while one of
these mocks is installed, that write was silently swallowed instead of
reaching the real pipe -- observed in CI as "Unable to deserialize cloned
data" (a corrupted/truncated byte stream on the parent's read side), not a
thrown exception.

Every "success" arm now forwards the real bytes to orig()/restore() instead
of fabricating a return value, so anything else sharing the fd during the
mocked window still gets its bytes delivered for real. writeAllSync (the
only production caller reaching this mock) always passes a Buffer, so the
forwarded calls use the buffer-form fs.writeSync overload unambiguously.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(#4306): extend fault-injection fd-swallow fix across the whole suite

The originally-fixed instance (tests/io.test.cjs) was one occurrence of a
copy-pasted defect: mocked fs.writeSync arms fabricated a return byte count
without ever forwarding the call to the real fs.writeSync, silently
discarding bytes. Under node:test's process-isolated runner, the parent
reads the child's real stdout to parse v8-serialized report frames
interleaved with plain output (confirmed against node's own
lib/internal/test_runner/runner.js and a matching upstream issue,
nodejs/node#64061) — a swallowed write on that fd corrupts the parent's
parse ("Unable to deserialize cloned data").

Adds a shared, safe capture helper to tests/helpers.cjs, captureFdSync(fd,
fn): it always forwards every write to the real fs.writeSync first, then
records only the observed fd's bytes, sliced by the real return count (not
the requested length), decoded once via Buffer.concat so a short write
can't split a multi-byte codepoint across two decodes.

17 test files migrate their local copy of the unsafe mock to this shared
helper. tests/worktree-base-ref.test.cjs keeps a narrower in-place fix
instead (it needs to record every fd a write touched, which the shared
helper doesn't expose).

tests/io.test.cjs gets two follow-up correctness fixes on top of the
already-committed forwarding fix: the EAGAIN/EINTR/short-write arms now
derive their recorded chunk from the real return count everywhere
(including the string-form overload), and the short-write test no longer
forces a Buffer-shaped truncation call onto a string-form write that could
land on the same fd.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: sim <sim@local>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-09-04 23:54:02 -04:00
committed by GitHub
parent 4499933807
commit 9ee6d54cc3
19 changed files with 208 additions and 380 deletions
Download Patch File Download Diff File Expand all files Collapse all files

61
tests/io.test.cjs
View File

@@ -395,7 +395,9 @@ function bug1008ChunkOf(data, offset, length) {
const end = length === undefined ? data.length : start + length;
return data.subarray(start, end).toString('utf8');
}
return String(data);
const str = String(data);
if (length === undefined) return str;
return Buffer.from(str, 'utf8').subarray(0, length).toString('utf8');
}
function bug1008WriteError(code, errno) {
@@ -415,9 +417,19 @@ describe('bug #1008: io.output() tolerates a full / slow non-blocking pipe', ()
if (fd !== 1) return orig(fd, data, offset, length);
calls += 1;
if (calls === 1) throw bug1008WriteError('EAGAIN', -11); // pipe momentarily full
const chunk = bug1008ChunkOf(data, offset, length);
written.push(chunk);
return Buffer.byteLength(chunk, 'utf8');
// #4306: deliver the retried write for real via `orig` rather than
// fabricating a byte count while discarding it. node:test's own
// process-isolated runner reads this file's real stdout to parse its
// child-to-parent result protocol; a stray write from that protocol
// landing on fd 1 while this mock is installed would otherwise be
// silently swallowed instead of reaching the real pipe, corrupting the
// parent's parse ("Unable to deserialize cloned data"). The pushed
// chunk is derived from `orig`'s real return count (never the
// requested length) so a genuine short write from the real fd is
// reflected accurately, matching the short-write test's pattern below.
const n = orig(fd, data, offset, length);
written.push(bug1008ChunkOf(data, offset, n));
return n;
});
const payload = { ok: true, n: 42 };
@@ -434,9 +446,11 @@ describe('bug #1008: io.output() tolerates a full / slow non-blocking pipe', ()
if (fd !== 1) return orig(fd, data, offset, length);
calls += 1;
if (calls === 1) throw bug1008WriteError('EINTR', -4);
const chunk = bug1008ChunkOf(data, offset, length);
written.push(chunk);
return Buffer.byteLength(chunk, 'utf8');
// #4306: see the EAGAIN test above — deliver the retried write for
// real, and derive the pushed chunk from its real return count.
const n = orig(fd, data, offset, length);
written.push(bug1008ChunkOf(data, offset, n));
return n;
});
assert.doesNotThrow(() => io.output('plain', true, 'PLAIN-RAW'));
@@ -449,10 +463,29 @@ describe('bug #1008: io.output() tolerates a full / slow non-blocking pipe', ()
const orig = fs.writeSync.bind(fs);
t.mock.method(fs, 'writeSync', (fd, data, offset, length) => {
if (fd !== 1) return orig(fd, data, offset, length);
if (!Buffer.isBuffer(data)) {
// Only Buffer-form writes (what io.output actually produces) are
// subject to the simulated short-write cap below. Anything else
// sharing fd 1 during this window (e.g. node:test's own interleaved
// report traffic, which can be string-form) must pass through with
// its real arguments — forcing it through the Buffer-shaped
// truncation math below would corrupt fs.writeSync's string-form
// overload (its 3rd/4th args are position/encoding, not
// offset/length).
return orig(fd, data, offset, length);
}
const chunk = bug1008ChunkOf(data, offset, length);
const part = chunk.slice(0, CAP);
written.push(part);
return Buffer.byteLength(part, 'utf8');
const partLen = Buffer.byteLength(part, 'utf8');
// #4306: deliver the truncated slice for real via `orig`, at the
// simulated cap, instead of fabricating a byte count while discarding
// the write — see the EAGAIN test above for why a swallowed write on a
// shared fd is unsafe. `orig`'s own return is used below so a real
// short write from the kernel (rather than our simulated one) is still
// reflected accurately.
const n = orig(fd, data, offset ?? 0, partLen);
written.push(bug1008ChunkOf(data, offset, n));
return n;
});
const payload = { message: 'a reasonably long ascii payload to force many short writes' };
@@ -508,9 +541,13 @@ describe('bug #1008: io.error() tolerates a full non-blocking stderr pipe', () =
if (fd !== 2) return restore(fd, data, offset, length);
calls += 1;
if (calls === 1) throw bug1008WriteError('EAGAIN', -11);
const chunk = bug1008ChunkOf(data, offset, length);
written.push(chunk);
return Buffer.byteLength(chunk, 'utf8');
// #4306: forward the retried write to the real writeSync instead of
// fabricating a byte count — see the io.output() EAGAIN test above.
// The pushed chunk is derived from the real return count, not the
// requested length.
const n = restore(fd, data, offset, length);
written.push(bug1008ChunkOf(data, offset, n));
return n;
};
try {
assert.throws(