Refactor encode function to use WeakSet for self reference tracking, …

…and to throw on this case

spec/common/providers/https.spec.ts

@@ -869,7 +869,7 @@ describe("encoding/decoding", () => {
     });
   });
 
-  it("encodes object with self reference", () => {
+  it("Throws object with self reference", () => {
     class TestClass {
       foo: string;
       bar: number;
@@ -881,87 +881,7 @@ describe("encoding/decoding", () => {
       }
     }
     const testObject = new TestClass("hello", 1);
-    expect(https.encode(testObject)).to.deep.equal({
-      foo: "hello",
-      bar: 1,
-      self: { foo: "hello", bar: 1 },
-    });
-  });
-
-  it("encodes object with circular reference", () => {
-    class TestClass {
-      foo: string;
-      bar: number;
-      self: TestClass;
-      constructor(foo: string, bar: number) {
-        this.foo = foo;
-        this.bar = bar;
-        this.self = this;
-      }
-    }
-    const testObject1 = new TestClass("hello", 1);
-    const testObject2 = new TestClass("world", 2);
-    testObject1.self = testObject2;
-    testObject2.self = testObject1;
-    expect(https.encode(testObject1)).to.deep.equal({
-      foo: "hello",
-      bar: 1,
-      self: { foo: "world", bar: 2, self: { foo: "hello", bar: 1 } },
-    });
-    expect(https.encode(testObject2)).to.deep.equal({
-      foo: "world",
-      bar: 2,
-      self: { foo: "hello", bar: 1, self: { foo: "world", bar: 2 } },
-    });
-  });
-
-  it("encodes object with circular reference in nested object", () => {
-    class TestClass {
-      foo: string;
-      bar: number;
-      nested: {
-        self: TestClass;
-      };
-      constructor(foo: string, bar: number) {
-        this.foo = foo;
-        this.bar = bar;
-        this.nested = { self: this };
-      }
-    }
-    const testObject = new TestClass("hello", 1);
-    expect(https.encode(testObject)).to.deep.equal({
-      foo: "hello",
-      bar: 1,
-      nested: { self: { foo: "hello", bar: 1 } },
-    });
-  });
-
-  it("encodes object with circular reference without histical data", () => {
-    class TestClass {
-      foo: string;
-      bar: number;
-      self: TestClass | undefined;
-      constructor(foo: string, bar: number) {
-        this.foo = foo;
-        this.bar = bar;
-      }
-    }
-    const testObject = new TestClass("hello", 1);
-    expect(https.encode(testObject)).to.deep.equal({
-      foo: "hello",
-      bar: 1,
-    });
-    testObject.self = testObject;
-    expect(https.encode(testObject)).to.deep.equal({
-      foo: "hello",
-      bar: 1,
-      self: { foo: "hello", bar: 1 },
-    });
-    delete testObject.self;
-    expect(https.encode(testObject)).to.deep.equal({
-      foo: "hello",
-      bar: 1,
-    });
+    expect(()=>https.encode(testObject)).to.throw(`Data cannot be encoded in JSON: ${testObject}`);
   });
 
   it("encodes function as an empty object", () => {

src/common/providers/https.ts

@@ -396,7 +396,7 @@ const LONG_TYPE = "type.googleapis.com/google.protobuf.Int64Value";
 const UNSIGNED_LONG_TYPE = "type.googleapis.com/google.protobuf.UInt64Value";
 
 /** @hidden */
-const SELF_REFERENCE_WEAKMAP = new WeakMap<any, any>();
+const SELF_REFERENCE_WEAKSET = new WeakSet<object|((...args:any[])=>any)>();
 /**
  * Encodes arbitrary data in our special format for JSON.
  * This is exposed only for testing.
@@ -423,35 +423,38 @@ export function encode(data: unknown): any {
   if (Array.isArray(data)) {
     return data.map(encode);
   }
-  const isObjectOrFunction = typeof data === "object" || typeof data === "function";
-
-  if (!isObjectOrFunction) {
-    // If we got this far, the data is not encodable.
+  if(!isFunctionOrObject(data)||SELF_REFERENCE_WEAKSET.has(data)) {
     logger.error("Data cannot be encoded in JSON.", data);
     throw new Error(`Data cannot be encoded in JSON: ${data}`);
   }
-  // implementation of an hidden argument to keep track of objects that have been encoded
-  if (SELF_REFERENCE_WEAKMAP.has(data)) {
-    return { ...SELF_REFERENCE_WEAKMAP.get(data) }; // return a shallow copy of the object
-  }
-
+
   // Sadly we don't have Object.fromEntries in Node 10, so we can't use a single
   // list comprehension
   const obj: Record<string, any> = {};
 
-  SELF_REFERENCE_WEAKMAP.set(data, obj);
+  SELF_REFERENCE_WEAKSET.add(data);
 
   for (const [k, v] of Object.entries(data)) {
     obj[k] = encode(v);
   }
 
   // clean after recursive call -
   // we don't want to keep references to objects that are not part of the current object
-  SELF_REFERENCE_WEAKMAP.delete(data);
+  SELF_REFERENCE_WEAKSET.delete(data);
 
   return obj;
 }
 
+function isFunctionOrObject(data: unknown): data is object|((...args:any[])=>any) {
+  const isObjectOrFunction = typeof data === "object" || typeof data === "function";
+
+  if (!isObjectOrFunction) {
+    // If we got this far, the data is not encodable.
+    return false;
+  }
+  return true;
+}
+
 /**
  * Decodes our special format for JSON into native types.
  * This is exposed only for testing.