From 3f23d68e465cd6d1fdd025723796a003c064612b Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Thu, 16 May 2019 13:09:31 +0200 Subject: [PATCH 1/3] util: simplify inspection limit handling This simplifies the handling of objects that exceed 128mb. Instead of using a separate property to identify that all following inputs should only return their constructor name it'll just set the depth to -1. That has the almost the same behavior as before while providing a better output in some cases. The performance should be almost identical as well. --- lib/internal/util/inspect.js | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js index 850740d3c2bc43..2d5080bd6dd292 100644 --- a/lib/internal/util/inspect.js +++ b/lib/internal/util/inspect.js @@ -520,17 +520,12 @@ function formatValue(ctx, value, recurseTimes, typedArray) { // any proxy handlers. const proxy = getProxyDetails(value); if (proxy !== undefined) { - if (ctx.showProxy && ctx.stop === undefined) { + if (ctx.showProxy) { return formatProxy(ctx, proxy, recurseTimes); } value = proxy[0]; } - if (ctx.stop !== undefined) { - const name = getConstructorName(value, ctx) || value[Symbol.toStringTag]; - return ctx.stylize(`[${name || 'Object'}]`, 'special'); - } - // Provide a hook for user-specified inspect functions. // Check that value is an object with an inspect function on it. if (ctx.customInspect) { @@ -788,7 +783,7 @@ function formatRaw(ctx, value, recurseTimes, typedArray) { // This limit also makes sure that huge objects don't block the event loop // significantly. if (newLength > 2 ** 27) { - ctx.stop = true; + ctx.depth = -1; } return res; } From 2282b779b419ec0cd542d51f38a2ae44ece924ec Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Thu, 16 May 2019 13:25:59 +0200 Subject: [PATCH 2/3] util: unify constructor inspection in `util.inspect` This makes sure that an objects constructor name is always returned in a similar fashion instead of having different outputs depending on the object shape and the code path taken. --- lib/internal/util/inspect.js | 34 ++++++++++++++----------- test/parallel/test-util-inspect.js | 41 ++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 15 deletions(-) diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js index 2d5080bd6dd292..791c6653a7346a 100644 --- a/lib/internal/util/inspect.js +++ b/lib/internal/util/inspect.js @@ -418,8 +418,15 @@ function getKeys(value, showHidden) { return keys; } -function getCtxStyle(constructor, tag) { - return constructor || tag || 'Object'; +function getCtxStyle(value, constructor, tag) { + let fallback = ''; + if (constructor === null) { + fallback = internalGetConstructorName(value); + if (fallback === tag) { + fallback = 'Object'; + } + } + return getPrefix(constructor, tag, fallback); } function formatProxy(ctx, proxy, recurseTimes) { @@ -723,25 +730,21 @@ function formatRaw(ctx, value, recurseTimes, typedArray) { formatter = formatIterator; // Handle other regular objects again. } else { - let fallback = ''; - if (constructor === null) { - fallback = internalGetConstructorName(value); - if (fallback === tag) { - fallback = 'Object'; - } - } if (keys.length === 0) { if (isExternal(value)) return ctx.stylize('[External]', 'special'); - return `${getPrefix(constructor, tag, fallback)}{}`; + return `${getCtxStyle(value, constructor, tag)}{}`; } - braces[0] = `${getPrefix(constructor, tag, fallback)}{`; + braces[0] = `${getCtxStyle(value, constructor, tag)}{`; } } } if (recurseTimes > ctx.depth && ctx.depth !== null) { - return ctx.stylize(`[${getCtxStyle(constructor, tag)}]`, 'special'); + let constructorName = getCtxStyle(value, constructor, tag).slice(0, -1); + if (constructor !== null) + constructorName = `[${constructorName}]`; + return ctx.stylize(constructorName, 'special'); } recurseTimes += 1; @@ -756,7 +759,8 @@ function formatRaw(ctx, value, recurseTimes, typedArray) { formatProperty(ctx, value, recurseTimes, keys[i], extrasType)); } } catch (err) { - return handleMaxCallStackSize(ctx, err, constructor, tag, indentationLvl); + const constructorName = getCtxStyle(value, constructor, tag).slice(0, -1); + return handleMaxCallStackSize(ctx, err, constructorName, indentationLvl); } ctx.seen.pop(); @@ -1016,12 +1020,12 @@ function groupArrayElements(ctx, output) { return output; } -function handleMaxCallStackSize(ctx, err, constructor, tag, indentationLvl) { +function handleMaxCallStackSize(ctx, err, constructorName, indentationLvl) { if (isStackOverflowError(err)) { ctx.seen.pop(); ctx.indentationLvl = indentationLvl; return ctx.stylize( - `[${getCtxStyle(constructor, tag)}: Inspection interrupted ` + + `[${constructorName}: Inspection interrupted ` + 'prematurely. Maximum call stack size exceeded.]', 'special' ); diff --git a/test/parallel/test-util-inspect.js b/test/parallel/test-util-inspect.js index 93b1d4439812f2..a1ce4903de0631 100644 --- a/test/parallel/test-util-inspect.js +++ b/test/parallel/test-util-inspect.js @@ -1094,6 +1094,47 @@ if (typeof Symbol !== 'undefined') { '[Set Iterator] { 1, ... 1 more item, extra: true }'); } +// Minimal inspection should still return as much information as possible about +// the constructor and Symbol.toStringTag. +{ + class Foo { + get [Symbol.toStringTag]() { + return 'ABC'; + } + } + const a = new Foo(); + assert.strictEqual(inspect(a, { depth: -1 }), 'Foo [ABC] {}'); + a.foo = true; + assert.strictEqual(inspect(a, { depth: -1 }), '[Foo [ABC]]'); + Object.defineProperty(a, Symbol.toStringTag, { + value: 'Foo', + configurable: true, + writable: true + }); + assert.strictEqual(inspect(a, { depth: -1 }), '[Foo]'); + delete a[Symbol.toStringTag]; + Object.setPrototypeOf(a, null); + assert.strictEqual(inspect(a, { depth: -1 }), '[Foo: null prototype]'); + delete a.foo; + assert.strictEqual(inspect(a, { depth: -1 }), '[Foo: null prototype] {}'); + Object.defineProperty(a, Symbol.toStringTag, { + value: 'ABC', + configurable: true + }); + assert.strictEqual( + inspect(a, { depth: -1 }), + '[Foo: null prototype] [ABC] {}' + ); + Object.defineProperty(a, Symbol.toStringTag, { + value: 'Foo', + configurable: true + }); + assert.strictEqual( + inspect(a, { depth: -1 }), + '[Object: null prototype] [Foo] {}' + ); +} + // Test alignment of items in container. // Assumes that the first numeric character is the start of an item. { From 2deb076eff7c3c4c2d8ef5b3200cfa48259b1808 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Thu, 16 May 2019 13:27:52 +0200 Subject: [PATCH 3/3] util: remove outdated comment It is probably not necessary to visualize the `code` property as part of the name of an error since all extra properties will be visible anyway due to https://github.com/nodejs/node/pull/272431. --- lib/internal/util/inspect.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js index 791c6653a7346a..07757f3fe0b6ad 100644 --- a/lib/internal/util/inspect.js +++ b/lib/internal/util/inspect.js @@ -868,7 +868,6 @@ function getFunctionBase(value, constructor, tag) { } function formatError(err, constructor, tag, ctx) { - // TODO(BridgeAR): Always show the error code if present. let stack = err.stack || ErrorPrototype.toString(err); // A stack trace may contain arbitrary data. Only manipulate the output