From 45948e054d13fdf8ae7c8bfd12d7da58c94666b9 Mon Sep 17 00:00:00 2001 From: visionmedia Date: Fri, 14 May 2010 07:52:49 -0700 Subject: [PATCH] Added better error messages for async and sync fs calls with paths --- src/node.cc | 22 +++-- src/node.h | 3 +- src/node_file.cc | 40 ++++++--- test/simple/test-fs-error-messages.js | 163 ++++++++++++++++++++++++++++++++++ 4 files changed, 209 insertions(+), 19 deletions(-) create mode 100644 test/simple/test-fs-error-messages.js diff --git a/src/node.cc b/src/node.cc index c9452a7..d220579 100644 --- a/src/node.cc +++ b/src/node.cc @@ -51,6 +51,7 @@ static Persistent process; static Persistent errno_symbol; static Persistent syscall_symbol; +static Persistent errpath_symbol; static Persistent dev_symbol; static Persistent ino_symbol; @@ -737,7 +738,9 @@ const char *signo_string(int signo) { Local ErrnoException(int errorno, const char *syscall, - const char *msg) { + const char *msg, + const char *path) { + Local e; Local estring = String::NewSymbol(errno_string(errorno)); if (!msg[0]) msg = strerror(errorno); Local message = String::NewSymbol(msg); @@ -745,16 +748,25 @@ Local ErrnoException(int errorno, Local cons1 = String::Concat(estring, String::NewSymbol(", ")); Local cons2 = String::Concat(cons1, message); - Local e = Exception::Error(cons2); - - Local obj = e->ToObject(); - if (errno_symbol.IsEmpty()) { syscall_symbol = NODE_PSYMBOL("syscall"); errno_symbol = NODE_PSYMBOL("errno"); + errpath_symbol = NODE_PSYMBOL("path"); + } + + if (path) { + Local cons3 = String::Concat(cons2, String::NewSymbol(" '")); + Local cons4 = String::Concat(cons3, String::New(path)); + Local cons5 = String::Concat(cons4, String::NewSymbol("'")); + e = Exception::Error(cons5); + } else { + e = Exception::Error(cons2); } + Local obj = e->ToObject(); + obj->Set(errno_symbol, Integer::New(errorno)); + if (path) obj->Set(errpath_symbol, String::New(path)); if (syscall) obj->Set(syscall_symbol, String::NewSymbol(syscall)); return e; } diff --git a/src/node.h b/src/node.h index bb9e90e..065bf2c 100644 --- a/src/node.h +++ b/src/node.h @@ -78,7 +78,8 @@ static inline void cb_destroy(v8::Persistent * cb) { v8::Local ErrnoException(int errorno, const char *syscall = NULL, - const char *msg = ""); + const char *msg = "", + const char *path = NULL); const char *signo_string(int errorno); } // namespace node diff --git a/src/node_file.cc b/src/node_file.cc index 1aee4b9..f36b79e 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -41,12 +41,26 @@ static int After(eio_req *req) { if (req->errorno != 0) { argc = 1; - argv[0] = ErrnoException(req->errorno); + switch (req->type) { + case EIO_STAT: + case EIO_LSTAT: + case EIO_LINK: + case EIO_UNLINK: + case EIO_RMDIR: + case EIO_RENAME: + case EIO_READLINK: + case EIO_OPEN: + case EIO_CHMOD: + case EIO_MKDIR: + argv[0] = ErrnoException(req->errorno, NULL, "", static_cast(req->ptr1)); + break; + default: + argv[0] = ErrnoException(req->errorno); + } } else { // Note: the error is always given the first argument of the callback. // If there is no error then then the first argument is null. argv[0] = Local::New(Null()); - switch (req->type) { case EIO_CLOSE: case EIO_RENAME: @@ -194,7 +208,7 @@ static Handle Stat(const Arguments& args) { } else { struct stat s; int ret = stat(*path, &s); - if (ret != 0) return ThrowException(ErrnoException(errno)); + if (ret != 0) return ThrowException(ErrnoException(errno, NULL, "", *path)); return scope.Close(BuildStatsObject(&s)); } } @@ -213,7 +227,7 @@ static Handle LStat(const Arguments& args) { } else { struct stat s; int ret = lstat(*path, &s); - if (ret != 0) return ThrowException(ErrnoException(errno)); + if (ret != 0) return ThrowException(ErrnoException(errno, NULL, "", *path)); return scope.Close(BuildStatsObject(&s)); } } @@ -270,7 +284,7 @@ static Handle Link(const Arguments& args) { ASYNC_CALL(link, args[2], *orig_path, *new_path) } else { int ret = link(*orig_path, *new_path); - if (ret != 0) return ThrowException(ErrnoException(errno)); + if (ret != 0) return ThrowException(ErrnoException(errno, NULL, "", *orig_path)); return Undefined(); } } @@ -289,7 +303,7 @@ static Handle ReadLink(const Arguments& args) { } else { char buf[PATH_MAX]; ssize_t bz = readlink(*path, buf, PATH_MAX); - if (bz == -1) return ThrowException(ErrnoException(errno)); + if (bz == -1) return ThrowException(ErrnoException(errno, NULL, "", *path)); return scope.Close(String::New(buf, bz)); } } @@ -308,7 +322,7 @@ static Handle Rename(const Arguments& args) { ASYNC_CALL(rename, args[2], *old_path, *new_path) } else { int ret = rename(*old_path, *new_path); - if (ret != 0) return ThrowException(ErrnoException(errno)); + if (ret != 0) return ThrowException(ErrnoException(errno, NULL, "", *old_path)); return Undefined(); } } @@ -385,7 +399,7 @@ static Handle Unlink(const Arguments& args) { ASYNC_CALL(unlink, args[1], *path) } else { int ret = unlink(*path); - if (ret != 0) return ThrowException(ErrnoException(errno)); + if (ret != 0) return ThrowException(ErrnoException(errno, NULL, "", *path)); return Undefined(); } } @@ -403,7 +417,7 @@ static Handle RMDir(const Arguments& args) { ASYNC_CALL(rmdir, args[1], *path) } else { int ret = rmdir(*path); - if (ret != 0) return ThrowException(ErrnoException(errno)); + if (ret != 0) return ThrowException(ErrnoException(errno, NULL, "", *path)); return Undefined(); } } @@ -422,7 +436,7 @@ static Handle MKDir(const Arguments& args) { ASYNC_CALL(mkdir, args[2], *path, mode) } else { int ret = mkdir(*path, mode); - if (ret != 0) return ThrowException(ErrnoException(errno)); + if (ret != 0) return ThrowException(ErrnoException(errno, NULL, "", *path)); return Undefined(); } } @@ -465,7 +479,7 @@ static Handle ReadDir(const Arguments& args) { ASYNC_CALL(readdir, args[1], *path, 0 /*flags*/) } else { DIR *dir = opendir(*path); - if (!dir) return ThrowException(ErrnoException(errno)); + if (!dir) return ThrowException(ErrnoException(errno, NULL, "", *path)); struct dirent *ent; @@ -506,7 +520,7 @@ static Handle Open(const Arguments& args) { ASYNC_CALL(open, args[3], *path, flags, mode) } else { int fd = open(*path, flags, mode); - if (fd < 0) return ThrowException(ErrnoException(errno)); + if (fd < 0) return ThrowException(ErrnoException(errno, NULL, "", *path)); return scope.Close(Integer::New(fd)); } } @@ -786,7 +800,7 @@ static Handle Chmod(const Arguments& args){ ASYNC_CALL(chmod, args[2], *path, mode); } else { int ret = chmod(*path, mode); - if (ret != 0) return ThrowException(ErrnoException(errno)); + if (ret != 0) return ThrowException(ErrnoException(errno, NULL, "", *path)); return Undefined(); } } diff --git a/test/simple/test-fs-error-messages.js b/test/simple/test-fs-error-messages.js new file mode 100644 index 0000000..5284f49 --- /dev/null +++ b/test/simple/test-fs-error-messages.js @@ -0,0 +1,163 @@ +require('../common'); + +var path = require('path'), + fs = require('fs'), + fn = path.join(fixturesDir, 'non-existent'), + existingFile = path.join(fixturesDir, 'exit.js'); + +// ASYNC_CALL + +fs.stat(fn, function(err) { + assert.equal(fn, err.path) + assert.ok(0 <= err.message.indexOf(fn)); +}); + +fs.lstat(fn, function(err) { + assert.ok(0 <= err.message.indexOf(fn)); +}); + +fs.readlink(fn, function(err) { + assert.ok(0 <= err.message.indexOf(fn)); +}); + +fs.link(fn, 'foo', function(err) { + assert.ok(0 <= err.message.indexOf(fn)); +}); + +fs.unlink(fn, function(err) { + assert.ok(0 <= err.message.indexOf(fn)); +}); + +fs.rename(fn, 'foo', function(err) { + assert.ok(0 <= err.message.indexOf(fn)); +}); + +fs.rmdir(fn, function(err) { + assert.ok(0 <= err.message.indexOf(fn)); +}); + +fs.mkdir(existingFile, 0666, function(err) { + assert.ok(0 <= err.message.indexOf(existingFile)); +}); + +fs.rmdir(existingFile, function(err) { + assert.ok(0 <= err.message.indexOf(existingFile)); +}); + +fs.chmod(fn, 0666, function(err) { + assert.ok(0 <= err.message.indexOf(fn)); +}); + +fs.open(fn, 'r', 0666, function(err) { + assert.ok(0 <= err.message.indexOf(fn)); +}); + +fs.readFile(fn, function(err) { + assert.ok(0 <= err.message.indexOf(fn)); +}); + +// Sync + +var errors = [], + expected = 0; + +try { + ++expected; + fs.statSync(fn); +} catch (err) { + errors.push('stat'); + assert.ok(0 <= err.message.indexOf(fn)); +} + +try { + ++expected; + fs.mkdirSync(existingFile, 0666); +} catch (err) { + errors.push('mkdir'); + assert.ok(0 <= err.message.indexOf(existingFile)); +} + +try { + ++expected; + fs.chmodSync(fn, 0666); +} catch (err) { + errors.push('chmod'); + assert.ok(0 <= err.message.indexOf(fn)); +} + +try { + ++expected; + fs.lstatSync(fn); +} catch (err) { + errors.push('lstat'); + assert.ok(0 <= err.message.indexOf(fn)); +} + +try { + ++expected; + fs.readlinkSync(fn); +} catch (err) { + errors.push('readlink'); + assert.ok(0 <= err.message.indexOf(fn)); +} + +try { + ++expected; + fs.linkSync(fn, 'foo'); +} catch (err) { + errors.push('link'); + assert.ok(0 <= err.message.indexOf(fn)); +} + +try { + ++expected; + fs.unlinkSync(fn); +} catch (err) { + errors.push('unlink'); + assert.ok(0 <= err.message.indexOf(fn)); +} + +try { + ++expected; + fs.rmdirSync(fn); +} catch (err) { + errors.push('rmdir'); + assert.ok(0 <= err.message.indexOf(fn)); +} + +try { + ++expected; + fs.rmdirSync(existingFile); +} catch (err) { + errors.push('rmdir'); + assert.ok(0 <= err.message.indexOf(existingFile)); +} + +try { + ++expected; + fs.openSync(fn, 'r'); +} catch (err) { + errors.push('opens'); + assert.ok(0 <= err.message.indexOf(fn)); +} + +try { + ++expected; + fs.renameSync(fn, 'foo'); +} catch (err) { + errors.push('rename'); + assert.ok(0 <= err.message.indexOf(fn)); +} + +try { + ++expected; + fs.readdirSync(fn); +} catch (err) { + errors.push('readdir'); + assert.ok(0 <= err.message.indexOf(fn)); +} + +process.addListener('exit', function () { + assert.equal(expected, errors.length, + 'Test fs sync exceptions raised, got ' + errors.length + ' expected ' + expected); +}); -- 2.7.4