ajaxerror not serialized properly (#4211)

* add failing unit test for ajaxError serialization not working  - all extendend properties get lost

* change all worker_transfer tests to use structuredClone for realistic test; note that jsdom Blob does not work with node native structuredClone so that one line is disabled

* serialize/deserialize Error so extends of Error pass through postMessage; e.g. AJAXError which already is registered for transfer but ignored because it's instance of Error

* changelog entry

* if the sublcass of a built-in object is registered, use the registered class, otherwise use the built-in to serialize/deserialize; fixes the issue that most Errors are (de)serialized by the builtin but AjaxError extend is not

* Revert "if the sublcass of a built-in object is registered, use the registered class, otherwise use the built-in to serialize/deserialize; fixes the issue that most Errors are (de)serialized by the builtin but AjaxError extend is not"

This reverts commit 5f378cd.

* when serializing/deserializing check if the class is registered

* style fix as reviewed

Co-authored-by: Harel M <harel.mazor@gmail.com>

* refactor for less duplication

* Revert "change all worker_transfer tests to use structuredClone for realistic test; note that jsdom Blob does not work with node native structuredClone so that one line is disabled"

This reverts commit cbf397d.

* change the ajaxerror test to not use structuredClone until jsdom provides it since we don't want a custom jest env

* improve changelog entry

* add more tests to web_worker_transfer

* deserialize uses $name

* add more tests - note how the test agains BigInt really shows that the implementation is missing the ability to serialize/deserialize some valid objects which can probably be seralized/deserialized by primitives

* explicit types

---------

Co-authored-by: Harel M <harel.mazor@gmail.com>

CHANGELOG.md

@@ -4,7 +4,7 @@
 - _...Add new stuff here..._
 
 ### 🐞 Bug fixes
-- _...Add new stuff here..._
+- Fix (de)serialization of extends of built-ins (currently only AjaxError) not working correctly in web_worker_transfer. Also refactored related web_worker_transfer code and added more tests ([#4024](https://github.com/maplibre/maplibre-gl-js/pull/4211))
 
 ## 4.4.1
 

src/util/web_worker_transfer.test.ts

@@ -1,3 +1,5 @@
+import {SerializedObject} from '../../dist/maplibre-gl';
+import {AJAXError} from './ajax';
 import {register, serialize, deserialize} from './web_worker_transfer';
 
 describe('web worker transfer', () => {
@@ -7,11 +9,17 @@ describe('web worker transfer', () => {
             buffer;
             blob;
             _cached;
+            dataView;
+            imageData;
+            array;
 
             constructor(n) {
                 this.n = n;
                 this.buffer = new ArrayBuffer(100);
+                this.dataView = new DataView(this.buffer);
+                this.imageData = new ImageData(8, 5);
                 this.blob = new Blob();
+                this.array = [true, 1, 'one', new ArrayBuffer(100)];
                 this.squared();
             }
 
@@ -30,15 +38,19 @@ describe('web worker transfer', () => {
         const transferables = [];
         const deserialized = deserialize(serialize(serializableMock, transferables)) as SerializableMock;
         expect(deserialize(serialize(serializableMock, transferables)) instanceof SerializableMock).toBeTruthy();
+        expect(serializableMock.dataView instanceof DataView).toBeTruthy();
 
         expect(serializableMock !== deserialized).toBeTruthy();
         expect(deserialized.constructor === SerializableMock).toBeTruthy();
         expect(deserialized.n === 10).toBeTruthy();
         expect(deserialized.buffer === serializableMock.buffer).toBeTruthy();
         expect(deserialized.blob === serializableMock.blob).toBeTruthy();
         expect(transferables[0] === serializableMock.buffer).toBeTruthy();
+        expect(transferables[1] === serializableMock.dataView.buffer).toBeTruthy();
         expect(deserialized._cached === undefined).toBeTruthy();
         expect(deserialized.squared() === 100).toBeTruthy();
+        expect(deserialized.dataView instanceof DataView).toBeTruthy();
+        expect(deserialized.array).toEqual(serializableMock.array);
     });
 
     test('anonymous class', () => {
@@ -80,4 +92,62 @@ describe('web worker transfer', () => {
         expect(deserialized.id).toBe(customSerialization.id);
         expect(deserialized._deserialized).toBeTruthy();
     });
+
+    test('AjaxError serialization', () => {
+        const status = 404;
+        const statusText = 'not found';
+        const url = 'https://example.com';
+
+        const ajaxError = new AJAXError(status, statusText, url, new Blob());
+        const serialized = serialize(ajaxError) as SerializedObject;
+        expect(serialized.$name).toBe(ajaxError.constructor.name);
+        const deserialized = deserialize(serialized) as AJAXError;
+        expect(deserialized.status).toBe(404);
+        expect(deserialized.statusText).toBe(statusText);
+        expect(deserialized.url).toBe(url);
+    });
+
+    test('serialize Object has _classRegistryKey', () => {
+        class BadClass {
+            _classRegistryKey: 'foo';
+        }
+        const trySerialize = () => {
+            serialize(new BadClass());
+        };
+        expect(trySerialize).toThrow();
+    });
+    test('serialize can not used reserved property #name', () => {
+        class BadClass {
+            static serialize() {
+                return {
+                    '$name': 'foo'
+                };
+            }
+        }
+        register('BadClass', BadClass);
+        const badObject = new BadClass();
+        expect(() => {
+            serialize(badObject);
+        }).toThrow();
+    });
+    test('deserialize Object has $name', () => {
+        const badObject = {
+            '$name': 'foo'
+        };
+        const tryDeserialize = () => {
+            deserialize(badObject);
+        };
+        expect(tryDeserialize).toThrow();
+    });
+
+    test('some objects can not be serialized', () => {
+        expect(() => {
+            serialize(BigInt(123));
+        }).toThrow();
+    });
+    test('some objects can not be deserialized', () => {
+        expect(() => {
+            deserialize(<SerializedObject><unknown>BigInt(123));
+        }).toThrow();
+    });
 });

src/util/web_worker_transfer.ts

@@ -21,6 +21,7 @@ type Registry = {
         klass: {
             new (...args: any): any;
             deserialize?: (input: Serialized) => unknown;
+            serialize?: (input: any, transferables: Transferable[]) => SerializedObject;
         };
         omit: ReadonlyArray<string>;
         shallow: ReadonlyArray<string>;
@@ -91,6 +92,43 @@ function isArrayBuffer(value: any): value is ArrayBuffer {
            (value instanceof ArrayBuffer || (value.constructor && value.constructor.name === 'ArrayBuffer'));
 }
 
+function getClassRegistryKey(input: Object|SerializedObject): string {
+    const klass = (input.constructor as any);
+    return (input as SerializedObject).$name || klass._classRegistryKey;
+}
+
+function isRegistered(input: unknown): boolean {
+    if (input === null || typeof input !== 'object') {
+        return false;
+    }
+    const classRegistryKey = getClassRegistryKey(input);
+    if (classRegistryKey && classRegistryKey !== 'Object') {
+        return true;
+    }
+    return false;
+}
+
+function isSerializeHandledByBuiltin(input: unknown) {
+    return (!isRegistered(input) && (
+        input === null ||
+        input === undefined ||
+        typeof input === 'boolean' ||
+        typeof input === 'number' ||
+        typeof input === 'string' ||
+        input instanceof Boolean ||
+        input instanceof Number ||
+        input instanceof String ||
+        input instanceof Date ||
+        input instanceof RegExp ||
+        input instanceof Blob ||
+        input instanceof Error ||
+        isArrayBuffer(input) ||
+        isImageBitmap(input) ||
+        ArrayBuffer.isView(input) ||
+        input instanceof ImageData)
+    );
+}
+
 /**
  * Serialize the given object for transfer to or from a web worker.
  *
@@ -104,46 +142,22 @@ function isArrayBuffer(value: any): value is ArrayBuffer {
  * this should happen in the client code, before using serialize().)
  */
 export function serialize(input: unknown, transferables?: Array<Transferable> | null): Serialized {
-    if (input === null ||
-        input === undefined ||
-        typeof input === 'boolean' ||
-        typeof input === 'number' ||
-        typeof input === 'string' ||
-        input instanceof Boolean ||
-        input instanceof Number ||
-        input instanceof String ||
-        input instanceof Date ||
-        input instanceof RegExp ||
-        input instanceof Blob ||
-        input instanceof Error) {
-        return input;
-    }
-
-    if (isArrayBuffer(input)) {
-        if (transferables) {
-            transferables.push(input);
-        }
-        return input;
-    }
-
-    if (isImageBitmap(input)) {
-        if (transferables) {
-            transferables.push(input);
+    if (isSerializeHandledByBuiltin(input)) {
+        if (isArrayBuffer(input) || isImageBitmap(input)) {
+            if (transferables) {
+                transferables.push(input);
+            }
         }
-        return input;
-    }
-
-    if (ArrayBuffer.isView(input)) {
-        const view = input;
-        if (transferables) {
-            transferables.push(view.buffer);
+        if (ArrayBuffer.isView(input)) {
+            const view = input;
+            if (transferables) {
+                transferables.push(view.buffer);
+            }
         }
-        return view;
-    }
-
-    if (input instanceof ImageData) {
-        if (transferables) {
-            transferables.push(input.data.buffer);
+        if (input instanceof ImageData) {
+            if (transferables) {
+                transferables.push(input.data.buffer);
+            }
         }
         return input;
     }
@@ -156,103 +170,85 @@ export function serialize(input: unknown, transferables?: Array<Transferable> |
         return serialized;
     }
 
-    if (typeof input === 'object') {
-        const klass = (input.constructor as any);
-        const name = klass._classRegistryKey;
-        if (!name) {
-            throw new Error(`can't serialize object of unregistered class ${klass.name}`);
-        }
-        if (!registry[name]) throw new Error(`${name} is not registered.`);
-
-        const properties: SerializedObject = klass.serialize ?
-            // (Temporary workaround) allow a class to provide static
-            // `serialize()` and `deserialize()` methods to bypass the generic
-            // approach.
-            // This temporary workaround lets us use the generic serialization
-            // approach for objects whose members include instances of dynamic
-            // StructArray types. Once we refactor StructArray to be static,
-            // we can remove this complexity.
-            (klass.serialize(input, transferables) as SerializedObject) : {};
-
-        if (!klass.serialize) {
-            for (const key in input) {
-                if (!input.hasOwnProperty(key)) continue; // eslint-disable-line no-prototype-builtins
-                if (registry[name].omit.indexOf(key) >= 0) continue;
-                const property = input[key];
-                properties[key] = registry[name].shallow.indexOf(key) >= 0 ?
-                    property :
-                    serialize(property, transferables);
-            }
-            if (input instanceof Error) {
-                properties.message = input.message;
-            }
-        } else {
-            if (transferables && properties === transferables[transferables.length - 1]) {
-                throw new Error('statically serialized object won\'t survive transfer of $name property');
-            }
+    if (typeof input !== 'object') {
+        throw new Error(`can't serialize object of type ${typeof input}`);
+    }
+    const classRegistryKey = getClassRegistryKey(input);
+    if (!classRegistryKey) {
+        throw new Error(`can't serialize object of unregistered class ${input.constructor.name}`);
+    }
+    if (!registry[classRegistryKey]) throw new Error(`${classRegistryKey} is not registered.`);
+    const {klass} = registry[classRegistryKey];
+    const properties: SerializedObject = klass.serialize ?
+        // (Temporary workaround) allow a class to provide static
+        // `serialize()` and `deserialize()` methods to bypass the generic
+        // approach.
+        // This temporary workaround lets us use the generic serialization
+        // approach for objects whose members include instances of dynamic
+        // StructArray types. Once we refactor StructArray to be static,
+        // we can remove this complexity.
+        (klass.serialize(input, transferables) as SerializedObject) : {};
+
+    if (!klass.serialize) {
+        for (const key in input) {
+            if (!input.hasOwnProperty(key)) continue; // eslint-disable-line no-prototype-builtins
+            if (registry[classRegistryKey].omit.indexOf(key) >= 0) continue;
+            const property = input[key];
+            properties[key] = registry[classRegistryKey].shallow.indexOf(key) >= 0 ?
+                property :
+                serialize(property, transferables);
         }
-
-        if (properties.$name) {
-            throw new Error('$name property is reserved for worker serialization logic.');
+        if (input instanceof Error) {
+            properties.message = input.message;
         }
-        if (name !== 'Object') {
-            properties.$name = name;
+    } else {
+        if (transferables && properties === transferables[transferables.length - 1]) {
+            throw new Error('statically serialized object won\'t survive transfer of $name property');
         }
+    }
 
-        return properties;
+    if (properties.$name) {
+        throw new Error('$name property is reserved for worker serialization logic.');
+    }
+    if (classRegistryKey !== 'Object') {
+        properties.$name = classRegistryKey;
     }
 
-    throw new Error(`can't serialize object of type ${typeof input}`);
+    return properties;
 }
 
 export function deserialize(input: Serialized): unknown {
-    if (input === null ||
-        input === undefined ||
-        typeof input === 'boolean' ||
-        typeof input === 'number' ||
-        typeof input === 'string' ||
-        input instanceof Boolean ||
-        input instanceof Number ||
-        input instanceof String ||
-        input instanceof Date ||
-        input instanceof RegExp ||
-        input instanceof Blob ||
-        input instanceof Error ||
-        isArrayBuffer(input) ||
-        isImageBitmap(input) ||
-        ArrayBuffer.isView(input) ||
-        input instanceof ImageData) {
+    if (isSerializeHandledByBuiltin(input)) {
         return input;
     }
 
     if (Array.isArray(input)) {
         return input.map(deserialize);
     }
 
-    if (typeof input === 'object') {
-        const name = input.$name || 'Object';
-        if (!registry[name]) {
-            throw new Error(`can't deserialize unregistered class ${name}`);
-        }
-        const {klass} = registry[name];
-        if (!klass) {
-            throw new Error(`can't deserialize unregistered class ${name}`);
-        }
-
-        if (klass.deserialize) {
-            return klass.deserialize(input);
-        }
+    if (typeof input !== 'object') {
+        throw new Error(`can't deserialize object of type ${typeof input}`);
+    }
+    const classRegistryKey = getClassRegistryKey(input) || 'Object';
+    if (!registry[classRegistryKey]) {
+        throw new Error(`can't deserialize unregistered class ${classRegistryKey}`);
+    }
+    const {klass} = registry[classRegistryKey];
+    if (!klass) {
+        throw new Error(`can't deserialize unregistered class ${classRegistryKey}`);
+    }
 
-        const result = Object.create(klass.prototype);
+    if (klass.deserialize) {
+        return klass.deserialize(input);
+    }
 
-        for (const key of Object.keys(input)) {
-            if (key === '$name') continue;
-            const value = (input as SerializedObject)[key];
-            result[key] = registry[name].shallow.indexOf(key) >= 0 ? value : deserialize(value);
-        }
+    const result = Object.create(klass.prototype);
 
-        return result;
+    for (const key of Object.keys(input)) {
+        if (key === '$name') continue;
+        const value = (input as SerializedObject)[key];
+        result[key] = registry[classRegistryKey].shallow.indexOf(key) >= 0 ? value : deserialize(value);
     }
 
-    throw new Error(`can't deserialize object of type ${typeof input}`);
+    return result;
 }