From 2e6bc64c32072f2e9d8e092aa471af37b6023254 Mon Sep 17 00:00:00 2001 From: Corie Watson Date: Tue, 11 Mar 2025 14:36:15 +0000 Subject: [PATCH 1/4] fix(logger): no longer detects duplicate object as circular --- spec/logger.spec.ts | 44 ++++++++++++++++++++++++++++++++++++++++++++ src/logger/index.ts | 29 +++++++++++++---------------- 2 files changed, 57 insertions(+), 16 deletions(-) diff --git a/spec/logger.spec.ts b/spec/logger.spec.ts index 4ad64703b..a42a57ee3 100644 --- a/spec/logger.spec.ts +++ b/spec/logger.spec.ts @@ -118,6 +118,50 @@ describe("logger", () => { }); }); + it("should not detect duplicate object as circular", () => { + const obj: any = { a: "foo" }; + const entry: logger.LogEntry = { + severity: "ERROR", + message: "testing circular", + a: obj, + b: obj, + }; + logger.write(entry); + expectStderr({ + severity: "ERROR", + message: "testing circular", + a: { a: "foo" }, + b: { a: "foo" }, + }); + }); + + it("should not detect duplicate object in array as circular", () => { + const obj: any = { a: "foo" }; + const arr: any = [ + { a: obj, b: obj }, + { a: obj, b: obj }, + ]; + const entry: logger.LogEntry = { + severity: "ERROR", + message: "testing circular", + a: arr, + b: arr, + }; + logger.write(entry); + expectStderr({ + severity: "ERROR", + message: "testing circular", + a: [ + { a: { a: "foo" }, b: { a: "foo" } }, + { a: { a: "foo" }, b: { a: "foo" } }, + ], + b: [ + { a: { a: "foo" }, b: { a: "foo" } }, + { a: { a: "foo" }, b: { a: "foo" } }, + ], + }); + }); + it("should not break on objects that override toJSON", () => { const obj: any = { a: new Date("August 26, 1994 12:24:00Z") }; diff --git a/src/logger/index.ts b/src/logger/index.ts index a9f2aec9e..4854951d4 100644 --- a/src/logger/index.ts +++ b/src/logger/index.ts @@ -56,28 +56,25 @@ function removeCircular(obj: any, refs: any[] = []): any { if (typeof obj !== "object" || !obj) { return obj; } - // If the object defines its own toJSON, prefer that. - if (obj.toJSON) { + // If the object defines its own toJSON method, use it. + if (obj.toJSON && typeof obj.toJSON === "function") { return obj.toJSON(); } - if (refs.includes(obj)) { + // Only check for circularity among ancestors in the recursion stack. + if (refs.indexOf(obj) !== -1) { return "[Circular]"; - } else { - refs.push(obj); } - let returnObj: any; - if (Array.isArray(obj)) { - returnObj = new Array(obj.length); - } else { - returnObj = {}; - } - for (const k in obj) { - if (refs.includes(obj[k])) { - returnObj[k] = "[Circular]"; - } else { - returnObj[k] = removeCircular(obj[k], refs); + // Add the current object to the recursion stack. + refs.push(obj); + + const returnObj: any = Array.isArray(obj) ? [] : {}; + for (const key in obj) { + if (Object.prototype.hasOwnProperty.call(obj, key)) { + returnObj[key] = removeCircular(obj[key], refs); } } + // Remove the current object from the stack once its properties are processed. + refs.pop(); return returnObj; } From cf2cb4d8874f2748d0bff6d5a7ffd2d4cae09d81 Mon Sep 17 00:00:00 2001 From: Corie Watson Date: Wed, 19 Mar 2025 12:47:52 +0000 Subject: [PATCH 2/4] perf(logger): improved performance of circular removal --- src/logger/index.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/logger/index.ts b/src/logger/index.ts index 4854951d4..b718b53bd 100644 --- a/src/logger/index.ts +++ b/src/logger/index.ts @@ -61,7 +61,7 @@ function removeCircular(obj: any, refs: any[] = []): any { return obj.toJSON(); } // Only check for circularity among ancestors in the recursion stack. - if (refs.indexOf(obj) !== -1) { + if (refs.includes(obj)) { return "[Circular]"; } // Add the current object to the recursion stack. @@ -69,10 +69,9 @@ function removeCircular(obj: any, refs: any[] = []): any { const returnObj: any = Array.isArray(obj) ? [] : {}; for (const key in obj) { - if (Object.prototype.hasOwnProperty.call(obj, key)) { - returnObj[key] = removeCircular(obj[key], refs); - } + returnObj[key] = removeCircular(obj[key], refs); } + // Remove the current object from the stack once its properties are processed. refs.pop(); return returnObj; From 2533542037b4220b377a3fb5792341f3d671c330 Mon Sep 17 00:00:00 2001 From: Corie Watson Date: Wed, 19 Mar 2025 13:24:16 +0000 Subject: [PATCH 3/4] chore(logger): changes due to lint --- src/logger/index.ts | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/logger/index.ts b/src/logger/index.ts index b718b53bd..f5dc0d717 100644 --- a/src/logger/index.ts +++ b/src/logger/index.ts @@ -67,9 +67,17 @@ function removeCircular(obj: any, refs: any[] = []): any { // Add the current object to the recursion stack. refs.push(obj); - const returnObj: any = Array.isArray(obj) ? [] : {}; - for (const key in obj) { - returnObj[key] = removeCircular(obj[key], refs); + // If the object is an array, run circular check on each element. + let returnObj: any; + if (Array.isArray(obj)) { + returnObj = obj.map((item) => removeCircular(item, refs)); + } + // If the object is a Map, run circular check on each key and value. + else { + returnObj = {}; + for (const key of Object.keys(obj)) { + returnObj[key] = removeCircular(obj[key], refs); + } } // Remove the current object from the stack once its properties are processed. From 3448a8b7fbb250c61b6aa94ad4747adf146a53c4 Mon Sep 17 00:00:00 2001 From: Corie Watson Date: Wed, 19 Mar 2025 13:41:17 +0000 Subject: [PATCH 4/4] perf(logger): nvm, the original is probably more efficient --- src/logger/index.ts | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/src/logger/index.ts b/src/logger/index.ts index f5dc0d717..2a2d85ae7 100644 --- a/src/logger/index.ts +++ b/src/logger/index.ts @@ -56,31 +56,28 @@ function removeCircular(obj: any, refs: any[] = []): any { if (typeof obj !== "object" || !obj) { return obj; } - // If the object defines its own toJSON method, use it. - if (obj.toJSON && typeof obj.toJSON === "function") { + // If the object defines its own toJSON, prefer that. + if (obj.toJSON) { return obj.toJSON(); } - // Only check for circularity among ancestors in the recursion stack. if (refs.includes(obj)) { return "[Circular]"; + } else { + refs.push(obj); } - // Add the current object to the recursion stack. - refs.push(obj); - - // If the object is an array, run circular check on each element. let returnObj: any; if (Array.isArray(obj)) { - returnObj = obj.map((item) => removeCircular(item, refs)); - } - // If the object is a Map, run circular check on each key and value. - else { + returnObj = new Array(obj.length); + } else { returnObj = {}; - for (const key of Object.keys(obj)) { - returnObj[key] = removeCircular(obj[key], refs); + } + for (const k in obj) { + if (refs.includes(obj[k])) { + returnObj[k] = "[Circular]"; + } else { + returnObj[k] = removeCircular(obj[k], refs); } } - - // Remove the current object from the stack once its properties are processed. refs.pop(); return returnObj; }