sdk-core: Allow to serialize and capture invalid attribute types (#365)

* sdk-core: Allow to serialize and capture invalid attribute types

* node, sdk-core: Implement similar fix to the breadcrumbs storage and limit object depth

* node: correct cast

* sdk-core: Catch Array.isArray with proxies

---------

Co-authored-by: Konrad Dysput <konrad.dysput@saucelabs.com>

Author: konraddysput

packages/node/src/breadcrumbs/FileBreadcrumbsStorage.ts

@@ -1,4 +1,5 @@
 import {
+    AttributeType,
     BacktraceAttachment,
     BacktraceAttachmentProvider,
     Breadcrumb,
@@ -107,9 +108,8 @@ export class FileBreadcrumbsStorage implements BreadcrumbsStorage {
             timestamp: TimeHelper.now(),
             type: BreadcrumbType[rawBreadcrumb.type].toLowerCase(),
             level: BreadcrumbLogLevel[rawBreadcrumb.level].toLowerCase(),
-            attributes: rawBreadcrumb.attributes,
+            attributes: this.prepareAttributes(rawBreadcrumb.attributes),
         };
-
         const breadcrumbJson = JSON.stringify(breadcrumb, jsonEscaper());
         const jsonLength = breadcrumbJson.length + 1; // newline
         const sizeLimit = this._limits.maximumTotalBreadcrumbsSize;
@@ -123,6 +123,46 @@ export class FileBreadcrumbsStorage implements BreadcrumbsStorage {
         return id;
     }
 
+    private prepareAttributes(attributes?: Record<string, AttributeType>): Record<string, AttributeType> | undefined {
+        const result: Record<string, AttributeType> = {};
+        if (!attributes) {
+            return undefined;
+        }
+        for (const key in attributes) {
+            const value = attributes[key];
+            switch (typeof value) {
+                case 'number':
+                case 'boolean':
+                case 'string':
+                case 'undefined':
+                    result[key] = value;
+                    break;
+                case 'bigint':
+                    result[key] = (value as bigint).toString();
+                    break;
+                case 'object': {
+                    if (!value) {
+                        result[key] = value;
+                        break;
+                    }
+                    const unknownValue = value as unknown;
+                    try {
+                        if (unknownValue instanceof Date) {
+                            result[key] = unknownValue.toISOString();
+                        } else if (unknownValue instanceof URL) {
+                            result[key] = unknownValue.toString();
+                        }
+                    } catch {
+                        // revoked proxy or broken object — drop it
+                    }
+                    // drop all other objects
+                    break;
+                }
+            }
+        }
+        return result;
+    }
+
     private static getFileName(index: number) {
         return `${FILE_PREFIX}-${index}`;
     }

packages/sdk-core/src/common/jsonSize.ts

@@ -30,7 +30,7 @@ function arraySize(array: unknown[], replacer?: JsonReplacer): number {
                 elementsLength += nullSize;
                 break;
             default:
-                elementsLength += _jsonSize(array, i.toString(), element, replacer);
+                elementsLength += _safeJsonSize(array, i.toString(), element, replacer);
         }
     }
 
@@ -45,7 +45,7 @@ const objectSize = (obj: object, replacer?: JsonReplacer): number => {
     let entriesLength = 0;
 
     for (const [k, v] of entries) {
-        const valueSize = _jsonSize(obj, k, v, replacer);
+        const valueSize = _safeJsonSize(obj, k, v, replacer);
         if (valueSize === 0) {
             continue;
         }
@@ -85,9 +85,25 @@ function keySize(key: unknown): number {
     }
 }
 
+function _safeJsonSize(parent: unknown, key: string, value: unknown, replacer?: JsonReplacer): number {
+    try {
+        return _jsonSize(parent, key, value, replacer);
+    } catch (err) {
+        return 0;
+    }
+}
+
 function _jsonSize(parent: unknown, key: string, value: unknown, replacer?: JsonReplacer): number {
-    if (value && typeof value === 'object' && 'toJSON' in value && typeof value.toJSON === 'function') {
-        value = value.toJSON() as object;
+    try {
+        if (value && typeof value === 'object' && 'toJSON' in value && typeof value.toJSON === 'function') {
+            value = value.toJSON() as object;
+        }
+    } catch (err) {
+        // handle proxy errors that will break other parts of the flow
+        if (err instanceof TypeError) {
+            return 0;
+        }
+        // continue in case of the error in the toJSON method or unsupported toJSON method
     }
 
     value = replacer ? replacer.call(parent, key, value) : value;
@@ -133,5 +149,5 @@ function _jsonSize(parent: unknown, key: string, value: unknown, replacer?: Json
  * @returns Final string length.
  */
 export function jsonSize(value: unknown, replacer?: JsonReplacer): number {
-    return _jsonSize(undefined, '', value, replacer);
+    return _safeJsonSize(undefined, '', value, replacer);
 }

packages/sdk-core/src/common/limitObjectDepth.ts

@@ -2,29 +2,48 @@ type DeepPartial<T extends object> = Partial<{ [K in keyof T]: T[K] extends obje
 
 const REMOVED_PLACEHOLDER = '<removed>';
 
-export type Limited<T extends object> = DeepPartial<T> | typeof REMOVED_PLACEHOLDER;
+export type Limited<T> = (T extends object ? DeepPartial<T> : T) | typeof REMOVED_PLACEHOLDER;
+
+export function limitObjectDepth<T>(val: T, depth: number): Limited<T> {
+    if (typeof val !== 'object' || !val) {
+        return val as Limited<T>;
+    }
 
-export function limitObjectDepth<T extends object>(obj: T, depth: number): Limited<T> {
     if (!(depth < Infinity)) {
-        return obj;
+        return val as Limited<T>;
     }
 
     if (depth < 0) {
         return REMOVED_PLACEHOLDER;
     }
 
-    const limitIfObject = (value: unknown) =>
-        typeof value === 'object' && value ? limitObjectDepth(value, depth - 1) : value;
+    try {
+        if ('toJSON' in val && typeof val.toJSON === 'function') {
+            return limitObjectDepth(val.toJSON(), depth);
+        }
+    } catch (err) {
+        if (err instanceof TypeError) {
+            return REMOVED_PLACEHOLDER;
+        }
+        // broken toJSON — fall through to iterate own properties
+    }
+
+    const limitChild = (value: unknown) => limitObjectDepth(value, depth - 1);
 
-    const result: DeepPartial<T> = {};
-    for (const key in obj) {
-        const value = obj[key];
-        if (Array.isArray(value)) {
-            result[key] = value.map(limitIfObject) as never;
-        } else {
-            result[key] = limitIfObject(value) as never;
+    const result: DeepPartial<T & object> = {};
+    for (const key in val) {
+        try {
+            const value = val[key];
+            if (Array.isArray(value)) {
+                result[key] = value.map(limitChild) as never;
+            } else {
+                result[key] = limitChild(value) as never;
+            }
+        } catch {
+            // catch revoked proxies and other broken objects
+            result[key] = REMOVED_PLACEHOLDER as never;
         }
     }
 
-    return result;
+    return result as Limited<T>;
 }

packages/sdk-core/src/model/http/BacktraceReportSubmission.ts

@@ -30,9 +30,20 @@ export class RequestBacktraceReportSubmission implements BacktraceReportSubmissi
         this._submissionUrl = SubmissionUrlInformation.toJsonReportSubmissionUrl(options.url, options.token);
     }
 
-    public send(data: BacktraceSubmitBody, attachments: BacktraceAttachment[], abortSignal?: AbortSignal) {
-        const json = JSON.stringify(data, jsonEscaper());
-        return this._requestHandler.postError(this._submissionUrl, json, attachments, abortSignal);
+    public send(
+        data: BacktraceSubmitBody,
+        attachments: BacktraceAttachment[],
+        abortSignal?: AbortSignal,
+    ): Promise<BacktraceReportSubmissionResult<BacktraceSubmissionResponse>> {
+        try {
+            const json = JSON.stringify(data, jsonEscaper());
+            return this._requestHandler.postError(this._submissionUrl, json, attachments, abortSignal);
+        } catch (error) {
+            // catch error generated during toJSON execution or unsupported objects to not cause the app crash.
+            return Promise.resolve(
+                BacktraceReportSubmissionResult.OnUnknownError(error instanceof Error ? error.message : String(error)),
+            );
+        }
     }
 
     public async sendAttachment(

packages/sdk-core/src/modules/attribute/ReportDataBuilder.ts

@@ -15,6 +15,19 @@ export class ReportDataBuilder {
             }
             switch (typeof attribute) {
                 case 'object': {
+                    try {
+                        // try to convert known objects into attributes
+                        if (attribute instanceof Date) {
+                            result.attributes[attributeKey] = attribute.toISOString();
+                            break;
+                        } else if (attribute instanceof URL) {
+                            result.attributes[attributeKey] = attribute.toString();
+                            break;
+                        }
+                    } catch {
+                        // invalid attribute type - not able to serialize, skip it.
+                        break;
+                    }
                     result.annotations[attributeKey] = attribute;
                     break;
                 }

packages/sdk-core/tests/breadcrumbs/breadcrumbsCreationTests.spec.ts

@@ -1,3 +1,4 @@
+import { AttributeType } from '../../src/index.js';
 import { BreadcrumbsManager } from '../../src/modules/breadcrumbs/BreadcrumbsManager.js';
 import { BreadcrumbLogLevel, BreadcrumbType } from '../../src/modules/breadcrumbs/index.js';
 import { InMemoryBreadcrumbsStorage } from '../../src/modules/breadcrumbs/storage/InMemoryBreadcrumbsStorage.js';
@@ -123,4 +124,22 @@ describe('Breadcrumbs creation tests', () => {
 
         expect(breadcrumb.attributes).toMatchObject(attributes);
     });
+    it('Should handle breadcrumb with not serializable attributes', () => {
+        const message = 'test';
+        const level = BreadcrumbLogLevel.Warning;
+        const attributes = {
+            url: new URL('https://example.com/path?q=1'),
+            date: new Date(),
+            objectCreatePrototype: Object.create(Date.prototype),
+            destroyedUrl: { ...new URL('https://example.com/path?q=1'), date: new Date() },
+        } as unknown as Record<string, AttributeType>;
+        const storage = new InMemoryBreadcrumbsStorage({ maximumBreadcrumbs: 100 });
+        const breadcrumbsManager = new BreadcrumbsManager(undefined, { storage: () => storage });
+        breadcrumbsManager.initialize();
+        breadcrumbsManager.log(message, level, attributes);
+        const [breadcrumb] = JSON.parse(storage.get() as string);
+
+        expect(breadcrumb.attributes['url']).toBeDefined();
+        expect(breadcrumb.attributes['date']).toBeDefined();
+    });
 });

packages/sdk-core/tests/client/attributesTests.spec.ts

@@ -1,6 +1,11 @@
 import { BacktraceTestClient } from '../mocks/BacktraceTestClient.js';
+import { testHttpClient } from '../mocks/testHttpClient.js';
 
 describe('Attributes tests', () => {
+    beforeEach(() => {
+        jest.mocked(testHttpClient.postError).mockClear();
+    });
+
     describe('Client attribute add', () => {
         it('Should add an attribute to the client cache', () => {
             const client = BacktraceTestClient.buildFakeClient();
@@ -80,4 +85,93 @@ describe('Attributes tests', () => {
             expect(scopedAttributeGetFunction).toHaveBeenCalledTimes(2);
         });
     });
+
+    describe('Non-serializable attributes', () => {
+        it('Should convert Date attribute to ISO string', async () => {
+            const client = BacktraceTestClient.buildFakeClient();
+            const date = new Date();
+
+            await client.send(new Error('test'), { date });
+
+            const [[, json]] = (client.requestHandler.postError as jest.Mock).mock.calls;
+            const body = JSON.parse(json);
+            expect(body.attributes.date).toEqual(date.toISOString());
+        });
+
+        it('Should convert URL attribute to string', async () => {
+            const client = BacktraceTestClient.buildFakeClient();
+            const url = new URL('https://example.com/path?q=1');
+
+            await client.send(new Error('test'), { url });
+
+            const [[, json]] = (client.requestHandler.postError as jest.Mock).mock.calls;
+            const body = JSON.parse(json);
+            expect(body.attributes.url).toEqual(url.toString());
+        });
+
+        it('Should handle URL instance as annotation', async () => {
+            const client = BacktraceTestClient.buildFakeClient();
+
+            await client.send(new Error('test'), {
+                destroyedClassInstance: { ...new URL('https://example.com') },
+            });
+
+            const [[, json]] = (client.requestHandler.postError as jest.Mock).mock.calls;
+            expect(() => JSON.parse(json)).not.toThrow();
+        });
+
+        it('Should handle Object.create with URL prototype', async () => {
+            const client = BacktraceTestClient.buildFakeClient();
+
+            await client.send(new Error('test'), {
+                createdObjectViaPrototype: Object.create(URL.prototype),
+            });
+
+            const [[, json]] = (client.requestHandler.postError as jest.Mock).mock.calls;
+            expect(() => JSON.parse(json)).not.toThrow();
+        });
+
+        it('Should return submission error for object with broken toJSON', async () => {
+            const client = BacktraceTestClient.buildFakeClient();
+
+            const result = await client.send(new Error('test'), {
+                brokenToJSON: {
+                    toJSON() {
+                        throw new Error('broken toJSON');
+                    },
+                },
+            });
+
+            expect(result.status).toEqual('Unknown');
+        });
+
+        it('Should handle spread class instance with private fields', async () => {
+            class Strict {
+                #data = 'secret';
+                toJSON() {
+                    return this.#data;
+                }
+            }
+            const client = BacktraceTestClient.buildFakeClient();
+
+            await client.send(new Error('test'), {
+                strict: { ...new Strict() },
+            });
+
+            const [[, json]] = (client.requestHandler.postError as jest.Mock).mock.calls;
+            expect(() => JSON.parse(json)).not.toThrow();
+        });
+
+        it('Should handle revoked proxy nested in object', async () => {
+            const { proxy, revoke } = Proxy.revocable({ toJSON: () => 'ok' }, {});
+            revoke();
+            const client = BacktraceTestClient.buildFakeClient();
+
+            const result = await client.send(new Error('test'), {
+                revokedProxy: { data: proxy },
+            });
+
+            expect(result.status).toEqual('Unknown');
+        });
+    });
 });