Skip to content

Commit 34dfc80

Browse files
jakewinsjakewins
authored
Merge pull request #102 from pontusmelke/1.0-unpackable-types
Better error handling for unpackable types
2 parents 3c3a35f + f918154 commit 34dfc80

File tree

4 files changed

+90
-48
lines changed

4 files changed

+90
-48
lines changed

‎src/v1/internal/connector.js

+13-6
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,7 @@ let _mappers = {
156156
* @access private
157157
*/
158158
class Connection {
159+
159160
/**
160161
* @constructor
161162
* @param channel - channel with a 'write' function and a 'onmessage'
@@ -319,28 +320,30 @@ class Connection {
319320
/** Queue an INIT-message to be sent to the database */
320321
initialize( clientName, token, observer ) {
321322
this._queueObserver(observer);
322-
this._packer.packStruct( INIT, [clientName, token] );
323+
this._packer.packStruct( INIT, [this._packable(clientName), this._packable(token)],
324+
(err) => this._handleFatalError(err) );
323325
this._chunker.messageBoundary();
324326
}
325327

326328
/** Queue a RUN-message to be sent to the database */
327329
run( statement, params, observer ) {
328330
this._queueObserver(observer);
329-
this._packer.packStruct( RUN, [statement, params] );
331+
this._packer.packStruct( RUN, [this._packable(statement), this._packable(params)],
332+
(err) => this._handleFatalError(err) );
330333
this._chunker.messageBoundary();
331334
}
332335

333336
/** Queue a PULL_ALL-message to be sent to the database */
334337
pullAll( observer ) {
335338
this._queueObserver(observer);
336-
this._packer.packStruct( PULL_ALL );
339+
this._packer.packStruct( PULL_ALL, [], (err) => this._handleFatalError(err) );
337340
this._chunker.messageBoundary();
338341
}
339342

340343
/** Queue a DISCARD_ALL-message to be sent to the database */
341344
discardAll( observer ) {
342345
this._queueObserver(observer);
343-
this._packer.packStruct( DISCARD_ALL );
346+
this._packer.packStruct( DISCARD_ALL, [], (err) => this._handleFatalError(err) );
344347
this._chunker.messageBoundary();
345348
}
346349

@@ -359,14 +362,14 @@ class Connection {
359362
}
360363
};
361364
this._queueObserver(wrappedObs);
362-
this._packer.packStruct( RESET );
365+
this._packer.packStruct( RESET, [], (err) => this._handleFatalError(err) );
363366
this._chunker.messageBoundary();
364367
}
365368

366369
/** Queue a ACK_FAILURE-message to be sent to the database */
367370
_ackFailure( observer ) {
368371
this._queueObserver(observer);
369-
this._packer.packStruct( ACK_FAILURE );
372+
this._packer.packStruct( ACK_FAILURE, [], (err) => this._handleFatalError(err) );
370373
this._chunker.messageBoundary();
371374
}
372375

@@ -408,6 +411,10 @@ class Connection {
408411
close(cb) {
409412
this._ch.close(cb);
410413
}
414+
415+
_packable(value) {
416+
return this._packer.packable(value, (err) => this._handleFatalError(err));
417+
}
411418
}
412419

413420
/**

‎src/v1/internal/packstream.js

+60-41
Original file line numberDiff line numberDiff line change
@@ -80,57 +80,77 @@ class Packer {
8080
this._ch = channel;
8181
}
8282

83-
pack (x) {
83+
/**
84+
* Creates a packable function out of the provided value
85+
* @param x the value to pack
86+
* @param onError callback for the case when value cannot be packed
87+
* @returns Function
88+
*/
89+
packable (x, onError) {
8490
if (x === null) {
85-
this._ch.writeUInt8( NULL );
91+
return () => this._ch.writeUInt8( NULL );
8692
} else if (x === true) {
87-
this._ch.writeUInt8( TRUE );
93+
return () => this._ch.writeUInt8( TRUE );
8894
} else if (x === false) {
89-
this._ch.writeUInt8( FALSE );
95+
return () => this._ch.writeUInt8( FALSE );
9096
} else if (typeof(x) == "number") {
91-
this.packFloat(x);
97+
return () => this.packFloat(x);
9298
} else if (typeof(x) == "string") {
93-
this.packString(x);
99+
return () => this.packString(x, onError);
94100
} else if (x instanceof Integer) {
95-
this.packInteger( x );
101+
return () => this.packInteger( x );
96102
} else if (x instanceof Array) {
97-
this.packListHeader(x.length);
98-
for(let i = 0; i < x.length; i++) {
99-
this.pack(x[i] === undefined ? null : x[i]);
103+
return () => {
104+
this.packListHeader(x.length, onError);
105+
for (let i = 0; i < x.length; i++) {
106+
this.packable(x[i] === undefined ? null : x[i], onError)();
107+
}
100108
}
101109
} else if (x instanceof Structure) {
102-
this.packStruct( x.signature, x.fields );
110+
var packableFields = [];
111+
for (var i = 0; i < x.fields.length; i++) {
112+
packableFields[i] = this.packable(x.fields[i], onError);
113+
}
114+
return () => this.packStruct( x.signature, packableFields );
103115
} else if (typeof(x) == "object") {
104-
let keys = Object.keys(x);
116+
return () => {
117+
let keys = Object.keys(x);
105118

106-
let count = 0;
107-
for(let i = 0; i < keys.length; i++) {
108-
if (x[keys[i]] !== undefined) {
109-
count++;
119+
let count = 0;
120+
for (let i = 0; i < keys.length; i++) {
121+
if (x[keys[i]] !== undefined) {
122+
count++;
123+
}
110124
}
111-
}
112-
113-
this.packMapHeader(count);
114-
for(let i = 0; i < keys.length; i++) {
115-
let key = keys[i];
116-
if (x[key] !== undefined) {
117-
this.packString(key);
118-
this.pack(x[key]);
125+
this.packMapHeader(count, onError);
126+
for (let i = 0; i < keys.length; i++) {
127+
let key = keys[i];
128+
if (x[key] !== undefined) {
129+
this.packString(key);
130+
this.packable(x[key], onError)();
131+
}
119132
}
120-
}
133+
};
121134
} else {
122-
throw newError("Cannot pack this value: " + x);
135+
if (onError) {
136+
onError(newError("Cannot pack this value: " + x));
137+
}
138+
return () => undefined;
123139
}
124140
}
125141

126-
packStruct ( signature, fields ) {
127-
fields = fields || [];
128-
this.packStructHeader(fields.length, signature);
129-
for(let i = 0; i < fields.length; i++) {
130-
this.pack(fields[i]);
142+
/**
143+
* Packs a struct
144+
* @param signature the signature of the struct
145+
* @param packableFields the fields of the struct, make sure you call `packable on all fields`
146+
*/
147+
packStruct ( signature, packableFields, onError) {
148+
packableFields = packableFields || [];
149+
this.packStructHeader(packableFields.length, signature, onError);
150+
for(let i = 0; i < packableFields.length; i++) {
151+
packableFields[i]();
131152
}
132153
}
133-
134154
packInteger (x) {
135155
var high = x.high,
136156
low = x.low;
@@ -156,13 +176,12 @@ class Packer {
156176
this._ch.writeInt32(low);
157177
}
158178
}
159-
160179
packFloat(x) {
161180
this._ch.writeUInt8(FLOAT_64);
162181
this._ch.writeFloat64(x);
163182
}
164183

165-
packString (x) {
184+
packString (x, onError) {
166185
let bytes = utf8.encode(x);
167186
let size = bytes.length;
168187
if (size < 0x10) {
@@ -185,11 +204,11 @@ class Packer {
185204
this._ch.writeUInt8(size%256);
186205
this._ch.writeBytes(bytes);
187206
} else {
188-
throw newError("UTF-8 strings of size " + size + " are not supported");
207+
onError(newError("UTF-8 strings of size " + size + " are not supported"));
189208
}
190209
}
191210

192-
packListHeader (size) {
211+
packListHeader (size, onError) {
193212
if (size < 0x10) {
194213
this._ch.writeUInt8(TINY_LIST | size);
195214
} else if (size < 0x100) {
@@ -206,11 +225,11 @@ class Packer {
206225
this._ch.writeUInt8((size/256>>0)%256);
207226
this._ch.writeUInt8(size%256);
208227
} else {
209-
throw newError("Lists of size " + size + " are not supported");
228+
onError(newError("Lists of size " + size + " are not supported"));
210229
}
211230
}
212231

213-
packMapHeader (size) {
232+
packMapHeader (size, onError) {
214233
if (size < 0x10) {
215234
this._ch.writeUInt8(TINY_MAP | size);
216235
} else if (size < 0x100) {
@@ -227,11 +246,11 @@ class Packer {
227246
this._ch.writeUInt8((size/256>>0)%256);
228247
this._ch.writeUInt8(size%256);
229248
} else {
230-
throw newError("Maps of size " + size + " are not supported");
249+
onError(newError("Maps of size " + size + " are not supported"));
231250
}
232251
}
233252

234-
packStructHeader (size, signature) {
253+
packStructHeader (size, signature, onError) {
235254
if (size < 0x10) {
236255
this._ch.writeUInt8(TINY_STRUCT | size);
237256
this._ch.writeUInt8(signature);
@@ -244,7 +263,7 @@ class Packer {
244263
this._ch.writeUInt8(size/256>>0);
245264
this._ch.writeUInt8(size%256);
246265
} else {
247-
throw newError("Structures of size " + size + " are not supported");
266+
onError(newError("Structures of size " + size + " are not supported"));
248267
}
249268
}
250269
}

‎test/internal/packstream.test.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ var alloc = require('../../lib/v1/internal/buf').alloc,
2626
Integer = integer.Integer;
2727

2828
describe('packstream', function() {
29+
30+
2931
it('should pack integers', function() {
3032
var n, i;
3133
// test small numbers
@@ -76,7 +78,7 @@ describe('packstream', function() {
7678
function packAndUnpack( val, bufferSize ) {
7779
bufferSize = bufferSize || 128;
7880
var buffer = alloc(bufferSize);
79-
new Packer( buffer ).pack( val );
81+
new Packer( buffer ).packable( val )();
8082
buffer.reset();
8183
return new Unpacker().unpack( buffer );
8284
}

‎test/v1/session.test.js

+14
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,20 @@ describe('session', function () {
292292
}, 1500);
293293
});
294294

295+
it('should fail nicely on unpackable values ', function (done) {
296+
// Given
297+
var unpackable = function(){throw Error()};
298+
299+
var statement = "RETURN {param}";
300+
var params = {param: unpackable};
301+
// When & Then
302+
session
303+
.run(statement, params)
304+
.catch(function (ignore) {
305+
done();
306+
})
307+
});
308+
295309
});
296310

297311

0 commit comments

Comments
 (0)