From 9ca9b08d903515fa5b5fe01648e722d3c802b1d1 Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Fri, 27 Jan 2023 05:07:55 -0800 Subject: [PATCH] fix: better formatting for sparse arrays (#20379) Right now arrays preview yields all array elements. In case of a sparse array with a single element on index 10000000, this results in a large string that OOM Node.js. This patch changes pretty-printing. For example: ```ts // Given this array const a = []; a[10] = 1; // Before this patch, pretty printing will yield: "[,,,,,,,,1]" // With this patch, pretty printing yields: "[empty x 9, 1]" ``` The new array pretty-printing is equal to what Chrome DevTools do to render sparse arrays. Fixes #20347 --- .../src/server/chromium/crExecutionContext.ts | 8 ++----- .../playwright-core/src/server/javascript.ts | 24 +++++++++++++++++++ .../src/server/webkit/wkExecutionContext.ts | 8 ++----- tests/page/jshandle-to-string.spec.ts | 17 +++++++++++++ tests/page/page-event-console.spec.ts | 2 +- 5 files changed, 46 insertions(+), 13 deletions(-) diff --git a/packages/playwright-core/src/server/chromium/crExecutionContext.ts b/packages/playwright-core/src/server/chromium/crExecutionContext.ts index 51cd5aca6587a..d54505d2836ef 100644 --- a/packages/playwright-core/src/server/chromium/crExecutionContext.ts +++ b/packages/playwright-core/src/server/chromium/crExecutionContext.ts @@ -137,11 +137,7 @@ function renderPreview(object: Protocol.Runtime.RemoteObject): string | undefine tokens.push(`${name}: ${value}`); return `{${tokens.join(', ')}}`; } - if (object.subtype === 'array' && object.preview) { - const result = []; - for (const { name, value } of object.preview.properties) - result[+name] = value; - return '[' + String(result) + ']'; - } + if (object.subtype === 'array' && object.preview) + return js.sparseArrayToString(object.preview.properties); return object.description; } diff --git a/packages/playwright-core/src/server/javascript.ts b/packages/playwright-core/src/server/javascript.ts index 7c05906bada27..7ba6601f7042b 100644 --- a/packages/playwright-core/src/server/javascript.ts +++ b/packages/playwright-core/src/server/javascript.ts @@ -328,3 +328,27 @@ export class JavaScriptErrorInEvaluate extends Error { export function isJavaScriptErrorInEvaluate(error: Error) { return error instanceof JavaScriptErrorInEvaluate; } + +export function sparseArrayToString(entries: { name: string, value?: any }[]): string { + const arrayEntries = []; + for (const { name, value } of entries) { + const index = +name; + if (isNaN(index) || index < 0) + continue; + arrayEntries.push({ index, value }); + } + arrayEntries.sort((a, b) => a.index - b.index); + let lastIndex = -1; + const tokens = []; + for (const { index, value } of arrayEntries) { + const emptyItems = index - lastIndex - 1; + if (emptyItems === 1) + tokens.push(`empty`); + else if (emptyItems > 1) + tokens.push(`empty x ${emptyItems}`); + tokens.push(String(value)); + lastIndex = index; + } + + return '[' + tokens.join(', ') + ']'; +} diff --git a/packages/playwright-core/src/server/webkit/wkExecutionContext.ts b/packages/playwright-core/src/server/webkit/wkExecutionContext.ts index d1cd048e020a2..85c9847c1300a 100644 --- a/packages/playwright-core/src/server/webkit/wkExecutionContext.ts +++ b/packages/playwright-core/src/server/webkit/wkExecutionContext.ts @@ -142,11 +142,7 @@ function renderPreview(object: Protocol.Runtime.RemoteObject): string | undefine tokens.push(`${name}: ${value}`); return `{${tokens.join(', ')}}`; } - if (object.subtype === 'array' && object.preview) { - const result = []; - for (const { name, value } of object.preview.properties!) - result[+name] = value; - return '[' + String(result) + ']'; - } + if (object.subtype === 'array' && object.preview) + return js.sparseArrayToString(object.preview.properties!); return object.description; } diff --git a/tests/page/jshandle-to-string.spec.ts b/tests/page/jshandle-to-string.spec.ts index af0298bab8437..930899ae3d880 100644 --- a/tests/page/jshandle-to-string.spec.ts +++ b/tests/page/jshandle-to-string.spec.ts @@ -32,6 +32,23 @@ it('should work for complicated objects', async ({ page, browserName }) => { expect(aHandle.toString()).toBe('JSHandle@object'); }); +it('should beautifully render sparse arrays', async ({ page, browserName }) => { + const [msg] = await Promise.all([ + page.waitForEvent('console'), + page.evaluateHandle(() => { + const a = []; + a[1] = 1; + a[10] = 2; + a[100] = 3; + console.log(a); + }), + ]); + if (browserName === 'firefox') + expect(msg.text()).toBe('Array'); + else + expect(msg.text()).toBe('[empty, 1, empty x 8, 2, empty x 89, 3]'); +}); + it('should work for promises', async ({ page }) => { // wrap the promise in an object, otherwise we will await. const wrapperHandle = await page.evaluateHandle(() => ({ b: Promise.resolve(123) })); diff --git a/tests/page/page-event-console.spec.ts b/tests/page/page-event-console.spec.ts index ae4299484ffe4..7e9d1a9a60805 100644 --- a/tests/page/page-event-console.spec.ts +++ b/tests/page/page-event-console.spec.ts @@ -183,7 +183,7 @@ it('should use object previews for arrays and objects', async ({ page, browserNa await page.evaluate(() => console.log([1, 2, 3], { a: 1 }, window)); if (browserName !== 'firefox') - expect(text).toEqual('[1,2,3] {a: 1} Window'); + expect(text).toEqual('[1, 2, 3] {a: 1} Window'); else expect(text).toEqual('Array JSHandle@object JSHandle@object'); });