diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index 6ec895f1a0c556..75052ab4c4f20f 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -10,6 +10,7 @@ const { PromisePrototypeFinally, PromisePrototypeThen, PromiseResolve, + PromiseReject, SafeArrayIterator, Symbol, Uint8Array, @@ -33,6 +34,7 @@ const { ERR_METHOD_NOT_IMPLEMENTED, }, AbortError, + aggregateTwoErrors, } = require('internal/errors'); const { isArrayBufferView } = require('internal/util/types'); const { rimrafPromises } = require('internal/fs/rimraf'); @@ -250,6 +252,19 @@ class FileHandle extends EventEmitterMixin(JSTransferable) { } } +async function handleFdClose(fileOpPromise, closeFunc) { + return PromisePrototypeThen( + fileOpPromise, + (result) => PromisePrototypeThen(closeFunc(), () => result), + (opError) => + PromisePrototypeThen( + closeFunc(), + () => PromiseReject(opError), + (closeError) => PromiseReject(aggregateTwoErrors(closeError, opError)) + ) + ); +} + async function fsCall(fn, handle, ...args) { if (handle[kRefs] === undefined) { throw new ERR_INVALID_ARG_TYPE('filehandle', 'FileHandle', handle); @@ -501,7 +516,7 @@ async function rename(oldPath, newPath) { async function truncate(path, len = 0) { const fd = await open(path, 'r+'); - return PromisePrototypeFinally(ftruncate(fd, len), fd.close); + return handleFdClose(ftruncate(fd, len), fd.close); } async function ftruncate(handle, len = 0) { @@ -632,7 +647,7 @@ async function lchmod(path, mode) { throw new ERR_METHOD_NOT_IMPLEMENTED('lchmod()'); const fd = await open(path, O_WRONLY | O_SYMLINK); - return PromisePrototypeFinally(fchmod(fd, mode), fd.close); + return handleFdClose(fchmod(fd, mode), fd.close); } async function lchown(path, uid, gid) { @@ -711,7 +726,7 @@ async function writeFile(path, data, options) { checkAborted(options.signal); const fd = await open(path, flag, options.mode); - return PromisePrototypeFinally( + return handleFdClose( writeFileHandle(fd, data, options.signal, options.encoding), fd.close); } @@ -736,7 +751,7 @@ async function readFile(path, options) { checkAborted(options.signal); const fd = await open(path, flag, 0o666); - return PromisePrototypeFinally(readFileHandle(fd, options), fd.close); + return handleFdClose(readFileHandle(fd, options), fd.close); } module.exports = { diff --git a/test/parallel/test-fs-promises-file-handle-aggregate-errors.js b/test/parallel/test-fs-promises-file-handle-aggregate-errors.js new file mode 100644 index 00000000000000..d4c3b37d652d12 --- /dev/null +++ b/test/parallel/test-fs-promises-file-handle-aggregate-errors.js @@ -0,0 +1,72 @@ +'use strict'; +// Flags: --expose-internals + +const common = require('../common'); +const tmpdir = require('../common/tmpdir'); + +// The following tests validate aggregate errors are thrown correctly +// when both an operation and close throw. + +const path = require('path'); +const { + readFile, + writeFile, + truncate, + lchmod, +} = require('fs/promises'); +const { + FileHandle, +} = require('internal/fs/promises'); + +const assert = require('assert'); +const originalFd = Object.getOwnPropertyDescriptor(FileHandle.prototype, 'fd'); + +let count = 0; +async function createFile() { + const filePath = path.join(tmpdir.path, `aggregate_errors_${++count}.txt`); + await writeFile(filePath, 'content'); + return filePath; +} + +async function checkAggregateError(op) { + try { + const filePath = await createFile(); + Object.defineProperty(FileHandle.prototype, 'fd', { + get: function() { + // Close is set by using a setter, + // so it needs to be set on the instance. + const originalClose = this.close; + this.close = async () => { + // close the file + await originalClose.call(this); + const closeError = new Error('CLOSE_ERROR'); + closeError.code = 456; + throw closeError; + }; + const opError = new Error('INTERNAL_ERROR'); + opError.code = 123; + throw opError; + } + }); + + await assert.rejects(op(filePath), common.mustCall((err) => { + assert.strictEqual(err.name, 'AggregateError'); + assert.strictEqual(err.code, 123); + assert.strictEqual(err.errors.length, 2); + assert.strictEqual(err.errors[0].message, 'INTERNAL_ERROR'); + assert.strictEqual(err.errors[1].message, 'CLOSE_ERROR'); + return true; + })); + } finally { + Object.defineProperty(FileHandle.prototype, 'fd', originalFd); + } +} +(async function() { + tmpdir.refresh(); + await checkAggregateError((filePath) => truncate(filePath)); + await checkAggregateError((filePath) => readFile(filePath)); + await checkAggregateError((filePath) => writeFile(filePath, '123')); + if (common.isOSX) { + await checkAggregateError((filePath) => lchmod(filePath, 0o777)); + } +})().then(common.mustCall()); diff --git a/test/parallel/test-fs-promises-file-handle-close-errors.js b/test/parallel/test-fs-promises-file-handle-close-errors.js new file mode 100644 index 00000000000000..b975b3f3e1583c --- /dev/null +++ b/test/parallel/test-fs-promises-file-handle-close-errors.js @@ -0,0 +1,67 @@ +'use strict'; +// Flags: --expose-internals + +const common = require('../common'); +const tmpdir = require('../common/tmpdir'); + +// The following tests validate aggregate errors are thrown correctly +// when both an operation and close throw. + +const path = require('path'); +const { + readFile, + writeFile, + truncate, + lchmod, +} = require('fs/promises'); +const { + FileHandle, +} = require('internal/fs/promises'); + +const assert = require('assert'); +const originalFd = Object.getOwnPropertyDescriptor(FileHandle.prototype, 'fd'); + +let count = 0; +async function createFile() { + const filePath = path.join(tmpdir.path, `close_errors_${++count}.txt`); + await writeFile(filePath, 'content'); + return filePath; +} + +async function checkCloseError(op) { + try { + const filePath = await createFile(); + Object.defineProperty(FileHandle.prototype, 'fd', { + get: function() { + // Close is set by using a setter, + // so it needs to be set on the instance. + const originalClose = this.close; + this.close = async () => { + // close the file + await originalClose.call(this); + const closeError = new Error('CLOSE_ERROR'); + closeError.code = 456; + throw closeError; + }; + return originalFd.get.call(this); + } + }); + + await assert.rejects(op(filePath), { + name: 'Error', + message: 'CLOSE_ERROR', + code: 456, + }); + } finally { + Object.defineProperty(FileHandle.prototype, 'fd', originalFd); + } +} +(async function() { + tmpdir.refresh(); + await checkCloseError((filePath) => truncate(filePath)); + await checkCloseError((filePath) => readFile(filePath)); + await checkCloseError((filePath) => writeFile(filePath, '123')); + if (common.isOSX) { + await checkCloseError((filePath) => lchmod(filePath, 0o777)); + } +})().then(common.mustCall()); diff --git a/test/parallel/test-fs-promises-file-handle-op-errors.js b/test/parallel/test-fs-promises-file-handle-op-errors.js new file mode 100644 index 00000000000000..7110b5d9d40467 --- /dev/null +++ b/test/parallel/test-fs-promises-file-handle-op-errors.js @@ -0,0 +1,61 @@ +'use strict'; +// Flags: --expose-internals + +const common = require('../common'); +const tmpdir = require('../common/tmpdir'); + +// The following tests validate aggregate errors are thrown correctly +// when both an operation and close throw. + +const path = require('path'); +const { + readFile, + writeFile, + truncate, + lchmod, +} = require('fs/promises'); +const { + FileHandle, +} = require('internal/fs/promises'); + +const assert = require('assert'); +const originalFd = Object.getOwnPropertyDescriptor(FileHandle.prototype, 'fd'); + +let count = 0; +async function createFile() { + const filePath = path.join(tmpdir.path, `op_errors_${++count}.txt`); + await writeFile(filePath, 'content'); + return filePath; +} + +async function checkOperationError(op) { + try { + const filePath = await createFile(); + Object.defineProperty(FileHandle.prototype, 'fd', { + get: function() { + // Verify that close is called when an error is thrown + this.close = common.mustCall(this.close); + const opError = new Error('INTERNAL_ERROR'); + opError.code = 123; + throw opError; + } + }); + + await assert.rejects(op(filePath), { + name: 'Error', + message: 'INTERNAL_ERROR', + code: 123, + }); + } finally { + Object.defineProperty(FileHandle.prototype, 'fd', originalFd); + } +} +(async function() { + tmpdir.refresh(); + await checkOperationError((filePath) => truncate(filePath)); + await checkOperationError((filePath) => readFile(filePath)); + await checkOperationError((filePath) => writeFile(filePath, '123')); + if (common.isOSX) { + await checkOperationError((filePath) => lchmod(filePath, 0o777)); + } +})().then(common.mustCall());