Skip to content

Commit 5bcade9

Browse files
jasnellnodejs-github-bot
authored andcommitted
stream: make endSync() optional in pipeToSync()
pipeToSync() threw ERR_INVALID_ARG_TYPE before writing anything when the writer had no endSync() method and preventClose was not set. endSync() is optional: the spec (pipeToSync() step 7) only calls it if the writer has it, and pipeTo() already treats it that way. A writer without endSync() now receives the data and is not closed. pipeToSync() still never falls back to the async end(). This also fixes the from-sync-writev case of benchmark/streams/iter-from-batching.js, whose writer has no endSync(). testPipeToSyncNoEndSync asserted the previous rejection and now checks that the data is written and end() is not called. The documentation of the writer requirements is corrected as well: only writeSync() is required. Assisted-by: OpenCode Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: #66648 Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
1 parent 82563d8 commit 5bcade9

3 files changed

Lines changed: 18 additions & 20 deletions

File tree

‎doc/api/stream_iter.md‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -714,7 +714,7 @@ added:
714714

715715
* `source` {Iterable} The sync data source.
716716
* `...transforms` {Function|Object} Zero or more sync transforms.
717-
* `writer` {Object} Destination with `write(chunk)` method.
717+
* `writer` {Object} Destination with a `writeSync(chunk)` method.
718718
* `options` {Object}
719719
* `failOnIncompleteClose` {boolean} If `true`, call `writer.fail()` when
720720
`writer.endSync()` cannot close the writer synchronously. Ignored when
@@ -727,8 +727,11 @@ added:
727727
Synchronous version of [`pipeTo()`][]. The `source`, all transforms, and the
728728
`writer` must be synchronous. Cannot accept async iterables or promises.
729729

730-
The `writer` must have the `*Sync` methods (`writeSync`, `writevSync`,
731-
`endSync`) and `fail()` for this to work.
730+
The `writer` must have a `writeSync()` method. The other methods are
731+
optional: `writevSync()` is used for batches of more than one chunk if it is
732+
present, `endSync()` is called to close the writer (unless `preventClose` is
733+
`true`), and `fail()` is called if the pipe fails (unless `preventFail` is
734+
`true`). A writer without `endSync()` is not closed.
732735

733736
`pipeToSync()` never falls back to the asynchronous writer methods. If
734737
`writer.endSync()` returns `-1` because the writer cannot close synchronously

‎lib/internal/streams/iter/pull.js‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1032,12 +1032,9 @@ function pipeToSync(source, ...args) {
10321032
context: 'options',
10331033
});
10341034
const hasWritevSync = typeof writer.writevSync === 'function';
1035+
// endSync() is optional: a writer without it is not closed.
10351036
const endSync = writer.endSync;
1036-
1037-
if (!options.preventClose && typeof endSync !== 'function') {
1038-
throw new ERR_INVALID_ARG_TYPE(
1039-
'writer.endSync', 'Function', endSync);
1040-
}
1037+
const hasEndSync = typeof endSync === 'function';
10411038

10421039
// Normalize source and create pipeline
10431040
const normalized = fromSync(source);
@@ -1074,7 +1071,7 @@ function pipeToSync(source, ...args) {
10741071
}
10751072
}
10761073

1077-
if (!options.preventClose) {
1074+
if (!options.preventClose && hasEndSync) {
10781075
closedSync = FunctionPrototypeCall(endSync, writer) >= 0;
10791076
}
10801077
} catch (error) {

‎test/parallel/test-stream-iter-pipeto-edge.js‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -47,20 +47,18 @@ async function testPipeToSyncEndSyncFailureDoesNotFailWriter() {
4747
assert.strictEqual(await result, 'abcdef');
4848
}
4949

50-
// pipeToSync requires endSync() when closing is enabled.
50+
// pipeToSync does not require endSync().
5151
async function testPipeToSyncNoEndSync() {
52-
let writeCalled = false;
53-
let endCalled = false;
52+
// endSync() is optional. Without it the data is still written and the
53+
// writer is not closed; pipeToSync() never falls back to end().
54+
const written = [];
5455
const writer = {
55-
writeSync() { writeCalled = true; return true; },
56-
end() { endCalled = true; },
56+
writeSync(chunk) { written.push(chunk); return true; },
57+
end: common.mustNotCall(),
58+
fail: common.mustNotCall(),
5759
};
58-
assert.throws(
59-
() => pipeToSync(fromSync('data'), writer),
60-
{ code: 'ERR_INVALID_ARG_TYPE' },
61-
);
62-
assert.strictEqual(writeCalled, false);
63-
assert.strictEqual(endCalled, false);
60+
assert.strictEqual(pipeToSync(fromSync('data'), writer), 4);
61+
assert.deepStrictEqual(written, [new TextEncoder().encode('data')]);
6462
}
6563

6664
// pipeToSync with preventFail: true — source error does NOT call fail()

0 commit comments

Comments
 (0)