fs: add stacktrace to fs/promises

Sync functions in fs throwed an error with a stacktrace which is helpful
for debugging. But functions in fs/promises throwed an error without
a stacktrace. This commit adds stacktraces by calling
Error.captureStacktrace and re-throwing the error.

Refs: https://github.com/nodejs/node/issues/34817
PR-URL: https://github.com/nodejs/node/pull/49849
Fixes: https://github.com/nodejs/node/issues/50160
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
This commit is contained in:
翠 / green 2023-10-26 11:35:40 +09:00 committed by GitHub
parent b24215e35f
commit c41cf6fd49
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 248 additions and 90 deletions

View File

@ -4,6 +4,7 @@ const {
ArrayPrototypePush,
ArrayPrototypePop,
Error,
ErrorCaptureStackTrace,
MathMax,
MathMin,
Promise,
@ -130,6 +131,15 @@ function lazyFsStreams() {
return fsStreams ??= require('internal/fs/streams');
}
// By the time the C++ land creates an error for a promise rejection (likely from a
// libuv callback), there is already no JS frames on the stack. So we need to
// wait until V8 resumes execution back to JS land before we have enough information
// to re-capture the stack trace.
function handleErrorFromBinding(error) {
ErrorCaptureStackTrace(error, handleErrorFromBinding);
return PromiseReject(error);
}
class FileHandle extends EventEmitter {
/**
* @param {InternalFSBinding.FileHandle | undefined} filehandle
@ -494,7 +504,11 @@ async function readFileHandle(filehandle, options) {
checkAborted(signal);
const statFields = await binding.fstat(filehandle.fd, false, kUsePromises);
const statFields = await PromisePrototypeThen(
binding.fstat(filehandle.fd, false, kUsePromises),
undefined,
handleErrorFromBinding,
);
checkAborted(signal);
@ -525,8 +539,11 @@ async function readFileHandle(filehandle, options) {
length = MathMin(size - totalRead, kReadFileBufferLength);
}
const bytesRead = (await binding.read(filehandle.fd, buffer, offset,
length, -1, kUsePromises)) ?? 0;
const bytesRead = (await PromisePrototypeThen(
binding.read(filehandle.fd, buffer, offset, length, -1, kUsePromises),
undefined,
handleErrorFromBinding,
)) ?? 0;
totalRead += bytesRead;
if (bytesRead === 0 ||
@ -574,8 +591,11 @@ async function access(path, mode = F_OK) {
path = getValidatedPath(path);
mode = getValidMode(mode, 'access');
return binding.access(pathModule.toNamespacedPath(path), mode,
kUsePromises);
return await PromisePrototypeThen(
binding.access(pathModule.toNamespacedPath(path), mode, kUsePromises),
undefined,
handleErrorFromBinding,
);
}
async function cp(src, dest, options) {
@ -589,10 +609,14 @@ async function copyFile(src, dest, mode) {
src = getValidatedPath(src, 'src');
dest = getValidatedPath(dest, 'dest');
mode = getValidMode(mode, 'copyFile');
return binding.copyFile(pathModule.toNamespacedPath(src),
pathModule.toNamespacedPath(dest),
mode,
kUsePromises);
return await PromisePrototypeThen(
binding.copyFile(pathModule.toNamespacedPath(src),
pathModule.toNamespacedPath(dest),
mode,
kUsePromises),
undefined,
handleErrorFromBinding,
);
}
// Note that unlike fs.open() which uses numeric file descriptors,
@ -601,9 +625,12 @@ async function open(path, flags, mode) {
path = getValidatedPath(path);
const flagsNumber = stringToFlags(flags);
mode = parseFileMode(mode, 'mode', 0o666);
return new FileHandle(
await binding.openFileHandle(pathModule.toNamespacedPath(path),
flagsNumber, mode, kUsePromises));
return new FileHandle(await PromisePrototypeThen(
binding.openFileHandle(pathModule.toNamespacedPath(path),
flagsNumber, mode, kUsePromises),
undefined,
handleErrorFromBinding,
));
}
async function read(handle, bufferOrParams, offset, length, position) {
@ -656,8 +683,11 @@ async function read(handle, bufferOrParams, offset, length, position) {
validatePosition(position, 'position', length);
}
const bytesRead = (await binding.read(handle.fd, buffer, offset, length,
position, kUsePromises)) || 0;
const bytesRead = (await PromisePrototypeThen(
binding.read(handle.fd, buffer, offset, length, position, kUsePromises),
undefined,
handleErrorFromBinding,
)) || 0;
return { __proto__: null, bytesRead, buffer };
}
@ -668,8 +698,11 @@ async function readv(handle, buffers, position) {
if (typeof position !== 'number')
position = null;
const bytesRead = (await binding.readBuffers(handle.fd, buffers, position,
kUsePromises)) || 0;
const bytesRead = (await PromisePrototypeThen(
binding.readBuffers(handle.fd, buffers, position, kUsePromises),
undefined,
handleErrorFromBinding,
)) || 0;
return { __proto__: null, bytesRead, buffers };
}
@ -698,15 +731,22 @@ async function write(handle, buffer, offsetOrOptions, length, position) {
position = null;
validateOffsetLengthWrite(offset, length, buffer.byteLength);
const bytesWritten =
(await binding.writeBuffer(handle.fd, buffer, offset,
length, position, kUsePromises)) || 0;
(await PromisePrototypeThen(
binding.writeBuffer(handle.fd, buffer, offset,
length, position, kUsePromises),
undefined,
handleErrorFromBinding,
)) || 0;
return { __proto__: null, bytesWritten, buffer };
}
validateStringAfterArrayBufferView(buffer, 'buffer');
validateEncoding(buffer, length);
const bytesWritten = (await binding.writeString(handle.fd, buffer, offset,
length, kUsePromises)) || 0;
const bytesWritten = (await PromisePrototypeThen(
binding.writeString(handle.fd, buffer, offset, length, kUsePromises),
undefined,
handleErrorFromBinding,
)) || 0;
return { __proto__: null, bytesWritten, buffer };
}
@ -720,17 +760,24 @@ async function writev(handle, buffers, position) {
return { __proto__: null, bytesWritten: 0, buffers };
}
const bytesWritten = (await binding.writeBuffers(handle.fd, buffers, position,
kUsePromises)) || 0;
const bytesWritten = (await PromisePrototypeThen(
binding.writeBuffers(handle.fd, buffers, position, kUsePromises),
undefined,
handleErrorFromBinding,
)) || 0;
return { __proto__: null, bytesWritten, buffers };
}
async function rename(oldPath, newPath) {
oldPath = getValidatedPath(oldPath, 'oldPath');
newPath = getValidatedPath(newPath, 'newPath');
return binding.rename(pathModule.toNamespacedPath(oldPath),
pathModule.toNamespacedPath(newPath),
kUsePromises);
return await PromisePrototypeThen(
binding.rename(pathModule.toNamespacedPath(oldPath),
pathModule.toNamespacedPath(newPath),
kUsePromises),
undefined,
handleErrorFromBinding,
);
}
async function truncate(path, len = 0) {
@ -741,7 +788,11 @@ async function truncate(path, len = 0) {
async function ftruncate(handle, len = 0) {
validateInteger(len, 'len');
len = MathMax(0, len);
return binding.ftruncate(handle.fd, len, kUsePromises);
return await PromisePrototypeThen(
binding.ftruncate(handle.fd, len, kUsePromises),
undefined,
handleErrorFromBinding,
);
}
async function rm(path, options) {
@ -762,15 +813,27 @@ async function rmdir(path, options) {
}
}
return binding.rmdir(path, kUsePromises);
return await PromisePrototypeThen(
binding.rmdir(path, kUsePromises),
undefined,
handleErrorFromBinding,
);
}
async function fdatasync(handle) {
return binding.fdatasync(handle.fd, kUsePromises);
return await PromisePrototypeThen(
binding.fdatasync(handle.fd, kUsePromises),
undefined,
handleErrorFromBinding,
);
}
async function fsync(handle) {
return binding.fsync(handle.fd, kUsePromises);
return await PromisePrototypeThen(
binding.fsync(handle.fd, kUsePromises),
undefined,
handleErrorFromBinding,
);
}
async function mkdir(path, options) {
@ -784,9 +847,13 @@ async function mkdir(path, options) {
path = getValidatedPath(path);
validateBoolean(recursive, 'options.recursive');
return binding.mkdir(pathModule.toNamespacedPath(path),
parseFileMode(mode, 'mode', 0o777), recursive,
kUsePromises);
return await PromisePrototypeThen(
binding.mkdir(pathModule.toNamespacedPath(path),
parseFileMode(mode, 'mode', 0o777), recursive,
kUsePromises),
undefined,
handleErrorFromBinding,
);
}
async function readdirRecursive(originalPath, options) {
@ -794,11 +861,15 @@ async function readdirRecursive(originalPath, options) {
const queue = [
[
originalPath,
await binding.readdir(
pathModule.toNamespacedPath(originalPath),
options.encoding,
!!options.withFileTypes,
kUsePromises,
await PromisePrototypeThen(
binding.readdir(
pathModule.toNamespacedPath(originalPath),
options.encoding,
!!options.withFileTypes,
kUsePromises,
),
undefined,
handleErrorFromBinding,
),
],
];
@ -814,11 +885,15 @@ async function readdirRecursive(originalPath, options) {
const direntPath = pathModule.join(path, dirent.name);
ArrayPrototypePush(queue, [
direntPath,
await binding.readdir(
direntPath,
options.encoding,
true,
kUsePromises,
await PromisePrototypeThen(
binding.readdir(
direntPath,
options.encoding,
true,
kUsePromises,
),
undefined,
handleErrorFromBinding,
),
]);
}
@ -837,11 +912,15 @@ async function readdirRecursive(originalPath, options) {
if (stat === 1) {
ArrayPrototypePush(queue, [
direntPath,
await binding.readdir(
pathModule.toNamespacedPath(direntPath),
options.encoding,
false,
kUsePromises,
await PromisePrototypeThen(
binding.readdir(
pathModule.toNamespacedPath(direntPath),
options.encoding,
false,
kUsePromises,
),
undefined,
handleErrorFromBinding,
),
]);
}
@ -858,11 +937,15 @@ async function readdir(path, options) {
if (options.recursive) {
return readdirRecursive(path, options);
}
const result = await binding.readdir(
pathModule.toNamespacedPath(path),
options.encoding,
!!options.withFileTypes,
kUsePromises,
const result = await PromisePrototypeThen(
binding.readdir(
pathModule.toNamespacedPath(path),
options.encoding,
!!options.withFileTypes,
kUsePromises,
),
undefined,
handleErrorFromBinding,
);
return options.withFileTypes ?
getDirectoryEntriesPromise(path, result) :
@ -872,8 +955,12 @@ async function readdir(path, options) {
async function readlink(path, options) {
options = getOptions(options);
path = getValidatedPath(path, 'oldPath');
return binding.readlink(pathModule.toNamespacedPath(path),
options.encoding, kUsePromises);
return await PromisePrototypeThen(
binding.readlink(pathModule.toNamespacedPath(path),
options.encoding, kUsePromises),
undefined,
handleErrorFromBinding,
);
}
async function symlink(target, path, type_) {
@ -889,60 +976,96 @@ async function symlink(target, path, type_) {
}
target = getValidatedPath(target, 'target');
path = getValidatedPath(path);
return binding.symlink(preprocessSymlinkDestination(target, type, path),
pathModule.toNamespacedPath(path),
stringToSymlinkType(type),
kUsePromises);
return await PromisePrototypeThen(
binding.symlink(preprocessSymlinkDestination(target, type, path),
pathModule.toNamespacedPath(path),
stringToSymlinkType(type),
kUsePromises),
undefined,
handleErrorFromBinding,
);
}
async function fstat(handle, options = { bigint: false }) {
const result = await binding.fstat(handle.fd, options.bigint, kUsePromises);
const result = await PromisePrototypeThen(
binding.fstat(handle.fd, options.bigint, kUsePromises),
undefined,
handleErrorFromBinding,
);
return getStatsFromBinding(result);
}
async function lstat(path, options = { bigint: false }) {
path = getValidatedPath(path);
const result = await binding.lstat(pathModule.toNamespacedPath(path),
options.bigint, kUsePromises);
const result = await PromisePrototypeThen(
binding.lstat(pathModule.toNamespacedPath(path),
options.bigint, kUsePromises),
undefined,
handleErrorFromBinding,
);
return getStatsFromBinding(result);
}
async function stat(path, options = { bigint: false }) {
path = getValidatedPath(path);
const result = await binding.stat(pathModule.toNamespacedPath(path),
options.bigint, kUsePromises);
const result = await PromisePrototypeThen(
binding.stat(pathModule.toNamespacedPath(path),
options.bigint, kUsePromises),
undefined,
handleErrorFromBinding,
);
return getStatsFromBinding(result);
}
async function statfs(path, options = { bigint: false }) {
path = getValidatedPath(path);
const result = await binding.statfs(pathModule.toNamespacedPath(path),
options.bigint, kUsePromises);
const result = await PromisePrototypeThen(
binding.statfs(pathModule.toNamespacedPath(path),
options.bigint, kUsePromises),
undefined,
handleErrorFromBinding,
);
return getStatFsFromBinding(result);
}
async function link(existingPath, newPath) {
existingPath = getValidatedPath(existingPath, 'existingPath');
newPath = getValidatedPath(newPath, 'newPath');
return binding.link(pathModule.toNamespacedPath(existingPath),
pathModule.toNamespacedPath(newPath),
kUsePromises);
return await PromisePrototypeThen(
binding.link(pathModule.toNamespacedPath(existingPath),
pathModule.toNamespacedPath(newPath),
kUsePromises),
undefined,
handleErrorFromBinding,
);
}
async function unlink(path) {
path = getValidatedPath(path);
return binding.unlink(pathModule.toNamespacedPath(path), kUsePromises);
return await PromisePrototypeThen(
binding.unlink(pathModule.toNamespacedPath(path), kUsePromises),
undefined,
handleErrorFromBinding,
);
}
async function fchmod(handle, mode) {
mode = parseFileMode(mode, 'mode');
return binding.fchmod(handle.fd, mode, kUsePromises);
return await PromisePrototypeThen(
binding.fchmod(handle.fd, mode, kUsePromises),
undefined,
handleErrorFromBinding,
);
}
async function chmod(path, mode) {
path = getValidatedPath(path);
mode = parseFileMode(mode, 'mode');
return binding.chmod(pathModule.toNamespacedPath(path), mode, kUsePromises);
return await PromisePrototypeThen(
binding.chmod(pathModule.toNamespacedPath(path), mode, kUsePromises),
undefined,
handleErrorFromBinding,
);
}
async function lchmod(path, mode) {
@ -957,50 +1080,76 @@ async function lchown(path, uid, gid) {
path = getValidatedPath(path);
validateInteger(uid, 'uid', -1, kMaxUserId);
validateInteger(gid, 'gid', -1, kMaxUserId);
return binding.lchown(pathModule.toNamespacedPath(path),
uid, gid, kUsePromises);
return await PromisePrototypeThen(
binding.lchown(pathModule.toNamespacedPath(path), uid, gid, kUsePromises),
undefined,
handleErrorFromBinding,
);
}
async function fchown(handle, uid, gid) {
validateInteger(uid, 'uid', -1, kMaxUserId);
validateInteger(gid, 'gid', -1, kMaxUserId);
return binding.fchown(handle.fd, uid, gid, kUsePromises);
return await PromisePrototypeThen(
binding.fchown(handle.fd, uid, gid, kUsePromises),
undefined,
handleErrorFromBinding,
);
}
async function chown(path, uid, gid) {
path = getValidatedPath(path);
validateInteger(uid, 'uid', -1, kMaxUserId);
validateInteger(gid, 'gid', -1, kMaxUserId);
return binding.chown(pathModule.toNamespacedPath(path),
uid, gid, kUsePromises);
return await PromisePrototypeThen(
binding.chown(pathModule.toNamespacedPath(path), uid, gid, kUsePromises),
undefined,
handleErrorFromBinding,
);
}
async function utimes(path, atime, mtime) {
path = getValidatedPath(path);
return binding.utimes(pathModule.toNamespacedPath(path),
toUnixTimestamp(atime),
toUnixTimestamp(mtime),
kUsePromises);
return await PromisePrototypeThen(
binding.utimes(pathModule.toNamespacedPath(path),
toUnixTimestamp(atime),
toUnixTimestamp(mtime),
kUsePromises),
undefined,
handleErrorFromBinding,
);
}
async function futimes(handle, atime, mtime) {
atime = toUnixTimestamp(atime, 'atime');
mtime = toUnixTimestamp(mtime, 'mtime');
return binding.futimes(handle.fd, atime, mtime, kUsePromises);
return await PromisePrototypeThen(
binding.futimes(handle.fd, atime, mtime, kUsePromises),
undefined,
handleErrorFromBinding,
);
}
async function lutimes(path, atime, mtime) {
path = getValidatedPath(path);
return binding.lutimes(pathModule.toNamespacedPath(path),
toUnixTimestamp(atime),
toUnixTimestamp(mtime),
kUsePromises);
return await PromisePrototypeThen(
binding.lutimes(pathModule.toNamespacedPath(path),
toUnixTimestamp(atime),
toUnixTimestamp(mtime),
kUsePromises),
undefined,
handleErrorFromBinding,
);
}
async function realpath(path, options) {
options = getOptions(options);
path = getValidatedPath(path);
return binding.realpath(path, options.encoding, kUsePromises);
return await PromisePrototypeThen(
binding.realpath(path, options.encoding, kUsePromises),
undefined,
handleErrorFromBinding,
);
}
async function mkdtemp(prefix, options) {
@ -1016,7 +1165,11 @@ async function mkdtemp(prefix, options) {
path = Buffer.concat([prefix, Buffer.from('XXXXXX')]);
}
return binding.mkdtemp(path, options.encoding, kUsePromises);
return await PromisePrototypeThen(
binding.mkdtemp(path, options.encoding, kUsePromises),
undefined,
handleErrorFromBinding,
);
}
async function writeFile(path, data, options) {

View File

@ -95,9 +95,13 @@ fs.promises.access(readOnlyFile, fs.constants.R_OK)
assert.strictEqual(err.code, 'ENOENT');
assert.strictEqual(err.path, doesNotExist);
};
const expectedErrorPromise = (err) => {
expectedError(err);
assert.match(err.stack, /at async Object\.access/);
};
fs.access(doesNotExist, common.mustCall(expectedError));
fs.promises.access(doesNotExist)
.then(common.mustNotCall(), common.mustCall(expectedError))
.then(common.mustNotCall(), common.mustCall(expectedErrorPromise))
.catch(throwNextTick);
}

View File

@ -72,7 +72,7 @@ async function validateWrongSignalParam() {
async function validateZeroByteLiar() {
const originalFStat = fsBinding.fstat;
fsBinding.fstat = common.mustCall(
() => (/* stat fields */ [0, 1, 2, 3, 4, 5, 6, 7, 0 /* size */])
async () => (/* stat fields */ [0, 1, 2, 3, 4, 5, 6, 7, 0 /* size */])
);
const readBuffer = await readFile(fn);
assert.strictEqual(readBuffer.toString(), largeBuffer.toString());

View File

@ -57,7 +57,8 @@ assert.strictEqual(
{
code: 'ENOENT',
name: 'Error',
message: /^ENOENT: no such file or directory, access/
message: /^ENOENT: no such file or directory, access/,
stack: /at async Function\.rejects/
}
).then(common.mustCall());