Skip to content

Commit 18a8187

Browse files
anonriganonrig
authored
fs: improve error performance of readdirSync
PR-URL: #50131 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
1 parent 9df864d commit 18a8187

File tree

5 files changed

+28
-35
lines changed

5 files changed

+28
-35
lines changed

‎lib/fs.js‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1410,17 +1410,16 @@ function readdirSyncRecursive(basePath, options) {
14101410
const readdirResults = [];
14111411
const pathsQueue = [basePath];
14121412

1413-
const ctx = { path: basePath };
14141413
function read(path) {
1415-
ctx.path = path;
14161414
const readdirResult = binding.readdir(
14171415
pathModule.toNamespacedPath(path),
14181416
encoding,
14191417
withFileTypes,
1420-
undefined,
1421-
ctx,
14221418
);
1423-
handleErrorFromBinding(ctx);
1419+
1420+
if (readdirResult === undefined) {
1421+
return;
1422+
}
14241423

14251424
if (withFileTypes) {
14261425
// Calling `readdir` with `withFileTypes=true`, the result is an array of arrays.
@@ -1519,12 +1518,13 @@ function readdirSync(path, options) {
15191518
return readdirSyncRecursive(path, options);
15201519
}
15211520

1522-
const ctx = { path };
1523-
const result = binding.readdir(pathModule.toNamespacedPath(path),
1524-
options.encoding, !!options.withFileTypes,
1525-
undefined, ctx);
1526-
handleErrorFromBinding(ctx);
1527-
return options.withFileTypes ? getDirents(path, result) : result;
1521+
const result = binding.readdir(
1522+
pathModule.toNamespacedPath(path),
1523+
options.encoding,
1524+
!!options.withFileTypes,
1525+
);
1526+
1527+
return result !== undefined && options.withFileTypes ? getDirents(path, result) : result;
15281528
}
15291529

15301530
/**

‎src/node_file.cc‎

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1891,8 +1891,8 @@ static void ReadDir(const FunctionCallbackInfo<Value>& args) {
18911891

18921892
bool with_types = args[2]->IsTrue();
18931893

1894-
FSReqBase* req_wrap_async = GetReqWrap(args, 3);
1895-
if (req_wrap_async != nullptr) { // readdir(path, encoding, withTypes, req)
1894+
if (argc > 3) { // readdir(path, encoding, withTypes, req)
1895+
FSReqBase* req_wrap_async = GetReqWrap(args, 3);
18961896
req_wrap_async->set_with_file_types(with_types);
18971897
FS_ASYNC_TRACE_BEGIN1(
18981898
UV_FS_SCANDIR, req_wrap_async, "path", TRACE_STR_COPY(*path))
@@ -1905,18 +1905,16 @@ static void ReadDir(const FunctionCallbackInfo<Value>& args) {
19051905
uv_fs_scandir,
19061906
*path,
19071907
0 /*flags*/);
1908-
} else { // readdir(path, encoding, withTypes, undefined, ctx)
1909-
CHECK_EQ(argc, 5);
1910-
FSReqWrapSync req_wrap_sync;
1908+
} else { // readdir(path, encoding, withTypes)
1909+
FSReqWrapSync req_wrap_sync("scandir", *path);
19111910
FS_SYNC_TRACE_BEGIN(readdir);
1912-
int err = SyncCall(env, args[4], &req_wrap_sync, "scandir",
1913-
uv_fs_scandir, *path, 0 /*flags*/);
1911+
int err = SyncCallAndThrowOnError(
1912+
env, &req_wrap_sync, uv_fs_scandir, *path, 0 /*flags*/);
19141913
FS_SYNC_TRACE_END(readdir);
1915-
if (err < 0) {
1916-
return; // syscall failed, no need to continue, error info is in ctx
1914+
if (is_uv_error(err)) {
1915+
return;
19171916
}
19181917

1919-
CHECK_GE(req_wrap_sync.req.result, 0);
19201918
int r;
19211919
std::vector<Local<Value>> name_v;
19221920
std::vector<Local<Value>> type_v;
@@ -1927,12 +1925,8 @@ static void ReadDir(const FunctionCallbackInfo<Value>& args) {
19271925
r = uv_fs_scandir_next(&(req_wrap_sync.req), &ent);
19281926
if (r == UV_EOF)
19291927
break;
1930-
if (r != 0) {
1931-
Local<Object> ctx = args[4].As<Object>();
1932-
ctx->Set(env->context(), env->errno_string(),
1933-
Integer::New(isolate, r)).Check();
1934-
ctx->Set(env->context(), env->syscall_string(),
1935-
OneByteString(isolate, "readdir")).Check();
1928+
if (is_uv_error(r)) {
1929+
env->ThrowUVException(r, "scandir", nullptr, *path);
19361930
return;
19371931
}
19381932

@@ -1943,8 +1937,7 @@ static void ReadDir(const FunctionCallbackInfo<Value>& args) {
19431937
&error);
19441938

19451939
if (filename.IsEmpty()) {
1946-
Local<Object> ctx = args[4].As<Object>();
1947-
ctx->Set(env->context(), env->error_string(), error).Check();
1940+
isolate->ThrowException(error);
19481941
return;
19491942
}
19501943

‎test/parallel/test-fs-readdir-types.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ tmpdir.refresh();
2929

3030
// Create the necessary files
3131
files.forEach(function(currentFile) {
32-
fs.closeSync(fs.openSync(`${readdirDir}/${currentFile}`, 'w'));
32+
fs.writeFileSync(`${readdirDir}/${currentFile}`, '', 'utf8');
3333
});
3434

3535

@@ -95,7 +95,7 @@ binding.readdir = common.mustCall((path, encoding, types, req, ctx) => {
9595
};
9696
oldReaddir(path, encoding, types, req);
9797
} else {
98-
const results = oldReaddir(path, encoding, types, req, ctx);
98+
const results = oldReaddir(path, encoding, types);
9999
results[1] = results[1].map(() => UNKNOWN);
100100
return results;
101101
}

‎test/parallel/test-repl-underscore.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -180,9 +180,9 @@ function testError() {
180180
/^Uncaught Error: ENOENT: no such file or directory, scandir '.*nonexistent\?'/,
181181
/Object\.readdirSync/,
182182
/^ {2}errno: -(2|4058),$/,
183-
" syscall: 'scandir',",
184183
" code: 'ENOENT',",
185-
" path: '/nonexistent?'",
184+
" syscall: 'scandir',",
185+
/^ {2}path: '*'/,
186186
'}',
187187
"'ENOENT'",
188188
"'scandir'",

‎typings/internalBinding/fs.d.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,8 +166,8 @@ declare namespace InternalFSBinding {
166166
function readdir(path: StringOrBuffer, encoding: unknown, withFileTypes: true, req: FSReqCallback<[string[], number[]]>): void;
167167
function readdir(path: StringOrBuffer, encoding: unknown, withFileTypes: false, req: FSReqCallback<string[]>): void;
168168
function readdir(path: StringOrBuffer, encoding: unknown, withFileTypes: boolean, req: undefined, ctx: FSSyncContext): string[] | [string[], number[]];
169-
function readdir(path: StringOrBuffer, encoding: unknown, withFileTypes: true, req: undefined, ctx: FSSyncContext): [string[], number[]];
170-
function readdir(path: StringOrBuffer, encoding: unknown, withFileTypes: false, req: undefined, ctx: FSSyncContext): string[];
169+
function readdir(path: StringOrBuffer, encoding: unknown, withFileTypes: true): [string[], number[]];
170+
function readdir(path: StringOrBuffer, encoding: unknown, withFileTypes: false): string[];
171171
function readdir(path: StringOrBuffer, encoding: unknown, withFileTypes: boolean, usePromises: typeof kUsePromises): Promise<string[] | [string[], number[]]>;
172172
function readdir(path: StringOrBuffer, encoding: unknown, withFileTypes: true, usePromises: typeof kUsePromises): Promise<[string[], number[]]>;
173173
function readdir(path: StringOrBuffer, encoding: unknown, withFileTypes: false, usePromises: typeof kUsePromises): Promise<string[]>;

0 commit comments

Comments
 (0)