Skip to content

Commit 7b351cc

Browse files
addaleaxnbbeeken
andauthored
feat: make circular input errors for EJSON expressive (#433)
* feat: make circular input errors for EJSON expressive Give errors roughly in the style of those thrown by `JSON.stringify()` on modern Node.js/V8 versions, where the path to the property and its circularity are visualized instead of just recursive indefinitely. This is just one suggested solution – it would be nice to have *some* kind of better error in these cases, and I think actually displaying the path would be nice in terms of UX, but I can also see an argument for avoiding the extra bits of complexity here. NODE-3226 Co-authored-by: Neal Beeken <[email protected]>
1 parent 3a2eff1 commit 7b351cc

File tree

2 files changed

+99
-7
lines changed

2 files changed

+99
-7
lines changed

src/extended_json.ts

+55-7
Original file line numberDiff line numberDiff line change
@@ -140,9 +140,20 @@ function deserializeValue(value: any, options: EJSON.Options = {}) {
140140
return value;
141141
}
142142

143+
type EJSONSerializeOptions = EJSON.Options & {
144+
seenObjects: { obj: unknown; propertyName: string }[];
145+
};
146+
143147
// eslint-disable-next-line @typescript-eslint/no-explicit-any
144-
function serializeArray(array: any[], options: EJSON.Options): any[] {
145-
return array.map((v: unknown) => serializeValue(v, options));
148+
function serializeArray(array: any[], options: EJSONSerializeOptions): any[] {
149+
return array.map((v: unknown, index: number) => {
150+
options.seenObjects.push({ propertyName: `index ${index}`, obj: null });
151+
try {
152+
return serializeValue(v, options);
153+
} finally {
154+
options.seenObjects.pop();
155+
}
156+
});
146157
}
147158

148159
function getISOString(date: Date) {
@@ -152,7 +163,37 @@ function getISOString(date: Date) {
152163
}
153164

154165
// eslint-disable-next-line @typescript-eslint/no-explicit-any
155-
function serializeValue(value: any, options: EJSON.Options): any {
166+
function serializeValue(value: any, options: EJSONSerializeOptions): any {
167+
if ((typeof value === 'object' || typeof value === 'function') && value !== null) {
168+
const index = options.seenObjects.findIndex(entry => entry.obj === value);
169+
if (index !== -1) {
170+
const props = options.seenObjects.map(entry => entry.propertyName);
171+
const leadingPart = props
172+
.slice(0, index)
173+
.map(prop => `${prop} -> `)
174+
.join('');
175+
const alreadySeen = props[index];
176+
const circularPart =
177+
' -> ' +
178+
props
179+
.slice(index + 1, props.length - 1)
180+
.map(prop => `${prop} -> `)
181+
.join('');
182+
const current = props[props.length - 1];
183+
const leadingSpace = ' '.repeat(leadingPart.length + alreadySeen.length / 2);
184+
const dashes = '-'.repeat(
185+
circularPart.length + (alreadySeen.length + current.length) / 2 - 1
186+
);
187+
188+
throw new TypeError(
189+
'Converting circular structure to EJSON:\n' +
190+
` ${leadingPart}${alreadySeen}${circularPart}${current}\n` +
191+
` ${leadingSpace}\\${dashes}/`
192+
);
193+
}
194+
options.seenObjects[options.seenObjects.length - 1].obj = value;
195+
}
196+
156197
if (Array.isArray(value)) return serializeArray(value, options);
157198

158199
if (value === undefined) return null;
@@ -232,15 +273,20 @@ const BSON_TYPE_MAPPINGS = {
232273
} as const;
233274

234275
// eslint-disable-next-line @typescript-eslint/no-explicit-any
235-
function serializeDocument(doc: any, options: EJSON.Options) {
276+
function serializeDocument(doc: any, options: EJSONSerializeOptions) {
236277
if (doc == null || typeof doc !== 'object') throw new Error('not an object instance');
237278

238279
const bsontype: BSONType['_bsontype'] = doc._bsontype;
239280
if (typeof bsontype === 'undefined') {
240281
// It's a regular object. Recursively serialize its property values.
241282
const _doc: Document = {};
242283
for (const name in doc) {
243-
_doc[name] = serializeValue(doc[name], options);
284+
options.seenObjects.push({ propertyName: name, obj: null });
285+
try {
286+
_doc[name] = serializeValue(doc[name], options);
287+
} finally {
288+
options.seenObjects.pop();
289+
}
244290
}
245291
return _doc;
246292
} else if (isBSONType(doc)) {
@@ -365,9 +411,11 @@ export namespace EJSON {
365411
replacer = undefined;
366412
space = 0;
367413
}
368-
options = Object.assign({}, { relaxed: true, legacy: false }, options);
414+
const serializeOptions = Object.assign({ relaxed: true, legacy: false }, options, {
415+
seenObjects: [{ propertyName: '(root)', obj: null }]
416+
});
369417

370-
const doc = serializeValue(value, options);
418+
const doc = serializeValue(value, serializeOptions);
371419
return JSON.stringify(doc, replacer as Parameters<JSON['stringify']>[1], space);
372420
}
373421

test/node/extended_json_tests.js

+44
Original file line numberDiff line numberDiff line change
@@ -500,6 +500,50 @@ describe('Extended JSON', function () {
500500
// expect(() => EJSON.serialize(badMap)).to.throw(); // uncomment when EJSON supports ES6 Map
501501
});
502502

503+
context('circular references', () => {
504+
it('should throw a helpful error message for input with circular references', function () {
505+
const obj = {
506+
some: {
507+
property: {
508+
array: []
509+
}
510+
}
511+
};
512+
obj.some.property.array.push(obj.some);
513+
expect(() => EJSON.serialize(obj)).to.throw(`\
514+
Converting circular structure to EJSON:
515+
(root) -> some -> property -> array -> index 0
516+
\\-----------------------------/`);
517+
});
518+
519+
it('should throw a helpful error message for input with circular references, one-level nested', function () {
520+
const obj = {};
521+
obj.obj = obj;
522+
expect(() => EJSON.serialize(obj)).to.throw(`\
523+
Converting circular structure to EJSON:
524+
(root) -> obj
525+
\\-------/`);
526+
});
527+
528+
it('should throw a helpful error message for input with circular references, one-level nested inside base object', function () {
529+
const obj = {};
530+
obj.obj = obj;
531+
expect(() => EJSON.serialize({ foo: obj })).to.throw(`\
532+
Converting circular structure to EJSON:
533+
(root) -> foo -> obj
534+
\\------/`);
535+
});
536+
537+
it('should throw a helpful error message for input with circular references, pointing back to base object', function () {
538+
const obj = { foo: {} };
539+
obj.foo.obj = obj;
540+
expect(() => EJSON.serialize(obj)).to.throw(`\
541+
Converting circular structure to EJSON:
542+
(root) -> foo -> obj
543+
\\--------------/`);
544+
});
545+
});
546+
503547
context('when dealing with legacy extended json', function () {
504548
describe('.stringify', function () {
505549
context('when serializing binary', function () {

0 commit comments

Comments
 (0)