Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 73 additions & 16 deletions lib/internal/fs/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,11 @@ const EventEmitter = require('events');
const { StringDecoder } = require('string_decoder');
const { kFSWatchStart, watch } = require('internal/fs/watchers');
const nonNativeWatcher = require('internal/fs/recursive_watch');
const { isIterable } = require('internal/streams/utils');
const {
isIterable,
isReadableErrored,
isReadableNodeStream,
} = require('internal/streams/utils');
const assert = require('internal/assert');

const permission = require('internal/process/permission');
Expand Down Expand Up @@ -1126,24 +1130,67 @@ function checkAborted(signal) {
throw new AbortError(undefined, { cause: signal.reason });
}

async function writeFileHandle(filehandle, data, signal, encoding) {
checkAborted(signal);
function makeWriteFileStreamErrorHandler(data) {
if (!isReadableNodeStream(data) ||
typeof data.removeListener !== 'function') {
return undefined;
}

let error;
let errored = false;
function onError(err) {
error = err;
errored = true;
}
const streamError = isReadableErrored(data);
const wasErrored = streamError != null;
if (streamError != null)
onError(streamError);
data.on('error', onError);

return {
__proto__: null,
check() {
if (errored)
throw error;
},
cleanup() {
if (wasErrored) {
process.nextTick(() => data.removeListener('error', onError));
} else {
data.removeListener('error', onError);
}
},
};
}

async function writeFileHandle(filehandle, data, signal, encoding, streamErrorHandler) {
if (isCustomIterable(data)) {
for await (const buf of data) {
streamErrorHandler ??= makeWriteFileStreamErrorHandler(data);
try {
checkAborted(signal);
const toWrite =
isArrayBufferView(buf) ? buf : Buffer.from(buf, encoding || 'utf8');
let remaining = toWrite.byteLength;
while (remaining > 0) {
const writeSize = MathMin(kWriteFileMaxChunkSize, remaining);
const { bytesWritten } = await write(
filehandle, toWrite, toWrite.byteLength - remaining, writeSize);
remaining -= bytesWritten;
streamErrorHandler?.check();
for await (const buf of data) {
checkAborted(signal);
streamErrorHandler?.check();
const toWrite =
isArrayBufferView(buf) ? buf : Buffer.from(buf, encoding || 'utf8');
let remaining = toWrite.byteLength;
while (remaining > 0) {
const writeSize = MathMin(kWriteFileMaxChunkSize, remaining);
const { bytesWritten } = await write(
filehandle, toWrite, toWrite.byteLength - remaining, writeSize);
remaining -= bytesWritten;
checkAborted(signal);
streamErrorHandler?.check();
}
}
} finally {
streamErrorHandler?.cleanup();
}
return;
}
checkAborted(signal);
data = new Uint8Array(data.buffer, data.byteOffset, data.byteLength);
let remaining = data.byteLength;
if (remaining === 0) return;
Expand Down Expand Up @@ -2096,13 +2143,23 @@ async function writeFile(path, data, options) {
}

validateAbortSignal(options.signal);
checkAborted(options.signal);
const streamErrorHandler = makeWriteFileStreamErrorHandler(data);
Comment on lines +2146 to +2147

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.

The abort check runs before the temporary stream error listener is attached. If data is already errored and the signal is already aborted, writeFile() rejects with AbortError, but the pending stream error is still emitted as an uncaughtException. Attach the handler first and clean it up when the abort check throws:

Suggested change
checkAborted(options.signal);
const streamErrorHandler = makeWriteFileStreamErrorHandler(data);
const streamErrorHandler = makeWriteFileStreamErrorHandler(data);
try {
checkAborted(options.signal);
} catch (err) {
streamErrorHandler?.cleanup();
throw err;
}

A regression test would look like this:

controller.abort();
stream.destroy(sourceError);
process.once('uncaughtException', mustNotCall());
await assert.rejects(writeFile(target, stream, { signal }), { code: 'ABORT_ERR' });
await nextTick();
assert.strictEqual(stream.listenerCount('error'), 0);

The same case should be covered for both path-based writeFile() and FileHandle.writeFile().


if (path instanceof FileHandle)
return writeFileHandle(path, data, options.signal, options.encoding);
return writeFileHandle(
path, data, options.signal, options.encoding, streamErrorHandler);

checkAborted(options.signal);
let fd;
try {
fd = await open(path, flag, options.mode);
} catch (err) {
streamErrorHandler?.cleanup();
throw err;
}

const fd = await open(path, flag, options.mode);
let writeOp = writeFileHandle(fd, data, options.signal, options.encoding);
let writeOp = writeFileHandle(
fd, data, options.signal, options.encoding, streamErrorHandler);

if (flush) {
writeOp = handleFdSync(writeOp, fd);
Expand Down
67 changes: 67 additions & 0 deletions test/parallel/test-fs-promises-file-handle-writeFile.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ async function doWriteBufferAndCancel() {

const dest = path.resolve(tmpDir, 'tmp.txt');
const otherDest = path.resolve(tmpDir, 'tmp-2.txt');
const errorDest = path.resolve(tmpDir, 'tmp-error.txt');
const stream = Readable.from(['a', 'b', 'c']);
const stream2 = Readable.from(['ümlaut', ' ', 'sechzig']);
const iterable = {
Expand All @@ -82,6 +83,27 @@ function iterableWith(value) {
}
};
}

function createEarlyErrorStream(error) {
const stream = new Readable({
read() {}
});
process.nextTick(() => stream.destroy(error));
return stream;
}

function createErroredStream(error) {
const stream = new Readable({
read() {}
});
stream.destroy(error);
return stream;
}

function waitForNextTick() {
return new Promise((resolve) => process.nextTick(resolve));
}

const bufferIterable = {
expected: 'abc',
*[Symbol.iterator]() {
Expand Down Expand Up @@ -111,6 +133,49 @@ async function doWriteStream() {
}
}

async function doWriteStreamError() {
const fileHandle = await open(errorDest, 'w+');
const error = new Error('early file handle writeFile stream error');
const stream = createEarlyErrorStream(error);
const uncaughtException = common.mustNotCall(
'stream errors should reject FileHandle.writeFile()');

process.once('uncaughtException', uncaughtException);
try {
await assert.rejects(
fileHandle.writeFile(stream),
{ message: error.message }
);
// FileHandle.writeFile() starts iteration before the next-tick error,
// so the stream async iterator retains its own error listener.
assert.strictEqual(stream.listenerCount('error'), 1);
} finally {
process.removeListener('uncaughtException', uncaughtException);
await fileHandle.close();
}
}

async function doWriteAlreadyErroredStream() {
const fileHandle = await open(errorDest, 'w+');
const error = new Error('already errored file handle writeFile stream');
const stream = createErroredStream(error);
const uncaughtException = common.mustNotCall(
'already errored streams should reject FileHandle.writeFile()');

process.once('uncaughtException', uncaughtException);
try {
await assert.rejects(
fileHandle.writeFile(stream),
{ message: error.message }
);
await waitForNextTick();
assert.strictEqual(stream.listenerCount('error'), 0);
} finally {
process.removeListener('uncaughtException', uncaughtException);
await fileHandle.close();
}
}

async function doWriteStreamWithCancel() {
const controller = new AbortController();
const { signal } = controller;
Expand Down Expand Up @@ -256,6 +321,8 @@ async function doWriteFromCurrentPosition() {
await doWriteBufferAndCancel();
await doWriteString();
await doWriteStream();
await doWriteStreamError();
await doWriteAlreadyErroredStream();
await doWriteStreamWithCancel();
await doWriteIterable();
await doWriteInvalidIterable();
Expand Down
89 changes: 89 additions & 0 deletions test/parallel/test-fs-promises-writefile.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ tmpdir.refresh();

const dest = path.resolve(tmpDir, 'tmp.txt');
const otherDest = path.resolve(tmpDir, 'tmp-2.txt');
const errorDest = path.resolve(tmpDir, 'tmp-error.txt');
const buffer = Buffer.from('abc'.repeat(1000));
const stream = Readable.from(['a', 'b', 'c']);
const stream2 = Readable.from(['ümlaut', ' ', 'sechzig']);
Expand All @@ -24,6 +25,16 @@ const iterable = {
yield 'c';
}
};
const streamLikeIterable = {
expected: 'abc',
pipe: common.mustNotCall('pipe should not be called for custom iterables'),
on: common.mustNotCall('on should not be called without removeListener'),
*[Symbol.iterator]() {
yield 'a';
yield 'b';
yield 'c';
}
};

const veryLargeIterable = {
expected: 'dogs running'.repeat(512 * 1024),
Expand All @@ -39,6 +50,27 @@ function iterableWith(value) {
}
};
}

function createEarlyErrorStream(error) {
const stream = new Readable({
read() {}
});
process.nextTick(() => stream.destroy(error));
return stream;
}

function createErroredStream(error) {
const stream = new Readable({
read() {}
});
stream.destroy(error);
return stream;
}

function waitForNextTick() {
return new Promise((resolve) => process.nextTick(resolve));
}

const bufferIterable = {
expected: 'abc',
*[Symbol.iterator]() {
Expand Down Expand Up @@ -77,6 +109,53 @@ async function doWriteStream() {
assert.deepStrictEqual(data, expected);
}

async function doWriteStreamError() {
const error = new Error('early writeFile stream error');
const stream = createEarlyErrorStream(error);
const uncaughtException = common.mustNotCall(
'stream errors should reject writeFile()');

process.once('uncaughtException', uncaughtException);
try {
await assert.rejects(
fsPromises.writeFile(errorDest, stream),
{ message: error.message }
);
assert.strictEqual(stream.listenerCount('error'), 0);
} finally {
process.removeListener('uncaughtException', uncaughtException);
}
}

async function doWriteAlreadyErroredStream() {
const error = new Error('already errored writeFile stream');
const stream = createErroredStream(error);
const uncaughtException = common.mustNotCall(
'already errored streams should reject writeFile()');

process.once('uncaughtException', uncaughtException);
try {
await assert.rejects(
fsPromises.writeFile(errorDest, stream),
{ message: error.message }
);
await waitForNextTick();
assert.strictEqual(stream.listenerCount('error'), 0);
} finally {
process.removeListener('uncaughtException', uncaughtException);
}
}

async function doWriteStreamOpenError() {
const stream = Readable.from(['a']);

await assert.rejects(
fsPromises.writeFile(path.resolve(tmpDir, 'not-found', 'tmp.txt'), stream),
{ code: 'ENOENT' }
);
assert.strictEqual(stream.listenerCount('error'), 0);
}

async function doWriteStreamWithCancel() {
const controller = new AbortController();
const { signal } = controller;
Expand All @@ -93,6 +172,12 @@ async function doWriteIterable() {
assert.deepStrictEqual(data, iterable.expected);
}

async function doWriteStreamLikeIterable() {
await fsPromises.writeFile(dest, streamLikeIterable);
const data = fs.readFileSync(dest, 'utf-8');
assert.deepStrictEqual(data, streamLikeIterable.expected);
}

async function doWriteInvalidIterable() {
await Promise.all(
[42, 42n, {}, Symbol('42'), true, undefined, null, NaN].map((value) =>
Expand Down Expand Up @@ -165,8 +250,12 @@ async function doWriteTypedArrays() {
await doWriteBufferAndCancel();
await doWriteString();
await doWriteStream();
await doWriteStreamError();
await doWriteAlreadyErroredStream();
await doWriteStreamOpenError();
await doWriteStreamWithCancel();
await doWriteIterable();
await doWriteStreamLikeIterable();
await doWriteInvalidIterable();
await doWriteIterableWithEncoding();
await doWriteBufferIterable();
Expand Down
Loading