Added better error messages for async and sync fs calls with paths

diff --git a/src/node.cc b/src/node.cc
index c9452a7..d220579 100644 (file)
--- a/src/node.cc
+++ b/src/node.cc
@@ -51,6 +51,7 @@ static Persistent<Object> process;
 
 static Persistent<String> errno_symbol;
 static Persistent<String> syscall_symbol;
+static Persistent<String> errpath_symbol;
 
 static Persistent<String> dev_symbol;
 static Persistent<String> ino_symbol;
@@ -737,7 +738,9 @@ const char *signo_string(int signo) {
 
 Local<Value> ErrnoException(int errorno,
                             const char *syscall,
-                            const char *msg) {
+                            const char *msg,
+                            const char *path) {
+  Local<Value> e;
   Local<String> estring = String::NewSymbol(errno_string(errorno));
   if (!msg[0]) msg = strerror(errorno);
   Local<String> message = String::NewSymbol(msg);
@@ -745,16 +748,25 @@ Local<Value> ErrnoException(int errorno,
   Local<String> cons1 = String::Concat(estring, String::NewSymbol(", "));
   Local<String> cons2 = String::Concat(cons1, message);
 
-  Local<Value> e = Exception::Error(cons2);
-
-  Local<Object> 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<String> cons3 = String::Concat(cons2, String::NewSymbol(" '"));
+    Local<String> cons4 = String::Concat(cons3, String::New(path));
+    Local<String> cons5 = String::Concat(cons4, String::NewSymbol("'"));
+    e = Exception::Error(cons5);
+  } else {
+    e = Exception::Error(cons2);
   }
 
+  Local<Object> 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 (file)
--- a/src/node.h
+++ b/src/node.h
@@ -78,7 +78,8 @@ static inline void cb_destroy(v8::Persistent<v8::Function> * cb) {
 
 v8::Local<v8::Value> 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 (file)
--- 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<const char*>(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<Value>::New(Null());
-
     switch (req->type) {
       case EIO_CLOSE:
       case EIO_RENAME:
@@ -194,7 +208,7 @@ static Handle<Value> 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<Value> 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<Value> 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<Value> 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<Value> 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<Value> 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<Value> 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<Value> 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<Value> 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<Value> 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<Value> 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 (file)
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);
+});