Merge branch 'master' of https://github.com/brianc/node-postgres

README.md

@@ -112,6 +112,7 @@ Many thanks to the following:
 * [drdaeman](https://github.com/drdaeman)
 * [booo](https://github.com/booo)
 * [neonstalwart](https://github.com/neonstalwart)
+* [homme](https://github.com/homme)
 
 ## Documentation
 

lib/client.js

@@ -28,7 +28,7 @@ sys.inherits(Client, EventEmitter);
 
 var p = Client.prototype;
 
-p.connect = function() {
+p.connect = function(callback) {
   var self = this;
   var con = this.connection;
   if(this.host && this.host.indexOf('/') === 0) {
@@ -84,11 +84,17 @@ p.connect = function() {
       }
     });
 
-    self.emit('connect');
+    if (!callback) {
+      self.emit('connect');
+    } else {
+      callback(null,self);
+      //remove callback for proper error handling after the connect event
+      callback = null;
+    }
 
     con.on('notification', function(msg) {
       self.emit('notification', msg);
-    })
+    });
 
   });
 
@@ -103,7 +109,11 @@ p.connect = function() {
 
   con.on('error', function(error) {
     if(!self.activeQuery) {
-      self.emit('error', error);
+      if(!callback) {
+        self.emit('error', error);
+      } else {
+        callback(error);
+      }
     } else {
       //need to sync after error during a prepared statement
       if(self.activeQuery.isPreparedStatement) {
@@ -116,7 +126,7 @@ p.connect = function() {
 
   con.on('notice', function(msg) {
     self.emit('notice', msg);
-  })
+  });
 
 };
 

lib/connection.js

@@ -374,16 +374,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;

lib/native/index.js

@@ -12,11 +12,28 @@ var p = Connection.prototype;
 
 var nativeConnect = p.connect;
 
-p.connect = function() {
+p.connect = function(cb) {
   var self = this;
   utils.buildLibpqConnectionString(this._config, function(err, conString) {
-    if(err) return self.emit('error', err);
+    if(err) {
+      return cb ? cb(err) : self.emit('error', err);
+    }
     nativeConnect.call(self, conString);
+    if(cb) {
+      var errCallback;
+      var connectCallback = function() {
+        //remove single-fire connection error callback
+        self.removeListener('error', errCallback);
+        cb(null);
+      }
+      errCallback = function(err) {
+        //remove singel-fire connection success callback
+        self.removeListener('connect', connectCallback);
+        cb(err);
+      }
+      self.once('connect', connectCallback);
+      self.once('error', errCallback);
+    }
   })
 }
 

package.json

@@ -1,5 +1,5 @@
 { "name": "pg",
-  "version": "0.5.6",
+  "version": "0.5.7",
   "description": "PostgreSQL client - pure javascript & libpq with the same API",
   "keywords" : ["postgres", "pg", "libpq", "postgre", "database", "rdbms"],
   "homepage": "http://github.com/brianc/node-postgres",

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 @@ public:
     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 @@ protected:
   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);

test/integration/client/error-handling-tests.js

@@ -1,11 +1,11 @@
 var helper = require(__dirname + '/test-helper');
-var sys = require('sys')
+var sys = require('sys');
 
 var createErorrClient = function() {
   var client = helper.client();
   client.on('error', function(err) {
     assert.ok(false, "client should not throw query error: " + sys.inspect(err));
-  })
+  });
   client.on('drain', client.end.bind(client));
   return client;
 };
@@ -21,7 +21,7 @@ test('error handling', function(){
     assert.emits(query, 'error', function(error) {
       test('error is a psql error', function() {
         assert.equal(error.severity, "ERROR");
-      })
+      });
     });
 
   });
@@ -54,9 +54,9 @@ test('error handling', function(){
         assert.emits(query, 'error', function(err) {
           ensureFuture(client);
         });
-      })
+      });
 
-      test("when a query is binding", function() { 
+      test("when a query is binding", function() {
 
         var query = client.query({
           text: 'select * from boom where age = $1',
@@ -76,7 +76,7 @@ test('error handling', function(){
 
         //TODO how to test for errors during execution?
       });
-    })
+    });
   });
 
   test('non-query error', function() {
@@ -88,18 +88,52 @@ test('error handling', function(){
     client.connect();
   });
 
+  test('non-query error with callback', function() {
+    var client = new Client({
+      user:'asldkfjsadlfkj'
+    });
+    client.connect(assert.calls(function(error, client) {
+      assert.ok(error);
+    }));
+  });
+
+});
+
+test('non-error calls supplied callback', function() {
+  var client = new Client({
+    user: helper.args.user,
+    password: helper.args.password,
+    host: helper.args.host,
+    port: helper.args.port,
+    database: helper.args.database
+  });
+
+  client.connect(assert.calls(function(err) {
+    assert.isNull(err);
+    client.end();
+  }))
 });
 
 test('when connecting to invalid host', function() {
-  return false;
   var client = new Client({
     user: 'brian',
     password: '1234',
     host: 'asldkfjasdf!!#1308140.com'
-  })
+  });
   assert.emits(client, 'error');
   client.connect();
-})
+});
+
+test('when connecting to invalid host with callback', function() {
+  var client = new Client({
+    user: 'brian',
+    password: '1234',
+    host: 'asldkfjasdf!!#1308140.com'
+  });
+  client.connect(function(error, client) {
+    assert.ok(error);
+  });
+});
 
 test('multiple connection errors (gh#31)', function() {
   return false;
@@ -111,18 +145,18 @@ test('multiple connection errors (gh#31)', function() {
       host: helper.args.host,
       port: helper.args.port,
       database: helper.args.database
-    })
+    });
     client.connect();
     assert.emits(client, 'error', function(e) {
       client.connect();
-      assert.emits(client, 'error')
-    })
-  })
+      assert.emits(client, 'error');
+    });
+  });
 
   test('with callback method', function() {
     var badConString = "tcp://aslkdfj:oi14081@"+helper.args.host+":"+helper.args.port+"/"+helper.args.database;
     return false;
-  })
+  });
 
-})
+});
 

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)
   }
 }

test/unit/client/test-helper.js

@@ -9,7 +9,7 @@ var makeClient = function() {
   };
   connection.queries = [];
   var client = new Client({connection: connection});
-  client.connect(helper.args.port, helper.args.host);
+  client.connect();
   client.connection.emit('connect');
   return client;
 };

test/unit/connection/error-tests.js

@@ -3,8 +3,8 @@ var Connection = require(__dirname + '/../../../lib/connection');
 var con = new Connection({stream: new MemoryStream()});
 test("connection emits stream errors", function() {
   assert.emits(con, 'error', function(err) {
-    assert.equal(err, "OMG!");
+    assert.equal(err.message, "OMG!");
   });
   con.connect();
-  con.stream.emit('error', "OMG!");
+  con.stream.emit('error', new Error("OMG!"));
 });