Skip to content

Commit

Permalink
fix(core): Fix circular reference error in email sending job
Browse files Browse the repository at this point in the history
Fixes to #3277
  • Loading branch information
michaelbromley committed Dec 17, 2024
1 parent 0c122bc commit 02bcdba
Show file tree
Hide file tree
Showing 6 changed files with 145 additions and 42 deletions.
11 changes: 11 additions & 0 deletions packages/core/e2e/entity-serialization.e2e-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,17 @@ describe('Entity serialization', () => {
assertCanBeSerialized(shippingLine);
});

it('serialize Order with nested ShippingMethod', async () => {
const ctx = await createCtx();
const order = await server.app
.get(OrderService)
.findOne(ctx, 1, ['lines', 'shippingLines.shippingMethod']);

expect(order).not.toBeNull();
expect(order instanceof Order).toBe(true);
assertCanBeSerialized(order);
});

function assertCanBeSerialized(entity: any) {
try {
const json = JSON.stringify(entity);
Expand Down
8 changes: 8 additions & 0 deletions packages/core/src/config/config.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,4 +146,12 @@ export class ConfigService implements VendureConfig {
}
return definedCustomFields;
}

/**
* This is a precaution against attempting to JSON.stringify() a reference to
* this class, which can lead to a circular reference error.
*/
protected toJSON() {
return {};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,14 @@ export class ShippingEligibilityChecker<
}
return true;
}

/**
* This is a precaution against attempting to JSON.stringify() a reference to
* this class, which can lead to a circular reference error.
*/
protected toJSON() {
return {};
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ export class ShippingMethod
* This is a fix for https://github.com/vendure-ecommerce/vendure/issues/3277,
* to prevent circular references which cause the JSON.stringify() to fail.
*/
protected toJSON() {
protected toJSON(): any {
return omit(this, ['allCheckers', 'allCalculators'] as any);
}
}
94 changes: 69 additions & 25 deletions packages/core/src/job-queue/job.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,36 +116,80 @@ describe('Job class', () => {
name: 'parent',
child: {
name: 'child',
parent: {
child: {
name: 'child',
parent: {
child: {
name: 'child',
parent: {
child: {
name: 'child',
parent: {
child: {
name: 'child',
parent: {
child: '[max depth reached]',
name: '[max depth reached]',
},
},
name: 'parent',
},
},
name: 'parent',
},
},
name: 'parent',
parent: '[circular *child.parent]',
},
});
});

it('handles objects with deep cycles', () => {
const parent = {
name: 'parent',
child1: {
name: 'child1',
child2: {
name: 'child2',
child3: {
name: 'child3',
child4: {
name: 'child4',
parent: {} as any,
},
},
},
},
};
parent.child1.child2.child3.child4.parent = parent;

const job = new Job({
queueName: 'test',
data: parent,
});

expect(job.data).toEqual({
name: 'parent',
child1: {
name: 'child1',
child2: {
name: 'child2',
child3: {
name: 'child3',
child4: {
name: 'child4',
parent: '[circular *child1.child2.child3.child4.parent]',
},
},
name: 'parent',
},
},
});
});

it('handles class instances with cycles', async () => {
class Parent {
name = 'parent';
child = new Child();
}

class Child {
name = 'child';
parent = undefined as Parent | undefined;
}

const parent = new Parent();
const child = parent.child;
child.parent = parent;

const job = new Job({
queueName: 'test',
data: parent as any,
});

expect(job.data).toEqual({
name: 'parent',
child: {
name: 'child',
parent: '[circular *child.parent]',
},
});
});
});
});
64 changes: 48 additions & 16 deletions packages/core/src/job-queue/job.ts
Original file line number Diff line number Diff line change
Expand Up @@ -231,27 +231,55 @@ export class Job<T extends JobData<T> = any> {
* already be serializable per the TS type, in practice data can slip through due to loss of
* type safety.
*/
private ensureDataIsSerializable(data: any, depth = 0): any {
private ensureDataIsSerializable(
data: any,
depth = 0,
seen = new WeakMap<any, string[]>(),
path: string[] = [],
): any {
if (10 < depth) {
return '[max depth reached]';
}
depth++;
let output: any;
if (data === null || data === undefined) {
return data;
}
// Handle Date objects
if (data instanceof Date) {
return data.toISOString();
} else if (isObject(data)) {
if (!output) {
output = {};
}
for (const key of Object.keys(data)) {
output[key] = this.ensureDataIsSerializable((data as any)[key], depth);
}

if (typeof data === 'object' && data !== null) {
const seenData = seen.get(data);
if (seenData && seenData.length < path.length) {
return `[circular *${path.join('.')}]`;
}
if (isClassInstance(data)) {
const descriptors = Object.getOwnPropertyDescriptors(Object.getPrototypeOf(data));
for (const name of Object.keys(descriptors)) {
const descriptor = descriptors[name];
if (typeof descriptor.get === 'function') {
output[name] = (data as any)[name];
seen.set(data, path);
}

depth++;
let output: any;
if (isObject(data)) {
output = {};
// If the object has a `.toJSON()` function defined, then
// prefer it to any other type of serialization.
if (this.hasToJSONFunction(data)) {
output = data.toJSON();
} else {
for (const key of Object.keys(data)) {
output[key] = this.ensureDataIsSerializable(
(data as any)[key],
depth,
seen,
path.concat(key),
);
}
if (isClassInstance(data)) {
const descriptors = Object.getOwnPropertyDescriptors(Object.getPrototypeOf(data));
for (const name of Object.keys(descriptors)) {
const descriptor = descriptors[name];
if (typeof descriptor.get === 'function') {
output[name] = (data as any)[name];
}
}
}
}
Expand All @@ -260,11 +288,15 @@ export class Job<T extends JobData<T> = any> {
output = [];
}
data.forEach((item, i) => {
output[i] = this.ensureDataIsSerializable(item, depth);
output[i] = this.ensureDataIsSerializable(item, depth, seen, path.concat(i.toString()));
});
} else {
return data;
}
return output;
}

private hasToJSONFunction(obj: any): obj is { toJSON(): any } {
return typeof obj?.toJSON === 'function';
}
}

0 comments on commit 02bcdba

Please sign in to comment.