diff --git a/lib/fs.js b/lib/fs.js index db262f5da8fc22..c8206fc9d882a9 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -441,11 +441,7 @@ fs.existsSync = function(path) { return false; } nullCheck(path); - const ctx = { path }; - binding.stat(pathModule.toNamespacedPath(path), undefined, ctx); - if (ctx.errno !== undefined) { - return false; - } + binding.stat(pathModule.toNamespacedPath(path)); return true; } catch (e) { return false; @@ -633,11 +629,12 @@ function readFileAfterClose(err) { } function tryStatSync(fd, isUserFd) { - const ctx = {}; - binding.fstat(fd, undefined, ctx); - if (ctx.errno !== undefined && !isUserFd) { - fs.closeSync(fd); - throw new errors.uvException(ctx); + var threw = true; + try { + binding.fstat(fd); + threw = false; + } finally { + if (threw && !isUserFd) fs.closeSync(fd); } } @@ -1114,11 +1111,7 @@ fs.stat = function(path, callback) { fs.fstatSync = function(fd) { validateUint32(fd, 'fd'); - const ctx = { fd }; - binding.fstat(fd, undefined, ctx); - if (ctx.errno !== undefined) { - throw new errors.uvException(ctx); - } + binding.fstat(fd); return statsFromValues(); }; @@ -1126,11 +1119,7 @@ fs.lstatSync = function(path) { handleError((path = getPathFromURL(path))); nullCheck(path); validatePath(path); - const ctx = { path }; - binding.lstat(pathModule.toNamespacedPath(path), undefined, ctx); - if (ctx.errno !== undefined) { - throw new errors.uvException(ctx); - } + binding.lstat(pathModule.toNamespacedPath(path)); return statsFromValues(); }; @@ -1138,11 +1127,7 @@ fs.statSync = function(path) { handleError((path = getPathFromURL(path))); nullCheck(path); validatePath(path); - const ctx = { path }; - binding.stat(pathModule.toNamespacedPath(path), undefined, ctx); - if (ctx.errno !== undefined) { - throw new errors.uvException(ctx); - } + binding.stat(pathModule.toNamespacedPath(path)); return statsFromValues(); }; @@ -1881,11 +1866,7 @@ fs.realpathSync = function realpathSync(p, options) { // On windows, check that the root exists. On unix there is no need. if (isWindows && !knownHard[base]) { - const ctx = { path: base }; - binding.lstat(pathModule.toNamespacedPath(base), undefined, ctx); - if (ctx.errno !== undefined) { - throw new errors.uvException(ctx); - } + binding.lstat(pathModule.toNamespacedPath(base)); knownHard[base] = true; } @@ -1925,11 +1906,7 @@ fs.realpathSync = function realpathSync(p, options) { // for our internal use. var baseLong = pathModule.toNamespacedPath(base); - const ctx = { path: base }; - binding.lstat(baseLong, undefined, ctx); - if (ctx.errno !== undefined) { - throw new errors.uvException(ctx); - } + binding.lstat(baseLong); if ((statValues[1/*mode*/] & S_IFMT) !== S_IFLNK) { knownHard[base] = true; @@ -1950,11 +1927,7 @@ fs.realpathSync = function realpathSync(p, options) { } } if (linkTarget === null) { - const ctx = { path: base }; - binding.stat(baseLong, undefined, ctx); - if (ctx.errno !== undefined) { - throw new errors.uvException(ctx); - } + binding.stat(baseLong); linkTarget = binding.readlink(baseLong); } resolvedLink = pathModule.resolve(previous, linkTarget); @@ -1972,11 +1945,7 @@ fs.realpathSync = function realpathSync(p, options) { // On windows, check that the root exists. On unix there is no need. if (isWindows && !knownHard[base]) { - const ctx = { path: base }; - binding.lstat(pathModule.toNamespacedPath(base), undefined, ctx); - if (ctx.errno !== undefined) { - throw new errors.uvException(ctx); - } + binding.lstat(pathModule.toNamespacedPath(base)); knownHard[base] = true; } } diff --git a/src/node_file.cc b/src/node_file.cc index c9bbea1ee5f621..a7581099f6b0e6 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -375,10 +375,11 @@ inline FSReqWrap* AsyncCall(Environment* env, // the error number and the syscall in the context instead of // creating an error in the C++ land. template -inline int SyncCall(Environment* env, Local ctx, fs_req_wrap* req_wrap, +inline void SyncCall(Environment* env, Local ctx, const char* syscall, Func fn, Args... args) { + fs_req_wrap req_wrap; env->PrintSyncTrace(); - int err = fn(env->event_loop(), &(req_wrap->req), args..., nullptr); + int err = fn(env->event_loop(), &req_wrap.req, args..., nullptr); if (err < 0) { Local context = env->context(); Local ctx_obj = ctx->ToObject(context).ToLocalChecked(); @@ -390,7 +391,6 @@ inline int SyncCall(Environment* env, Local ctx, fs_req_wrap* req_wrap, env->syscall_string(), OneByteString(isolate, syscall)).FromJust(); } - return err; } #define SYNC_DEST_CALL(func, path, dest, ...) \ @@ -426,8 +426,7 @@ void Access(const FunctionCallbackInfo& args) { AsyncCall(env, args, "access", UTF8, AfterNoArgs, uv_fs_access, *path, mode); } else { // access(path, mode, undefined, ctx) - fs_req_wrap req_wrap; - SyncCall(env, args[3], &req_wrap, "access", uv_fs_access, *path, mode); + SyncCall(env, args[3], "access", uv_fs_access, *path, mode); } } @@ -447,8 +446,7 @@ void Close(const FunctionCallbackInfo& args) { AsyncCall(env, args, "close", UTF8, AfterNoArgs, uv_fs_close, fd); } else { // close(fd, undefined, ctx) - fs_req_wrap req_wrap; - SyncCall(env, args[2], &req_wrap, "close", uv_fs_close, fd); + SyncCall(env, args[2], "close", uv_fs_close, fd); } } @@ -539,7 +537,6 @@ static void InternalModuleStat(const FunctionCallbackInfo& args) { static void Stat(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - Local context = env->context(); CHECK_GE(args.Length(), 1); @@ -550,20 +547,15 @@ static void Stat(const FunctionCallbackInfo& args) { CHECK_EQ(args.Length(), 2); AsyncCall(env, args, "stat", UTF8, AfterStat, uv_fs_stat, *path); - } else { // stat(path, undefined, ctx) - CHECK_EQ(args.Length(), 3); - fs_req_wrap req_wrap; - int err = SyncCall(env, args[2], &req_wrap, "stat", uv_fs_stat, *path); - if (err == 0) { - FillStatsArray(env->fs_stats_field_array(), - static_cast(req_wrap.req.ptr)); - } + } else { // stat(path) + SYNC_CALL(stat, *path, *path) + FillStatsArray(env->fs_stats_field_array(), + static_cast(SYNC_REQ.ptr)); } } static void LStat(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - Local context = env->context(); CHECK_GE(args.Length(), 1); @@ -574,37 +566,28 @@ static void LStat(const FunctionCallbackInfo& args) { CHECK_EQ(args.Length(), 2); AsyncCall(env, args, "lstat", UTF8, AfterStat, uv_fs_lstat, *path); - } else { // lstat(path, undefined, ctx) - CHECK_EQ(args.Length(), 3); - fs_req_wrap req_wrap; - int err = SyncCall(env, args[2], &req_wrap, "lstat", uv_fs_lstat, *path); - if (err == 0) { - FillStatsArray(env->fs_stats_field_array(), - static_cast(req_wrap.req.ptr)); - } + } else { // lstat(path) + SYNC_CALL(lstat, *path, *path) + FillStatsArray(env->fs_stats_field_array(), + static_cast(SYNC_REQ.ptr)); } } static void FStat(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - Local context = env->context(); CHECK(args[0]->IsInt32()); - int fd = static_cast(args[0]->Int32Value(context).FromJust()); + int fd = args[0]->Int32Value(); if (args[1]->IsObject()) { // fstat(fd, req) CHECK_EQ(args.Length(), 2); AsyncCall(env, args, "fstat", UTF8, AfterStat, uv_fs_fstat, fd); - } else { // fstat(fd, undefined, ctx) - CHECK_EQ(args.Length(), 3); - fs_req_wrap req_wrap; - int err = SyncCall(env, args[2], &req_wrap, "fstat", uv_fs_fstat, fd); - if (err == 0) { - FillStatsArray(env->fs_stats_field_array(), - static_cast(req_wrap.req.ptr)); - } + } else { // fstat(fd) + SYNC_CALL(fstat, nullptr, fd) + FillStatsArray(env->fs_stats_field_array(), + static_cast(SYNC_REQ.ptr)); } } diff --git a/test/parallel/test-fs-error-messages.js b/test/parallel/test-fs-error-messages.js index 625b791b0b1a7a..f4d203e7d5e79f 100644 --- a/test/parallel/test-fs-error-messages.js +++ b/test/parallel/test-fs-error-messages.js @@ -20,7 +20,7 @@ // USE OR OTHER DEALINGS IN THE SOFTWARE. 'use strict'; -const common = require('../common'); +require('../common'); const fixtures = require('../common/fixtures'); const assert = require('assert'); const fs = require('fs'); @@ -29,105 +29,72 @@ const existingFile = fixtures.path('exit.js'); const existingFile2 = fixtures.path('create-file.js'); const existingDir = fixtures.path('empty'); const existingDir2 = fixtures.path('keys'); -const uv = process.binding('uv'); // ASYNC_CALL -fs.stat(fn, common.mustCall((err) => { +fs.stat(fn, function(err) { assert.strictEqual(fn, err.path); - assert.strictEqual( - err.message, - `ENOENT: no such file or directory, stat '${fn}'`); - assert.strictEqual(err.errno, uv.UV_ENOENT); - assert.strictEqual(err.code, 'ENOENT'); - assert.strictEqual(err.syscall, 'stat'); -})); - -fs.lstat(fn, common.mustCall((err) => { - assert.strictEqual(fn, err.path); - assert.strictEqual( - err.message, - `ENOENT: no such file or directory, lstat '${fn}'`); - assert.strictEqual(err.errno, uv.UV_ENOENT); - assert.strictEqual(err.code, 'ENOENT'); - assert.strictEqual(err.syscall, 'lstat'); -})); - -{ - const fd = fs.openSync(existingFile, 'r'); - fs.closeSync(fd); - fs.fstat(fd, common.mustCall((err) => { - assert.strictEqual(err.message, 'EBADF: bad file descriptor, fstat'); - assert.strictEqual(err.errno, uv.UV_EBADF); - assert.strictEqual(err.code, 'EBADF'); - assert.strictEqual(err.syscall, 'fstat'); - })); -} + assert.ok(err.message.includes(fn)); +}); -fs.realpath(fn, common.mustCall((err) => { - assert.strictEqual(fn, err.path); - assert.strictEqual( - err.message, - `ENOENT: no such file or directory, lstat '${fn}'`); - assert.strictEqual(err.errno, uv.UV_ENOENT); - assert.strictEqual(err.code, 'ENOENT'); - assert.strictEqual(err.syscall, 'lstat'); -})); - -fs.readlink(fn, common.mustCall((err) => { +fs.lstat(fn, function(err) { assert.ok(err.message.includes(fn)); -})); +}); -fs.link(fn, 'foo', common.mustCall((err) => { +fs.readlink(fn, function(err) { assert.ok(err.message.includes(fn)); -})); +}); -fs.link(existingFile, existingFile2, common.mustCall((err) => { +fs.link(fn, 'foo', function(err) { + assert.ok(err.message.includes(fn)); +}); + +fs.link(existingFile, existingFile2, function(err) { assert.ok(err.message.includes(existingFile)); assert.ok(err.message.includes(existingFile2)); -})); +}); -fs.symlink(existingFile, existingFile2, common.mustCall((err) => { +fs.symlink(existingFile, existingFile2, function(err) { assert.ok(err.message.includes(existingFile)); assert.ok(err.message.includes(existingFile2)); -})); +}); -fs.unlink(fn, common.mustCall((err) => { +fs.unlink(fn, function(err) { assert.ok(err.message.includes(fn)); -})); +}); -fs.rename(fn, 'foo', common.mustCall((err) => { +fs.rename(fn, 'foo', function(err) { assert.ok(err.message.includes(fn)); -})); +}); -fs.rename(existingDir, existingDir2, common.mustCall((err) => { +fs.rename(existingDir, existingDir2, function(err) { assert.ok(err.message.includes(existingDir)); assert.ok(err.message.includes(existingDir2)); -})); +}); -fs.rmdir(fn, common.mustCall((err) => { +fs.rmdir(fn, function(err) { assert.ok(err.message.includes(fn)); -})); +}); -fs.mkdir(existingFile, 0o666, common.mustCall((err) => { +fs.mkdir(existingFile, 0o666, function(err) { assert.ok(err.message.includes(existingFile)); -})); +}); -fs.rmdir(existingFile, common.mustCall((err) => { +fs.rmdir(existingFile, function(err) { assert.ok(err.message.includes(existingFile)); -})); +}); -fs.chmod(fn, 0o666, common.mustCall((err) => { +fs.chmod(fn, 0o666, function(err) { assert.ok(err.message.includes(fn)); -})); +}); -fs.open(fn, 'r', 0o666, common.mustCall((err) => { +fs.open(fn, 'r', 0o666, function(err) { assert.ok(err.message.includes(fn)); -})); +}); -fs.readFile(fn, common.mustCall((err) => { +fs.readFile(fn, function(err) { assert.ok(err.message.includes(fn)); -})); +}); // Sync @@ -139,13 +106,7 @@ try { fs.statSync(fn); } catch (err) { errors.push('stat'); - assert.strictEqual(fn, err.path); - assert.strictEqual( - err.message, - `ENOENT: no such file or directory, stat '${fn}'`); - assert.strictEqual(err.errno, uv.UV_ENOENT); - assert.strictEqual(err.code, 'ENOENT'); - assert.strictEqual(err.syscall, 'stat'); + assert.ok(err.message.includes(fn)); } try { @@ -169,40 +130,7 @@ try { fs.lstatSync(fn); } catch (err) { errors.push('lstat'); - assert.strictEqual(fn, err.path); - assert.strictEqual( - err.message, - `ENOENT: no such file or directory, lstat '${fn}'`); - assert.strictEqual(err.errno, uv.UV_ENOENT); - assert.strictEqual(err.code, 'ENOENT'); - assert.strictEqual(err.syscall, 'lstat'); -} - -try { - ++expected; - const fd = fs.openSync(existingFile, 'r'); - fs.closeSync(fd); - fs.fstatSync(fd); -} catch (err) { - errors.push('fstat'); - assert.strictEqual(err.message, 'EBADF: bad file descriptor, fstat'); - assert.strictEqual(err.errno, uv.UV_EBADF); - assert.strictEqual(err.code, 'EBADF'); - assert.strictEqual(err.syscall, 'fstat'); -} - -try { - ++expected; - fs.realpathSync(fn); -} catch (err) { - errors.push('realpath'); - assert.strictEqual(fn, err.path); - assert.strictEqual( - err.message, - `ENOENT: no such file or directory, lstat '${fn}'`); - assert.strictEqual(err.errno, uv.UV_ENOENT); - assert.strictEqual(err.code, 'ENOENT'); - assert.strictEqual(err.syscall, 'lstat'); + assert.ok(err.message.includes(fn)); } try { @@ -296,4 +224,9 @@ try { assert.ok(err.message.includes(fn)); } -assert.strictEqual(expected, errors.length); +process.on('exit', function() { + assert.strictEqual( + expected, errors.length, + `Test fs sync exceptions raised, got ${errors.length} expected ${expected}` + ); +}); diff --git a/test/parallel/test-fs-sync-fd-leak.js b/test/parallel/test-fs-sync-fd-leak.js index e7107546e5b217..7e785ea3a2a82b 100644 --- a/test/parallel/test-fs-sync-fd-leak.js +++ b/test/parallel/test-fs-sync-fd-leak.js @@ -23,7 +23,6 @@ require('../common'); const assert = require('assert'); const fs = require('fs'); -const uv = process.binding('uv'); // ensure that (read|write|append)FileSync() closes the file descriptor fs.openSync = function() { @@ -40,30 +39,29 @@ fs.writeSync = function() { throw new Error('BAM'); }; -process.binding('fs').fstat = function(fd, _, ctx) { - ctx.errno = uv.UV_EBADF; - ctx.syscall = 'fstat'; +process.binding('fs').fstat = function() { + throw new Error('BAM'); }; let close_called = 0; ensureThrows(function() { fs.readFileSync('dummy'); -}, 'EBADF: bad file descriptor, fstat'); +}); ensureThrows(function() { fs.writeFileSync('dummy', 'xxx'); -}, 'BAM'); +}); ensureThrows(function() { fs.appendFileSync('dummy', 'xxx'); -}, 'BAM'); +}); -function ensureThrows(cb, message) { +function ensureThrows(cb) { let got_exception = false; close_called = 0; try { cb(); } catch (e) { - assert.strictEqual(e.message, message); + assert.strictEqual(e.message, 'BAM'); got_exception = true; }