diff --git a/README.md b/README.md index 90c015cf..3c2fa6cc 100644 --- a/README.md +++ b/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 diff --git a/lib/client.js b/lib/client.js index 852b3087..6db60566 100644 --- a/lib/client.js +++ b/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); - }) + }); }; diff --git a/lib/connection.js b/lib/connection.js index 66dfa16d..fce859bf 100644 --- a/lib/connection.js +++ b/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; diff --git a/lib/native/index.js b/lib/native/index.js index 37b2befd..92c89789 100644 --- a/lib/native/index.js +++ b/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); + } }) } diff --git a/package.json b/package.json index ea8b0a0a..3f6b7e52 100644 --- a/package.json +++ b/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", diff --git a/src/binding.cc b/src/binding.cc index 0123021c..3ec55e7a 100644 --- a/src/binding.cc +++ b/src/binding.cc @@ -21,7 +21,6 @@ static Persistent row_symbol; static Persistent notice_symbol; static Persistent severity_symbol; static Persistent code_symbol; -static Persistent message_symbol; static Persistent detail_symbol; static Persistent hint_symbol; static Persistent 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 msg = Object::New(); + //instantiate the return object as an Error with the summary Postgres message + Local msg = Local::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/integration/client/error-handling-tests.js b/test/integration/client/error-handling-tests.js index d34cb0fd..39a9d632 100644 --- a/test/integration/client/error-handling-tests.js +++ b/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; - }) + }); -}) +}); diff --git a/test/test-helper.js b/test/test-helper.js index 45a87ee0..9adb8651 100644 --- 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) } } diff --git a/test/unit/client/test-helper.js b/test/unit/client/test-helper.js index 0520992f..a8294023 100644 --- a/test/unit/client/test-helper.js +++ b/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; }; diff --git a/test/unit/connection/error-tests.js b/test/unit/connection/error-tests.js index 5bf9dc6e..bccffac4 100644 --- a/test/unit/connection/error-tests.js +++ b/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!")); });