Skip to content

Commit

Permalink
Convert ERROR / OK packets into user objects
Browse files Browse the repository at this point in the history
This turns error packets into Error objects, and ok packets into regular
objects. All protocol-level information is stripped since it should not
be relevant for the end user.
  • Loading branch information
felixge committed Aug 22, 2010
1 parent 4b9010a commit cc25df7
Show file tree
Hide file tree
Showing 5 changed files with 126 additions and 19 deletions.
31 changes: 30 additions & 1 deletion lib/mysql/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -198,9 +198,10 @@ Client.prototype._handlePacket = function(packet) {

if (type == Parser.OK_PACKET) {
if (delegate) {
delegate(null, packet);
delegate(null, Client._packetToUserObject(packet));
}
} else if (type == Parser.ERROR_PACKET) {
packet = Client._packetToUserObject(packet);
if (delegate) {
delegate(packet);
} else {
Expand Down Expand Up @@ -231,6 +232,34 @@ Client.prototype._sendAuth = function(greeting) {
this.write(packet);
};

Client._packetToUserObject = function(packet) {

This comment has been minimized.

Copy link
@cjcorrigan

cjcorrigan Oct 16, 2010

Is there any reason why this function, unlike all others is attached directly to Client rather than to Client.prototype?

This comment has been minimized.

Copy link
@felixge

felixge Oct 16, 2010

Author Collaborator

Yeah, it doesn't depend on the state of a client instance, so this makes it "class method" in my book. How would you do it?

var userObject = (packet.type == Parser.ERROR_PACKET)
? new Error()
: {};

for (var key in packet) {
var newKey = key;
if (key == 'type' || key == 'number' || key == 'length' || key == 'received') {
continue;
}

if (key == 'errorMessage') {
newKey = 'message';
} else if (key == 'errorNumber') {
newKey = 'number';
}

userObject[newKey] = packet[key];
}

return userObject;

packet.message = packet.errorMessage;
delete packet.errorMessage;
packet.number = packet.errorNumber;
delete packet.errorNumber;
};

// Client Flags
Client.LONG_PASSWORD = 1;
Client.FOUND_ROWS = 2;
Expand Down
14 changes: 11 additions & 3 deletions lib/mysql/query.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
if (global.GENTLY) require = GENTLY.hijack(require);

var sys = require('sys'),
EventEmitter = require('events').EventEmitter,
Parser = require('./parser');
Parser = require('./parser'),
Client;

function Query() {
EventEmitter.call(this);
Expand All @@ -11,12 +14,17 @@ module.exports = Query;
Query.prototype._handlePacket = function(packet) {
var self = this;

// We can't do this require() on top of the file.
// That's because there is circular dependency and we're overwriting
// module.exports
Client = Client || require('./client');

switch (packet.type) {
case Parser.OK_PACKET:
this.emit('end', packet);
this.emit('end', Client._packetToUserObject(packet));
break;
case Parser.ERROR_PACKET:
this.emit('error', packet);
this.emit('error', Client._packetToUserObject(packet));
break;
case Parser.FIELD_PACKET:
if (!this._fields) {
Expand Down
77 changes: 68 additions & 9 deletions test/simple/test-client.js
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,8 @@ test(function _dequeue() {
});

test(function _handlePacket() {
var USER_OBJECT = {};

(function testGreeting() {
var PACKET = {type: Parser.GREETING_PACKET};

Expand All @@ -360,10 +362,16 @@ test(function _handlePacket() {
})();

(function testNormalOk() {
var PACKET = {type: Parser.OK_PACKET},
TASK = {delegate: gently.expect(function okCb(err, packet) {
assert.strictEqual(packet, PACKET);
})};
var PACKET = {type: Parser.OK_PACKET};

gently.expect(Client, '_packetToUserObject', function (packet) {
assert.strictEqual(packet, PACKET);
return USER_OBJECT;
});

var TASK = {delegate: gently.expect(function okCb(err, packet) {
assert.strictEqual(packet, USER_OBJECT);
})};
gently.expect(client, '_dequeue');

client._queue = [TASK];
Expand All @@ -379,10 +387,16 @@ test(function _handlePacket() {
})();

(function testNormalError() {
var PACKET = {type: Parser.ERROR_PACKET},
TASK = {delegate: gently.expect(function errCb(packet) {
assert.strictEqual(packet, PACKET);
})};
var PACKET = {type: Parser.ERROR_PACKET};

gently.expect(Client, '_packetToUserObject', function (packet) {
assert.strictEqual(packet, PACKET);
return USER_OBJECT;
});

var TASK = {delegate: gently.expect(function errCb(packet) {
assert.strictEqual(packet, USER_OBJECT);
})};
gently.expect(client, '_dequeue');

client._queue = [TASK];
Expand All @@ -393,9 +407,14 @@ test(function _handlePacket() {
var PACKET = {type: Parser.ERROR_PACKET};
client._queue = [{}];

gently.expect(Client, '_packetToUserObject', function (packet) {
assert.strictEqual(packet, PACKET);
return USER_OBJECT;
});

gently.expect(client, 'emit', function(event, err) {
assert.equal(event, 'error');
assert.strictEqual(err, PACKET);
assert.strictEqual(err, USER_OBJECT);
});
gently.expect(client, '_dequeue');
client._handlePacket(PACKET);
Expand Down Expand Up @@ -480,3 +499,43 @@ test(function _sendPacket() {

client._sendAuth(GREETING);
});

test(function _packetToUserObject() {
gently.restore(Client, '_packetToUserObject');

(function testOkPacket() {
var PACKET = {
type: Parser.OK_PACKET,
length: 65,
received: 65,
number: 92,
foo: 'bar',
};

var ok = Client._packetToUserObject(PACKET);

assert.notStrictEqual(PACKET, ok);
assert.ok(!(ok instanceof Error));
assert.equal(ok.foo, PACKET.foo);
assert.equal(ok.type, undefined);
assert.equal(ok.length, undefined);
assert.equal(ok.received, undefined);
})();

(function testErrorPacket() {
var PACKET = {
type: Parser.ERROR_PACKET,
foo: 'bar',
errorMessage: 'oh no',
errorNumber: 1007
};

var err = Client._packetToUserObject(PACKET);

assert.ok(err instanceof Error);
assert.equal(err.message, 'oh no');
assert.equal(err.errorMessage, undefined);
assert.equal(err.number, 1007);
assert.equal(err.errorNumber, undefined);
})();
});
21 changes: 16 additions & 5 deletions test/simple/test-query.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
require('../common');
var Query = require('mysql/query'),
var ClientStub = GENTLY.stub('./client'),
Query = require('mysql/query'),
EventEmitter = require('events').EventEmitter,
Parser = require('mysql/parser'),
query,
Expand All @@ -18,22 +19,32 @@ test(function constructor() {

test(function _handlePacket() {
(function testOkPacket() {
var PACKET = {type: Parser.OK_PACKET};
var PACKET = {type: Parser.OK_PACKET}, USER_OBJECT = {};

gently.expect(ClientStub, '_packetToUserObject', function (packet) {
assert.strictEqual(packet, PACKET);
return USER_OBJECT;
});

gently.expect(query, 'emit', function (event, packet) {
assert.equal(event, 'end');
assert.strictEqual(packet, PACKET);
assert.strictEqual(packet, USER_OBJECT);
});

query._handlePacket(PACKET);
})();

(function testErrorPacket() {
var PACKET = {type: Parser.ERROR_PACKET};
var PACKET = {type: Parser.ERROR_PACKET}, USER_OBJECT = {};

gently.expect(ClientStub, '_packetToUserObject', function (packet) {
assert.strictEqual(packet, PACKET);
return USER_OBJECT;
});

gently.expect(query, 'emit', function (event, packet) {
assert.equal(event, 'error');
assert.strictEqual(packet, PACKET);
assert.strictEqual(packet, USER_OBJECT);
});

query._handlePacket(PACKET);
Expand Down
2 changes: 1 addition & 1 deletion test/system/test-client-query.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ client.connect();
client.query(
'CREATE DATABASE '+TEST_CONFIG.database,
gently.expect(function createDbCb(err) {
if (err && err.errorNumber != Client.ERROR_DB_CREATE_EXISTS) {
if (err && err.number != Client.ERROR_DB_CREATE_EXISTS) {
throw err;
}
})
Expand Down

0 comments on commit cc25df7

Please sign in to comment.