All errors are now instances of the built in Error class

Homme Zwaagstra · Homme Zwaagstra · commit 21b597ef1789 · 2011-09-22T12:36:12.000+01:00
This is a fix for issue brianc#50. It alters both the native binding and the javascript binding to ensure that any errors returned by Postgresql are returned to the client code as instances of the built in Error class. The test code has been updated to assert that this is the case. Rather than run some individual tests the updated test code ensures this is the case for *all* errors encountered during a test run.
diff --git a/lib/connection.js b/lib/connection.js
@@ -379,16 +379,30 @@ p.parseD = function(msg) {
 };
 
 //parses error
-p.parseE = function(msg) {
+p.parseE = function(input) {
   var fields = {};
+  var msg, item;
   var fieldType = this.readString(1);
   while(fieldType != '\0') {
     fields[fieldType] = this.parseCString();
     fieldType = this.readString(1);
   }
+  if (input.name === 'error') {
+    // the msg is an Error instance
+    msg = new Error(fields.M);
+    for (item in input) {
+      // copy input properties to the error
+      if (input.hasOwnProperty(item)) {
+        msg[item] = input[item];
+      }
+    }
+  } else {
+    // the msg is an object literal
+    msg = input;
+    msg.message = fields.M;
+  }
   msg.severity = fields.S;
   msg.code = fields.C;
-  msg.message = fields.M;
   msg.detail = fields.D;
   msg.hint = fields.H;
   msg.position = fields.P;
diff --git a/src/binding.cc b/src/binding.cc
@@ -21,7 +21,6 @@ static Persistent<String> row_symbol;
 static Persistent<String> notice_symbol;
 static Persistent<String> severity_symbol;
 static Persistent<String> code_symbol;
-static Persistent<String> message_symbol;
 static Persistent<String> detail_symbol;
 static Persistent<String> hint_symbol;
 static Persistent<String> position_symbol;
@@ -59,7 +58,6 @@ class Connection : public EventEmitter {
     row_symbol = NODE_PSYMBOL("_row");
     severity_symbol = NODE_PSYMBOL("severity");
     code_symbol = NODE_PSYMBOL("code");
-    message_symbol = NODE_PSYMBOL("message");
     detail_symbol = NODE_PSYMBOL("detail");
     hint_symbol = NODE_PSYMBOL("hint");
     position_symbol = NODE_PSYMBOL("position");
@@ -473,10 +471,12 @@ class Connection : public EventEmitter {
   void HandleErrorResult(const PGresult* result)
   {
     HandleScope scope;
-    Local<Object> msg = Object::New();
+    //instantiate the return object as an Error with the summary Postgres message
+    Local<Object> msg = Local<Object>::Cast(Exception::Error(String::New(PQresultErrorField(result, PG_DIAG_MESSAGE_PRIMARY))));
+
+    //add the other information returned by Postgres to the error object
     AttachErrorField(result, msg, severity_symbol, PG_DIAG_SEVERITY);
     AttachErrorField(result, msg, code_symbol, PG_DIAG_SQLSTATE);
-    AttachErrorField(result, msg, message_symbol, PG_DIAG_MESSAGE_PRIMARY);
     AttachErrorField(result, msg, detail_symbol, PG_DIAG_MESSAGE_DETAIL);
     AttachErrorField(result, msg, hint_symbol, PG_DIAG_MESSAGE_HINT);
     AttachErrorField(result, msg, position_symbol, PG_DIAG_STATEMENT_POSITION);
diff --git a/test/test-helper.js b/test/test-helper.js
@@ -35,6 +35,11 @@ assert.emits = function(item, eventName, callback, message) {
   },2000);
 
   item.once(eventName, function() {
+    if (eventName === 'error') {
+      // belt and braces test to ensure all error events return an error
+      assert.ok(arguments[0] instanceof Error,
+                "Expected error events to throw instances of Error but found: " +  sys.inspect(arguments[0]));
+    }
     called = true;
     clearTimeout(id);
     assert.ok(true);
@@ -105,6 +110,7 @@ assert.throws = function(offender) {
   try {
     offender();
   } catch (e) {
+    assert.ok(e instanceof Error, "Expected " + offender + " to throw instances of Error");
     return;
   }
   assert.ok(false, "Expected " + offender + " to throw exception");
@@ -117,12 +123,14 @@ assert.length = function(actual, expectedLength) {
 var expect = function(callback, timeout) {
   var executed = false;
   var id = setTimeout(function() {
-    assert.ok(executed, "Expected execution of funtion to be fired");
+    assert.ok(executed, "Expected execution of function to be fired");
   }, timeout || 2000)
 
   return function(err, queryResult) {
     clearTimeout(id);
-    assert.ok(true);
+    if (err) {
+      assert.ok(err instanceof Error, "Expected errors to be instances of Error: " + sys.inspect(err));
+    }
     callback.apply(this, arguments)
   }
 }
