chore: serialize circular objects (#14045)

packages/playwright-core/src/client/jsHandle.ts

@@ -84,7 +84,7 @@ export function serializeArgument(arg: any): channels.SerializedArgument {
     if (value instanceof JSHandle)
       return { h: pushHandle(value._channel) };
     return { fallThrough: value };
-  }, new Set());
+  });
   return { value, handles };
 }
 

packages/playwright-core/src/protocol/channels.ts

@@ -173,6 +173,8 @@ export type SerializedValue = {
     v: SerializedValue,
   }[],
   h?: number,
+  id?: number,
+  ref?: number,
 };
 
 export type SerializedArgument = {

packages/playwright-core/src/protocol/protocol.yml

@@ -85,6 +85,10 @@ SerializedValue:
           v: SerializedValue
     # An index in the handles array from SerializedArgument.
     h: number?
+    # Index of the object in value-type for circular reference resolution.
+    id: number?
+    # Ref to the object in value-type for circular reference resolution.
+    ref: number?
 
 
 # Represents a value with handle references.

packages/playwright-core/src/protocol/serializers.ts

@@ -20,7 +20,7 @@ import type { SerializedError, SerializedValue } from './channels';
 export function serializeError(e: any): SerializedError {
   if (isError(e))
     return { error: { message: e.message, stack: e.stack, name: e.name } };
-  return { value: serializeValue(e, value => ({ fallThrough: value }), new Set()) };
+  return { value: serializeValue(e, value => ({ fallThrough: value })) };
 }
 
 export function parseError(error: SerializedError): Error {
@@ -41,6 +41,12 @@ export function parseError(error: SerializedError): Error {
 }
 
 export function parseSerializedValue(value: SerializedValue, handles: any[] | undefined): any {
+  return innerParseSerializedValue(value, handles, new Map());
+}
+
+function innerParseSerializedValue(value: SerializedValue, handles: any[] | undefined, refs: Map<number, object>): any {
+  if (value.ref !== undefined)
+    return refs.get(value.ref);
   if (value.n !== undefined)
     return value.n;
   if (value.s !== undefined)
@@ -65,12 +71,19 @@ export function parseSerializedValue(value: SerializedValue, handles: any[] | un
     return new Date(value.d);
   if (value.r !== undefined)
     return new RegExp(value.r.p, value.r.f);
-  if (value.a !== undefined)
-    return value.a.map((a: any) => parseSerializedValue(a, handles));
+
+  if (value.a !== undefined) {
+    const result: any[] = [];
+    refs.set(value.id!, result);
+    for (const v of value.a)
+      result.push(innerParseSerializedValue(v, handles, refs));
+    return result;
+  }
   if (value.o !== undefined) {
     const result: any = {};
+    refs.set(value.id!, result);
     for (const { k, v } of value.o)
-      result[k] = parseSerializedValue(v, handles);
+      result[k] = innerParseSerializedValue(v, handles, refs);
     return result;
   }
   if (value.h !== undefined) {
@@ -82,15 +95,22 @@ export function parseSerializedValue(value: SerializedValue, handles: any[] | un
 }
 
 export type HandleOrValue = { h: number } | { fallThrough: any };
-export function serializeValue(value: any, handleSerializer: (value: any) => HandleOrValue, visited: Set<any>): SerializedValue {
+type VisitorInfo = {
+  visited: Map<object, number>;
+  lastId: number;
+};
+
+export function serializeValue(value: any, handleSerializer: (value: any) => HandleOrValue): SerializedValue {
+  return innerSerializeValue(value, handleSerializer, { lastId: 0, visited: new Map() });
+}
+
+function innerSerializeValue(value: any, handleSerializer: (value: any) => HandleOrValue, visitorInfo: VisitorInfo): SerializedValue {
   const handle = handleSerializer(value);
   if ('fallThrough' in handle)
     value = handle.fallThrough;
   else
     return handle;
 
-  if (visited.has(value))
-    throw new Error('Argument is a circular structure');
   if (typeof value === 'symbol')
     return { v: 'undefined' };
   if (Object.is(value, undefined))
@@ -123,21 +143,26 @@ export function serializeValue(value: any, handleSerializer: (value: any) => Han
     return { d: value.toJSON() };
   if (isRegExp(value))
     return { r: { p: value.source, f: value.flags } };
+
+  const id = visitorInfo.visited.get(value);
+  if (id)
+    return { ref: id };
+
   if (Array.isArray(value)) {
     const a = [];
-    visited.add(value);
+    const id = ++visitorInfo.lastId;
+    visitorInfo.visited.set(value, id);
     for (let i = 0; i < value.length; ++i)
-      a.push(serializeValue(value[i], handleSerializer, visited));
-    visited.delete(value);
-    return { a };
+      a.push(innerSerializeValue(value[i], handleSerializer, visitorInfo));
+    return { a, id };
   }
   if (typeof value === 'object') {
     const o: { k: string, v: SerializedValue }[] = [];
-    visited.add(value);
+    const id = ++visitorInfo.lastId;
+    visitorInfo.visited.set(value, id);
     for (const name of Object.keys(value))
-      o.push({ k: name, v: serializeValue(value[name], handleSerializer, visited) });
-    visited.delete(value);
-    return { o };
+      o.push({ k: name, v: innerSerializeValue(value[name], handleSerializer, visitorInfo) });
+    return { o, id };
   }
   throw new Error('Unexpected value');
 }

packages/playwright-core/src/protocol/validator.ts

@@ -72,6 +72,8 @@ export function createScheme(tChannel: (name: string) => Validator): Scheme {
       v: tType('SerializedValue'),
     }))),
     h: tOptional(tNumber),
+    id: tOptional(tNumber),
+    ref: tOptional(tNumber),
   });
   scheme.SerializedArgument = tObject({
     value: tType('SerializedValue'),

packages/playwright-core/src/server/dispatchers/jsHandleDispatcher.ts

@@ -74,5 +74,5 @@ export function parseValue(v: channels.SerializedValue): any {
 }
 
 export function serializeResult(arg: any): channels.SerializedValue {
-  return serializeValue(arg, value => ({ fallThrough: value }), new Set());
+  return serializeValue(arg, value => ({ fallThrough: value }));
 }

packages/playwright-core/src/server/isomorphic/utilityScriptSerializers.ts

@@ -19,13 +19,19 @@ export type SerializedValue =
     { v: 'null' | 'undefined' | 'NaN' | 'Infinity' | '-Infinity' | '-0' } |
     { d: string } |
     { r: { p: string, f: string} } |
-    { a: SerializedValue[] } |
-    { o: { k: string, v: SerializedValue }[] } |
+    { a: SerializedValue[], id: number } |
+    { o: { k: string, v: SerializedValue }[], id: number } |
+    { ref: number } |
     { h: number };
 
 export type HandleOrValue = { h: number } | { fallThrough: any };
 
-export function source(aliasComplexAndCircularObjects: boolean = false) {
+type VisitorInfo = {
+  visited: Map<object, number>;
+  lastId: number;
+};
+
+export function source() {
 
   function isRegExp(obj: any): obj is RegExp {
     return obj instanceof RegExp || Object.prototype.toString.call(obj) === '[object RegExp]';
@@ -39,10 +45,12 @@ export function source(aliasComplexAndCircularObjects: boolean = false) {
     return obj instanceof Error || (obj && obj.__proto__ && obj.__proto__.name === 'Error');
   }
 
-  function parseEvaluationResultValue(value: SerializedValue, handles: any[] = []): any {
+  function parseEvaluationResultValue(value: SerializedValue, handles: any[] = [], refs: Map<number, object> = new Map()): any {
     if (Object.is(value, undefined))
       return undefined;
     if (typeof value === 'object' && value) {
+      if ('ref' in value)
+        return refs.get(value.ref);
       if ('v' in value) {
         if (value.v === 'undefined')
           return undefined;
@@ -62,12 +70,18 @@ export function source(aliasComplexAndCircularObjects: boolean = false) {
         return new Date(value.d);
       if ('r' in value)
         return new RegExp(value.r.p, value.r.f);
-      if ('a' in value)
-        return value.a.map((a: any) => parseEvaluationResultValue(a, handles));
+      if ('a' in value) {
+        const result: any[] = [];
+        refs.set(value.id, result);
+        for (const a of value.a)
+          result.push(parseEvaluationResultValue(a, handles, refs));
+        return result;
+      }
       if ('o' in value) {
         const result: any = {};
+        refs.set(value.id, result);
         for (const { k, v } of value.o)
-          result[k] = parseEvaluationResultValue(v, handles);
+          result[k] = parseEvaluationResultValue(v, handles, refs);
         return result;
       }
       if ('h' in value)
@@ -77,21 +91,10 @@ export function source(aliasComplexAndCircularObjects: boolean = false) {
   }
 
   function serializeAsCallArgument(value: any, handleSerializer: (value: any) => HandleOrValue): SerializedValue {
-    return serialize(value, handleSerializer, new Set());
+    return serialize(value, handleSerializer, { visited: new Map(), lastId: 0 });
   }
 
-  function serialize(value: any, handleSerializer: (value: any) => HandleOrValue, visited: Set<any>): SerializedValue {
-    if (!aliasComplexAndCircularObjects)
-      return innerSerialize(value, handleSerializer, visited);
-    try {
-      const alias = serializeComplexObjectAsAlias(value);
-      return alias || innerSerialize(value, handleSerializer, visited);
-    } catch (error) {
-      return error.stack;
-    }
-  }
-
-  function serializeComplexObjectAsAlias(value: any): string | undefined {
+  function serialize(value: any, handleSerializer: (value: any) => HandleOrValue, visitorInfo: VisitorInfo): SerializedValue {
     if (value && typeof value === 'object') {
       if (globalThis.Window && value instanceof globalThis.Window)
         return 'ref: <Window>';
@@ -100,22 +103,16 @@ export function source(aliasComplexAndCircularObjects: boolean = false) {
       if (globalThis.Node && value instanceof globalThis.Node)
         return 'ref: <Node>';
     }
+    return innerSerialize(value, handleSerializer, visitorInfo);
   }
 
-  function innerSerialize(value: any, handleSerializer: (value: any) => HandleOrValue, visited: Set<any>): SerializedValue {
+  function innerSerialize(value: any, handleSerializer: (value: any) => HandleOrValue, visitorInfo: VisitorInfo): SerializedValue {
     const result = handleSerializer(value);
     if ('fallThrough' in result)
       value = result.fallThrough;
     else
       return result;
 
-    if (visited.has(value)) {
-      if (aliasComplexAndCircularObjects) {
-        const alias = serializeComplexObjectAsAlias(value);
-        return alias || '[Circular Ref]';
-      }
-      throw new Error('Argument is a circular structure');
-    }
     if (typeof value === 'symbol')
       return { v: 'undefined' };
     if (Object.is(value, undefined))
@@ -151,18 +148,23 @@ export function source(aliasComplexAndCircularObjects: boolean = false) {
     if (isRegExp(value))
       return { r: { p: value.source, f: value.flags } };
 
+    const id = visitorInfo.visited.get(value);
+    if (id)
+      return { ref: id };
+
     if (Array.isArray(value)) {
       const a = [];
-      visited.add(value);
+      const id = ++visitorInfo.lastId;
+      visitorInfo.visited.set(value, id);
       for (let i = 0; i < value.length; ++i)
-        a.push(serialize(value[i], handleSerializer, visited));
-      visited.delete(value);
-      return { a };
+        a.push(serialize(value[i], handleSerializer, visitorInfo));
+      return { a, id };
     }
 
     if (typeof value === 'object') {
       const o: { k: string, v: SerializedValue }[] = [];
-      visited.add(value);
+      const id = ++visitorInfo.lastId;
+      visitorInfo.visited.set(value, id);
       for (const name of Object.keys(value)) {
         let item;
         try {
@@ -171,12 +173,11 @@ export function source(aliasComplexAndCircularObjects: boolean = false) {
           continue;  // native bindings will throw sometimes
         }
         if (name === 'toJSON' && typeof item === 'function')
-          o.push({ k: name, v: { o: [] } });
+          o.push({ k: name, v: { o: [], id: 0 } });
         else
-          o.push({ k: name, v: serialize(item, handleSerializer, visited) });
+          o.push({ k: name, v: serialize(item, handleSerializer, visitorInfo) });
       }
-      visited.delete(value);
-      return { o };
+      return { o, id };
     }
   }
 

packages/playwright-core/src/server/page.ts

@@ -729,7 +729,7 @@ export class PageBinding {
   constructor(name: string, playwrightFunction: frames.FunctionWithSource, needsHandle: boolean) {
     this.name = name;
     this.playwrightFunction = playwrightFunction;
-    this.source = `(${addPageBinding.toString()})(${JSON.stringify(name)}, ${needsHandle}, (${source})(true))`;
+    this.source = `(${addPageBinding.toString()})(${JSON.stringify(name)}, ${needsHandle}, (${source})())`;
     this.needsHandle = needsHandle;
   }
 
@@ -743,7 +743,7 @@ export class PageBinding {
         const handle = await context.evaluateHandle(takeHandle, { name, seq }).catch(e => null);
         result = await binding.playwrightFunction({ frame: context.frame, page, context: page._browserContext }, handle);
       } else {
-        const args = serializedArgs!.map(a => parseEvaluationResultValue(a, []));
+        const args = serializedArgs!.map(a => parseEvaluationResultValue(a));
         result = await binding.playwrightFunction({ frame: context.frame, page, context: page._browserContext }, ...args);
       }
       context.evaluate(deliverResult, { name, seq, result }).catch(e => debugLogger.log('error', e));

tests/page/jshandle-json-value.spec.ts

@@ -29,9 +29,9 @@ it('should work with dates', async ({ page }) => {
   expect(date.toJSON()).toBe('2017-09-26T00:00:00.000Z');
 });
 
-it('should throw for circular objects', async ({ page }) => {
-  const windowHandle = await page.evaluateHandle('window');
-  let error = null;
-  await windowHandle.jsonValue().catch(e => error = e);
-  expect(error.message).toContain('Argument is a circular structure');
+it('should handle circular objects', async ({ page }) => {
+  const handle = await page.evaluateHandle('const a = {}; a.b = a; a');
+  const a: any = {};
+  a.b = a;
+  expect(await handle.jsonValue()).toEqual(a);
 });

tests/page/page-evaluate-handle.spec.ts

@@ -63,13 +63,6 @@ it('should accept multiple nested handles', async ({ page }) => {
   });
 });
 
-it('should throw for circular objects', async ({ page }) => {
-  const a = { x: 1 };
-  a['y'] = a;
-  const error = await page.evaluate(x => x, a).catch(e => e);
-  expect(error.message).toContain('Argument is a circular structure');
-});
-
 it('should accept same handle multiple times', async ({ page }) => {
   const foo = await page.evaluateHandle(() => 1);
   expect(await page.evaluate(x => x, { foo, bar: [foo], baz: { foo } })).toEqual({ foo: 1, bar: [1], baz: { foo: 1 } });

tests/page/page-evaluate.spec.ts

@@ -331,17 +331,23 @@ it('should properly serialize null fields', async ({ page }) => {
 });
 
 it('should return undefined for non-serializable objects', async ({ page }) => {
-  expect(await page.evaluate(() => window)).toBe(undefined);
+  expect(await page.evaluate(() => function() {})).toBe(undefined);
 });
 
-it('should fail for circular object', async ({ page }) => {
+it('should alias Window, Document and Node', async ({ page }) => {
+  const object = await page.evaluate('[window, document, document.body]');
+  expect(object).toEqual(['ref: <Window>', 'ref: <Document>', 'ref: <Node>']);
+});
+
+it('should work for circular object', async ({ page }) => {
   const result = await page.evaluate(() => {
     const a = {} as any;
-    const b = { a };
-    a.b = b;
+    a.b = a;
     return a;
   });
-  expect(result).toBe(undefined);
+  const a = {} as any;
+  a.b = a;
+  expect(result).toEqual(a);
 });
 
 it('should be able to throw a tricky error', async ({ page }) => {

tests/page/page-expose-function.spec.ts

@@ -292,11 +292,11 @@ it('should alias Window, Document and Node', async ({ page }) => {
   expect(object).toEqual(['ref: <Window>', 'ref: <Document>', 'ref: <Node>']);
 });
 
-it('should trim cycles', async ({ page }) => {
+it('should serialize cycles', async ({ page }) => {
   let object: any;
   await page.exposeBinding('log', (source, obj) => object = obj);
-  await page.evaluate('const a = { a: 1 }; a.a = a; window.log(a)');
-  expect(object).toEqual({
-    a: '[Circular Ref]',
-  });
+  await page.evaluate('const a = {}; a.b = a; window.log(a)');
+  const a: any = {};
+  a.b = a;
+  expect(object).toEqual(a);
 });

utils/build/build.js

@@ -239,6 +239,7 @@ onChanges.push({
   committed: false,
   inputs: [
     'packages/playwright-core/src/server/injected/**',
+    'packages/playwright-core/src/server/isomorphic/**',
     'utils/generate_injected.js',
   ],
   script: 'utils/generate_injected.js',