Skip to content

Commit

Permalink
fs: add stacktrace to fs/promises
Browse files Browse the repository at this point in the history
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: nodejs#34817
  • Loading branch information
sapphi-red committed Sep 25, 2023
1 parent 77597d3 commit 5ec520c
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 40 deletions.
104 changes: 66 additions & 38 deletions lib/internal/fs/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const {
ArrayPrototypePush,
ArrayPrototypePop,
Error,
ErrorCaptureStackTrace,
MathMax,
MathMin,
NumberIsSafeInteger,
Expand Down Expand Up @@ -129,6 +130,11 @@ function lazyFsStreams() {
return fsStreams ??= require('internal/fs/streams');
}

function handleErrorFromBinding(error) {
ErrorCaptureStackTrace(error, handleErrorFromBinding);
return PromiseReject(error);
}

class FileHandle extends EventEmitter {
/**
* @param {InternalFSBinding.FileHandle | undefined} filehandle
Expand Down Expand Up @@ -480,7 +486,8 @@ async function readFileHandle(filehandle, options) {

checkAborted(signal);

const statFields = await binding.fstat(filehandle.fd, false, kUsePromises);
const statFields = await binding.fstat(filehandle.fd, false, kUsePromises)
.catch(handleErrorFromBinding);

checkAborted(signal);

Expand Down Expand Up @@ -512,7 +519,8 @@ async function readFileHandle(filehandle, options) {
}

const bytesRead = (await binding.read(filehandle.fd, buffer, offset,
length, -1, kUsePromises)) ?? 0;
length, -1, kUsePromises)
.catch(handleErrorFromBinding)) ?? 0;
totalRead += bytesRead;

if (bytesRead === 0 ||
Expand Down Expand Up @@ -561,7 +569,8 @@ async function access(path, mode = F_OK) {

mode = getValidMode(mode, 'access');
return binding.access(pathModule.toNamespacedPath(path), mode,
kUsePromises);
kUsePromises)
.catch(handleErrorFromBinding);
}

async function cp(src, dest, options) {
Expand All @@ -578,7 +587,8 @@ async function copyFile(src, dest, mode) {
return binding.copyFile(pathModule.toNamespacedPath(src),
pathModule.toNamespacedPath(dest),
mode,
kUsePromises);
kUsePromises)
.catch(handleErrorFromBinding);
}

// Note that unlike fs.open() which uses numeric file descriptors,
Expand All @@ -589,7 +599,8 @@ async function open(path, flags, mode) {
mode = parseFileMode(mode, 'mode', 0o666);
return new FileHandle(
await binding.openFileHandle(pathModule.toNamespacedPath(path),
flagsNumber, mode, kUsePromises));
flagsNumber, mode, kUsePromises)
.catch(handleErrorFromBinding));
}

async function read(handle, bufferOrParams, offset, length, position) {
Expand Down Expand Up @@ -640,7 +651,8 @@ async function read(handle, bufferOrParams, offset, length, position) {
position = -1;

const bytesRead = (await binding.read(handle.fd, buffer, offset, length,
position, kUsePromises)) || 0;
position, kUsePromises)
.catch(handleErrorFromBinding)) || 0;

return { __proto__: null, bytesRead, buffer };
}
Expand All @@ -652,7 +664,8 @@ async function readv(handle, buffers, position) {
position = null;

const bytesRead = (await binding.readBuffers(handle.fd, buffers, position,
kUsePromises)) || 0;
kUsePromises)
.catch(handleErrorFromBinding)) || 0;
return { __proto__: null, bytesRead, buffers };
}

Expand Down Expand Up @@ -682,14 +695,16 @@ async function write(handle, buffer, offsetOrOptions, length, position) {
validateOffsetLengthWrite(offset, length, buffer.byteLength);
const bytesWritten =
(await binding.writeBuffer(handle.fd, buffer, offset,
length, position, kUsePromises)) || 0;
length, position, kUsePromises)
.catch(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;
length, kUsePromises)
.catch(handleErrorFromBinding)) || 0;
return { __proto__: null, bytesWritten, buffer };
}

Expand All @@ -704,7 +719,8 @@ async function writev(handle, buffers, position) {
}

const bytesWritten = (await binding.writeBuffers(handle.fd, buffers, position,
kUsePromises)) || 0;
kUsePromises)
.catch(handleErrorFromBinding)) || 0;
return { __proto__: null, bytesWritten, buffers };
}

Expand All @@ -713,7 +729,8 @@ async function rename(oldPath, newPath) {
newPath = getValidatedPath(newPath, 'newPath');
return binding.rename(pathModule.toNamespacedPath(oldPath),
pathModule.toNamespacedPath(newPath),
kUsePromises);
kUsePromises)
.catch(handleErrorFromBinding);
}

async function truncate(path, len = 0) {
Expand All @@ -724,7 +741,7 @@ 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 binding.ftruncate(handle.fd, len, kUsePromises).catch(handleErrorFromBinding);
}

async function rm(path, options) {
Expand All @@ -745,15 +762,15 @@ async function rmdir(path, options) {
}
}

return binding.rmdir(path, kUsePromises);
return binding.rmdir(path, kUsePromises).catch(handleErrorFromBinding);
}

async function fdatasync(handle) {
return binding.fdatasync(handle.fd, kUsePromises);
return binding.fdatasync(handle.fd, kUsePromises).catch(handleErrorFromBinding);
}

async function fsync(handle) {
return binding.fsync(handle.fd, kUsePromises);
return binding.fsync(handle.fd, kUsePromises).catch(handleErrorFromBinding);
}

async function mkdir(path, options) {
Expand All @@ -769,7 +786,8 @@ async function mkdir(path, options) {

return binding.mkdir(pathModule.toNamespacedPath(path),
parseFileMode(mode, 'mode', 0o777), recursive,
kUsePromises);
kUsePromises)
.catch(handleErrorFromBinding);
}

async function readdirRecursive(originalPath, options) {
Expand All @@ -782,7 +800,7 @@ async function readdirRecursive(originalPath, options) {
options.encoding,
!!options.withFileTypes,
kUsePromises,
),
).catch(handleErrorFromBinding),
],
];

Expand All @@ -802,7 +820,7 @@ async function readdirRecursive(originalPath, options) {
options.encoding,
true,
kUsePromises,
),
).catch(handleErrorFromBinding),
]);
}
}
Expand All @@ -825,7 +843,7 @@ async function readdirRecursive(originalPath, options) {
options.encoding,
false,
kUsePromises,
),
).catch(handleErrorFromBinding),
]);
}
}
Expand All @@ -846,7 +864,7 @@ async function readdir(path, options) {
options.encoding,
!!options.withFileTypes,
kUsePromises,
);
).catch(handleErrorFromBinding);
return options.withFileTypes ?
getDirectoryEntriesPromise(path, result) :
result;
Expand All @@ -856,7 +874,8 @@ async function readlink(path, options) {
options = getOptions(options);
path = getValidatedPath(path, 'oldPath');
return binding.readlink(pathModule.toNamespacedPath(path),
options.encoding, kUsePromises);
options.encoding, kUsePromises)
.catch(handleErrorFromBinding);
}

async function symlink(target, path, type_) {
Expand All @@ -875,32 +894,36 @@ async function symlink(target, path, type_) {
return binding.symlink(preprocessSymlinkDestination(target, type, path),
pathModule.toNamespacedPath(path),
stringToSymlinkType(type),
kUsePromises);
kUsePromises)
.catch(handleErrorFromBinding);
}

async function fstat(handle, options = { bigint: false }) {
const result = await binding.fstat(handle.fd, options.bigint, kUsePromises);
const result = await binding.fstat(handle.fd, options.bigint, kUsePromises).catch(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);
options.bigint, kUsePromises)
.catch(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);
options.bigint, kUsePromises)
.catch(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);
options.bigint, kUsePromises)
.catch(handleErrorFromBinding);
return getStatFsFromBinding(result);
}

Expand All @@ -909,23 +932,24 @@ async function link(existingPath, newPath) {
newPath = getValidatedPath(newPath, 'newPath');
return binding.link(pathModule.toNamespacedPath(existingPath),
pathModule.toNamespacedPath(newPath),
kUsePromises);
kUsePromises)
.catch(handleErrorFromBinding);
}

async function unlink(path) {
path = getValidatedPath(path);
return binding.unlink(pathModule.toNamespacedPath(path), kUsePromises);
return binding.unlink(pathModule.toNamespacedPath(path), kUsePromises).catch(handleErrorFromBinding);
}

async function fchmod(handle, mode) {
mode = parseFileMode(mode, 'mode');
return binding.fchmod(handle.fd, mode, kUsePromises);
return binding.fchmod(handle.fd, mode, kUsePromises).catch(handleErrorFromBinding);
}

async function chmod(path, mode) {
path = getValidatedPath(path);
mode = parseFileMode(mode, 'mode');
return binding.chmod(pathModule.toNamespacedPath(path), mode, kUsePromises);
return binding.chmod(pathModule.toNamespacedPath(path), mode, kUsePromises).catch(handleErrorFromBinding);
}

async function lchmod(path, mode) {
Expand All @@ -941,49 +965,53 @@ async function lchown(path, uid, gid) {
validateInteger(uid, 'uid', -1, kMaxUserId);
validateInteger(gid, 'gid', -1, kMaxUserId);
return binding.lchown(pathModule.toNamespacedPath(path),
uid, gid, kUsePromises);
uid, gid, kUsePromises)
.catch(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 binding.fchown(handle.fd, uid, gid, kUsePromises).catch(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);
uid, gid, kUsePromises)
.catch(handleErrorFromBinding);
}

async function utimes(path, atime, mtime) {
path = getValidatedPath(path);
return binding.utimes(pathModule.toNamespacedPath(path),
toUnixTimestamp(atime),
toUnixTimestamp(mtime),
kUsePromises);
kUsePromises)
.catch(handleErrorFromBinding);
}

async function futimes(handle, atime, mtime) {
atime = toUnixTimestamp(atime, 'atime');
mtime = toUnixTimestamp(mtime, 'mtime');
return binding.futimes(handle.fd, atime, mtime, kUsePromises);
return binding.futimes(handle.fd, atime, mtime, kUsePromises).catch(handleErrorFromBinding);
}

async function lutimes(path, atime, mtime) {
path = getValidatedPath(path);
return binding.lutimes(pathModule.toNamespacedPath(path),
toUnixTimestamp(atime),
toUnixTimestamp(mtime),
kUsePromises);
kUsePromises)
.catch(handleErrorFromBinding);
}

async function realpath(path, options) {
options = getOptions(options);
path = getValidatedPath(path);
return binding.realpath(path, options.encoding, kUsePromises);
return binding.realpath(path, options.encoding, kUsePromises).catch(handleErrorFromBinding);
}

async function mkdtemp(prefix, options) {
Expand All @@ -999,7 +1027,7 @@ async function mkdtemp(prefix, options) {
path = Buffer.concat([prefix, Buffer.from('XXXXXX')]);
}

return binding.mkdtemp(path, options.encoding, kUsePromises);
return binding.mkdtemp(path, options.encoding, kUsePromises).catch(handleErrorFromBinding);
}

async function writeFile(path, data, options) {
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-fs-promises-readfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-fs-promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -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/
}
);

Expand Down

0 comments on commit 5ec520c

Please sign in to comment.