Skip to content

Commit 25b1fc2

Browse files
AVaksmanAVaksman
authored
fix: handle large int out of bounds error in streams (#590)
1 parent cd7887f commit 25b1fc2

File tree

4 files changed

+121
-52
lines changed

4 files changed

+121
-52
lines changed

‎src/entity.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -452,7 +452,7 @@ export namespace entity {
452452
"'options.wrapNumbers' as\n" +
453453
'{\n' +
454454
' integerTypeCastFunction: provide <your_custom_function>\n' +
455-
' properties: optionally specify property name(s) to be cutom casted' +
455+
' properties: optionally specify property name(s) to be custom casted\n' +
456456
'}\n'
457457
);
458458
}

‎src/request.ts‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -290,10 +290,17 @@ class DatastoreRequest {
290290
return;
291291
}
292292

293-
const entities = entity.formatArray(
294-
resp!.found! as ResponseResult[],
295-
options.wrapNumbers
296-
);
293+
let entities: Entity[] = [];
294+
295+
try {
296+
entities = entity.formatArray(
297+
resp!.found! as ResponseResult[],
298+
options.wrapNumbers
299+
);
300+
} catch (err) {
301+
stream.destroy(err);
302+
return;
303+
}
297304
const nextKeys = (resp!.deferred || [])
298305
.map(entity.keyFromKeyProto)
299306
.map(entity.keyToKeyProto);
@@ -778,10 +785,15 @@ class DatastoreRequest {
778785
let entities: Entity[] = [];
779786

780787
if (resp.batch.entityResults) {
781-
entities = entity.formatArray(
782-
resp.batch.entityResults,
783-
options.wrapNumbers
784-
);
788+
try {
789+
entities = entity.formatArray(
790+
resp.batch.entityResults,
791+
options.wrapNumbers
792+
);
793+
} catch (err) {
794+
stream.destroy(err);
795+
return;
796+
}
785797
}
786798

787799
// Emit each result right away, then get the rest if necessary.

‎test/entity.ts‎

Lines changed: 25 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,25 @@ import * as sinon from 'sinon';
1919
import {Datastore} from '../src';
2020
import {Entity, entity, ValueProto} from '../src/entity';
2121
import {IntegerTypeCastOptions} from '../src/query';
22-
import {AnyARecord} from 'dns';
22+
23+
export function outOfBoundsError(opts: {
24+
propertyName?: string;
25+
integerValue: string | number;
26+
}) {
27+
return new Error(
28+
'We attempted to return all of the numeric values, but ' +
29+
(opts.propertyName ? opts.propertyName + ' ' : '') +
30+
'value ' +
31+
opts.integerValue +
32+
" is out of bounds of 'Number.MAX_SAFE_INTEGER'.\n" +
33+
"To prevent this error, please consider passing 'options.wrapNumbers=true' or\n" +
34+
"'options.wrapNumbers' as\n" +
35+
'{\n' +
36+
' integerTypeCastFunction: provide <your_custom_function>\n' +
37+
' properties: optionally specify property name(s) to be custom casted\n' +
38+
'}\n'
39+
);
40+
}
2341

2442
describe('entity', () => {
2543
let entity: Entity;
@@ -101,24 +119,6 @@ describe('entity', () => {
101119
});
102120

103121
describe('integerTypeCastFunction is not provided', () => {
104-
const expectedError = (opts: {
105-
integerValue?: number;
106-
propertyName?: string;
107-
}) => {
108-
return new Error(
109-
'We attempted to return all of the numeric values, but ' +
110-
(opts.propertyName ? opts.propertyName + ' ' : '') +
111-
'value ' +
112-
opts.integerValue +
113-
" is out of bounds of 'Number.MAX_SAFE_INTEGER'.\n" +
114-
"To prevent this error, please consider passing 'options.wrapNumbers=true' or\n" +
115-
"'options.wrapNumbers' as\n" +
116-
'{\n' +
117-
' integerTypeCastFunction: provide <your_custom_function>\n' +
118-
' properties: optionally specify property name(s) to be cutom casted' +
119-
'}\n'
120-
);
121-
};
122122
it('should throw if integerTypeCastOptions is provided but integerTypeCastFunction is not', () => {
123123
assert.throws(
124124
() => new entity.Int(valueProto, {}).valueOf(),
@@ -142,11 +142,11 @@ describe('entity', () => {
142142

143143
assert.throws(() => {
144144
new entity.Int(valueProto).valueOf();
145-
}, expectedError(valueProto));
145+
}, outOfBoundsError(valueProto));
146146

147147
assert.throws(() => {
148148
new entity.Int(valueProto2).valueOf();
149-
}, expectedError(valueProto2));
149+
}, outOfBoundsError(valueProto2));
150150
});
151151

152152
it('should throw if integer value is outside of bounds passing strings or Numbers', () => {
@@ -156,12 +156,12 @@ describe('entity', () => {
156156
// should throw when Number is passed
157157
assert.throws(() => {
158158
new entity.Int(largeIntegerValue).valueOf();
159-
}, expectedError({integerValue: largeIntegerValue}));
159+
}, outOfBoundsError({integerValue: largeIntegerValue}));
160160

161161
// should throw when string is passed
162162
assert.throws(() => {
163163
new entity.Int(smallIntegerValue.toString()).valueOf();
164-
}, expectedError({integerValue: smallIntegerValue}));
164+
}, outOfBoundsError({integerValue: smallIntegerValue}));
165165
});
166166

167167
it('should not auto throw on initialization', () => {
@@ -559,24 +559,6 @@ describe('entity', () => {
559559
});
560560

561561
it('should throw if integer value is outside of bounds', () => {
562-
const expectedError = (opts: {
563-
integerValue: number;
564-
propertyName: string;
565-
}) => {
566-
return new Error(
567-
'We attempted to return all of the numeric values, but ' +
568-
(opts.propertyName ? opts.propertyName + ' ' : '') +
569-
'value ' +
570-
opts.integerValue +
571-
" is out of bounds of 'Number.MAX_SAFE_INTEGER'.\n" +
572-
"To prevent this error, please consider passing 'options.wrapNumbers=true' or\n" +
573-
"'options.wrapNumbers' as\n" +
574-
'{\n' +
575-
' integerTypeCastFunction: provide <your_custom_function>\n' +
576-
' properties: optionally specify property name(s) to be cutom casted' +
577-
'}\n'
578-
);
579-
};
580562
const largeIntegerValue = Number.MAX_SAFE_INTEGER + 1;
581563
const smallIntegerValue = Number.MIN_SAFE_INTEGER - 1;
582564

@@ -594,11 +576,11 @@ describe('entity', () => {
594576

595577
assert.throws(() => {
596578
entity.decodeValueProto(valueProto);
597-
}, expectedError(valueProto));
579+
}, outOfBoundsError(valueProto));
598580

599581
assert.throws(() => {
600582
entity.decodeValueProto(valueProto2);
601-
}, expectedError(valueProto2));
583+
}, outOfBoundsError(valueProto2));
602584
});
603585
});
604586

‎test/request.ts‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {google} from '../proto/datastore';
2626
import * as ds from '../src';
2727
import {entity, Entity, KeyProto, EntityProto} from '../src/entity.js';
2828
import {IntegerTypeCastOptions, Query, QueryProto} from '../src/query.js';
29+
import {outOfBoundsError} from './entity';
2930
import {
3031
AllocateIdsResponse,
3132
RequestConfig,
@@ -373,6 +374,41 @@ describe('Request', () => {
373374
});
374375
});
375376
});
377+
378+
it('should emit an error from results decoding', done => {
379+
const largeInt = '922337203685477850';
380+
request.request_ = (config: RequestConfig, callback: Function) => {
381+
callback(null, {
382+
found: [
383+
{
384+
entity: {
385+
properties: {
386+
points: {
387+
integerValue: largeInt,
388+
valueType: 'integerValue',
389+
},
390+
},
391+
},
392+
},
393+
],
394+
});
395+
};
396+
397+
const stream = request.createReadStream(key);
398+
399+
stream
400+
.on('data', () => {})
401+
.on('error', (err: Error) => {
402+
assert.deepStrictEqual(
403+
err,
404+
outOfBoundsError({integerValue: largeInt})
405+
);
406+
setImmediate(() => {
407+
assert.strictEqual(stream.destroyed, true);
408+
done();
409+
});
410+
});
411+
});
376412
});
377413

378414
describe('success', () => {
@@ -951,6 +987,45 @@ describe('Request', () => {
951987
})
952988
.emit('reading');
953989
});
990+
991+
it('should emit an error from results decoding', done => {
992+
const largeInt = '922337203685477850';
993+
sandbox.stub(entity, 'queryToQueryProto');
994+
995+
request.request_ = (config: RequestConfig, callback: Function) => {
996+
callback(null, {
997+
batch: {
998+
entityResults: [
999+
{
1000+
entity: {
1001+
properties: {
1002+
points: {
1003+
integerValue: largeInt,
1004+
valueType: 'integerValue',
1005+
},
1006+
},
1007+
},
1008+
},
1009+
],
1010+
},
1011+
});
1012+
};
1013+
1014+
const stream = request.runQueryStream({});
1015+
1016+
stream
1017+
.on('error', (err: Error) => {
1018+
assert.deepStrictEqual(
1019+
err,
1020+
outOfBoundsError({integerValue: largeInt})
1021+
);
1022+
setImmediate(() => {
1023+
assert.strictEqual(stream.destroyed, true);
1024+
done();
1025+
});
1026+
})
1027+
.emit('reading');
1028+
});
9541029
});
9551030

9561031
describe('success', () => {

0 commit comments

Comments
 (0)