Skip to content

Commit f2d2317

Browse files
authored
fix: Export support logs is not working (#2531)
Exporting logs is currently failing with this error message: ``` Uncaught TypeError TypeError: Converting circular structure to JSON --> starting at object with constructor 'LinkedHashMap' | property 'hashCodeMap' -> object with constructor 'InternalHashCodeMap' --- property 'host_0' closes the circle at getReduxDataString (/Users/ericlin/dev/web-client-ui/packages/log/src/LogExport.ts:125:15) ``` We are handling circular references in `makeSafeToStringify()` before calling `JSON.stringify()`, but it seems that some of them are not being handled properly. This was tricky to debug since it was unclear to me where/how exactly it was failing. I spent some time trying to fix our custom implementation before settling with a library, [safe-stable-stringify](https://www.npmjs.com/package/safe-stable-stringify) (MIT license), that would handle circular structures and BigInt in a [performant](https://github.com/BridgeAR/safe-stable-stringify?tab=readme-ov-file#performance--benchmarks) way. There are a few differences between our previous custom implementation vs `safe-stable-stringify`: - Circular references are replaced with “[Circular]”, whereas before we also showed the path that the reference points to. This is a requested feature that doesn't seem will be worked on BridgeAR/safe-stable-stringify#43 - The order of keys are different when compared to previous logs. By default the library guarantees a deterministic key order instead of relying on insertion order, but this can be disabled to match the former behaviour if desired. - BigInts are converted to a number instead of a string in this library The blacklist was updated with new entries to reduce the log size and prevent length errors when exporting: - For instance, the `client` object, which was already blacklisted, was also found in a few other paths, which have now been blacklisted as well. Tested by: - Exporting logs on an account with many queries successfully with a reasonable size (62kb) - Comparing the result to logs from before this issue and verified that useful data has not been omitted. Note: After more testing, I tried using the new blacklist with our original custom implementation, and it appears to also work. I narrowed it down to `draftManager/draftStorage`, which seemed to be causing the circular structure error. However, when using the library, there is no error when serializing this key, albeit producing a 31mb log file, indicating that there is still something wrong with our implementation. We could solve this specific issue by blacklisting this key, but switching to a library might be a better idea to prevent such cases in the future.
1 parent ceda82b commit f2d2317

File tree

4 files changed

+40
-81
lines changed

4 files changed

+40
-81
lines changed

package-lock.json

Lines changed: 18 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/log/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@
2222
},
2323
"dependencies": {
2424
"event-target-shim": "^6.0.2",
25-
"jszip": "^3.10.1"
25+
"jszip": "^3.10.1",
26+
"safe-stable-stringify": "^2.5.0"
2627
},
2728
"files": [
2829
"dist"

packages/log/src/LogExport.test.ts

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ describe('getReduxDataString', () => {
2121
const expected = JSON.stringify(
2222
{
2323
key1: 'value1',
24-
key2: 'Circular ref to root',
24+
key2: '[Circular]',
2525
},
2626
null,
2727
2
@@ -30,17 +30,12 @@ describe('getReduxDataString', () => {
3030
});
3131

3232
it('should handle BigInt values', () => {
33-
const reduxData = {
34-
key1: BigInt('12345678901234567890'),
35-
};
33+
const reduxData = { key1: BigInt('12345678901234567890') };
3634
const result = getReduxDataString(reduxData);
37-
const expected = JSON.stringify(
38-
{
39-
key1: '12345678901234567890',
40-
},
41-
null,
42-
2
43-
);
35+
// Hardcode expected value since JSON.stringify would throw
36+
const expected = `{
37+
"key1": 12345678901234567890
38+
}`;
4439
expect(result).toBe(expected);
4540
});
4641

packages/log/src/LogExport.ts

Lines changed: 14 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import JSZip from 'jszip';
2+
import stringify from 'safe-stable-stringify';
23
import type LogHistory from './LogHistory';
34

45
// List of objects to blacklist
@@ -7,9 +8,14 @@ import type LogHistory from './LogHistory';
78
export const DEFAULT_PATH_BLACKLIST: string[][] = [
89
['api'],
910
['client'],
11+
['clientUtils'],
1012
['dashboardData', '*', 'connection'],
1113
['dashboardData', '*', 'sessionWrapper'],
14+
['dashboardData', '*', 'openedMap'],
15+
['draftManager', 'draftStorage'],
1216
['layoutStorage'],
17+
['schemaService', 'client'],
18+
['schemaService', 'schemaStorage'],
1319
['storage'],
1420
];
1521

@@ -55,77 +61,18 @@ function isOnBlackList(currPath: string[], blacklist: string[][]): boolean {
5561
return false;
5662
}
5763

58-
/**
59-
* Returns a new object that is safe to stringify
60-
* All circular references are replaced by the path to the value creating a circular ref
61-
* A circular ref will display 'Circular ref to root.someKey' in place of the circular ref
62-
* All BigInts are replaced with their toString since BigInts error JSON.stringify
63-
* https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify#Exceptions
64-
*
65-
* This tries to stringify each key of the object as an easy way to determine if it is safe
66-
* If it fails to stringify, then the values that failed repeat the same steps recursively
67-
* The unsafe objects are kept in a map with their path (e.g. root.someKey.someOtherKey)
68-
* Then if the object is seen again, it must be a circular ref since that object could not be stringified safely
69-
*
70-
* @param obj Object to make safe to stringify
71-
* @param blacklist List of JSON paths to blacklist. A JSON path is a list representing the path to that value (e.g. client.data would be `['client', 'data']`)
72-
*/
73-
function makeSafeToStringify(
74-
obj: Record<string, unknown>,
75-
blacklist: string[][],
76-
path = 'root',
77-
potentiallyCircularValues: Map<Record<string, unknown>, string> = new Map([
78-
[obj, 'root'],
79-
])
80-
): Record<string, unknown> {
81-
const output: Record<string, unknown> = {};
82-
83-
Object.entries(obj).forEach(([key, val]) => {
84-
try {
85-
JSON.stringify(val, stringifyReplacer(blacklist));
86-
output[key] = val;
87-
} catch (e) {
88-
// The value must be a Circular object or BigInt here
89-
const valRecord = val as Record<string, unknown>;
90-
91-
if (typeof val === 'bigint') {
92-
output[key] = val.toString();
93-
} else if (potentiallyCircularValues.has(valRecord)) {
94-
// We've seen this object before, so it's a circular ref
95-
output[key] = `Circular ref to ${potentiallyCircularValues.get(
96-
valRecord
97-
)}`;
98-
} else {
99-
// Haven't seen the object before, but somewhere in it there is a circular ref
100-
// The ref could point to this object or just to another child
101-
const curPath = `${path}.${key}`;
102-
potentiallyCircularValues.set(valRecord, curPath);
103-
// Convert the path to an array and remove the root
104-
const curPathArray = curPath.split('.').slice(1);
105-
// If the path is on the blacklist, it will eventually be replaced by undefined, so avoid the recursive call
106-
if (!isOnBlackList(curPathArray, blacklist)) {
107-
output[key] = makeSafeToStringify(
108-
val as Record<string, unknown>,
109-
blacklist,
110-
curPath,
111-
potentiallyCircularValues
112-
);
113-
}
114-
}
115-
}
116-
});
117-
118-
return output;
119-
}
120-
12164
export function getReduxDataString(
12265
reduxData: Record<string, unknown>,
12366
blacklist: string[][] = []
12467
): string {
125-
return JSON.stringify(
126-
makeSafeToStringify(reduxData, blacklist),
127-
stringifyReplacer(blacklist),
128-
2 // Indent w/ 2 spaces
68+
return (
69+
// Using safe-stable-stringify which handles circular references and BigInt
70+
// All circular references are replaced with "[Circular]", and BigInt values are converted to a number
71+
stringify(
72+
reduxData,
73+
stringifyReplacer(blacklist),
74+
2 // Indent w/ 2 spaces
75+
) ?? ''
12976
);
13077
}
13178

0 commit comments

Comments
 (0)