Stack traces rendered by Error.stack and by console differ wrt built-in functions. |
||
Issue descriptionWhen an Error object is created in V8, we capture a "simple" stack trace. We use that to format the Error.stack string. If DevTools is opened, we capture a "detailed" stack trace in addition. That stack trace is passed to the message listener callback and is used to render the stack trace in the console. For the simple stack trace, we include all frames except for functions with native script, but not marked as "native" (yes, naming is confusing...). See [0]. Functions that are visible to user script are marked as native. The reason we do filtering this way is because native functions can call helper functions that are just an implementation detail. For example String.prototype.replace may call ExpandReplacement. When it throws, we do want to include String.prototype.replace on the stack trace, but not ExpandReplacement. Both are compiled from native script, but only the former is marked as "native". For the detailed stack trace, we have a similar check [1] that is supposed to be in sync with [0]. But it's not. It only checks for the script type, and omits frames that have native scripts. That means that built-ins implemented in JS which are intended to show up on the stack trace actually don't, because of this check being out-of-sync. The result is that the detailed stack trace actually show less than the simple stack trace. The problem gets worse since we have a way to reconstruct the detailed stack trace from the simple stack trace for the case when DevTools is opened late. Being out-of-sync means that the stack trace rendered on the console depends on when DevTools was opened. Changing the current behavior of the detailed stack trace to be in sync with the simple stack trace would cause layout test failures. The frame for built-in functions should be included in the stack trace shown in the console because: - omitting the built-in function frame would misrepresent the stack. - the stack trace would be different depending on whether the built-in is called, or a polyfill of it. - Error.stack and the stack trace shown in the console would differ. - there is value in knowing which function called the function that throws, even if it's a built-in function. [0] https://cs.chromium.org/chromium/src/v8/src/isolate.cc?q=IsVisibleInStackTrace&sq=package:chromium&l=322&dr=CSs [1] https://cs.chromium.org/chromium/src/v8/src/frames.cc?q=IsValidFrame&sq=package:chromium&l=175&dr=CSs
,
Jun 29 2016
,
Jul 8 2016
I looked into disabling the mechanism of reconstructing detailed from simple stack traces as Yang described above. It results only in a single layout test failure: inspector/console/console-stack-overflow.html: --- chromium/src/out/Release/layout-test-results/inspector/console/console-stack-overflow-expected.txt +++ chromium/src/out/Release/layout-test-results/inspector/console/console-stack-overflow-actual.txt @@ -1,5 +1,5 @@ CONSOLE ERROR: line 6: Uncaught RangeError: Maximum call stack size exceeded Tests that when stack overflow exception happens when inspector is open the stack trace is correctly shown in console. -console-stack-overflow.html:6 Uncaught RangeError: Maximum call stack size exceededoverflow @ console-stack-overflow.html:6overflow @ console-stack-overflow.html:8overflow @ console-stack-overflow.html:8overflow @ console-stack-overflow.html:8overflow @ console-stack-overflow.html:8overflow @ console-stack-overflow.html:8overflow @ console-stack-overflow.html:8overflow @ console-stack-overflow.html:8overflow @ console-stack-overflow.html:8overflow @ console-stack-overflow.html:8 +console-stack-overflow.html:6 Uncaught RangeError: Maximum call stack size exceededoverflow @ console-stack-overflow.html:6overflow @ console-stack-overflow.html:8overflow @ console-stack-overflow.html:8overflow @ console-stack-overflow.html:8overflow @ console-stack-overflow.html:8overflow @ console-stack-overflow.html:8overflow @ console-stack-overflow.html:8overflow @ console-stack-overflow.html:8overflow @ console-stack-overflow.html:8overflow @ console-stack-overflow.html:8overflow @ console-stack-overflow.html:8overflow @ console-stack-overflow.html:8overflow @ console-stack-overflow.html:8overflow @ console-stack-overflow.html:8overflow @ console-stack-overflow.html:8overflow @ console-stack-overflow.html:8overflow @ console-stack-overflow.html:8overflow @ console-stack-overflow.html:8overflow @ console-stack-overflow.html:8overflow @ console-stack-overflow.html:8overflow @ console-stack-overflow.html:8overflow @ console-stack-overflow.html:8overflow @ console-stack-overflow.html:8overflow @ console-st What do the DevTools folks think about removing this feature entirely?
,
Jul 8 2016
Looks like the new stack trace is even longer, not "simpler"? I guess that works for us. Aleksey?
,
Jul 8 2016
It looks good!
,
Jul 12 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3a3061047bfecf33807356f76382e56b270e66c2 commit 3a3061047bfecf33807356f76382e56b270e66c2 Author: jgruber <jgruber@chromium.org> Date: Tue Jul 12 07:04:51 2016 Mark console-stack-overflow as NeedsManualRebaseline In preparation of disabling simple->detailed stack trace conversion in V8. BUG=624285 R=kozyatinskiy@chromium.org, yangguo@chromium.org Review-Url: https://codereview.chromium.org/2141523003 Cr-Commit-Position: refs/heads/master@{#404790} [modify] https://crrev.com/3a3061047bfecf33807356f76382e56b270e66c2/third_party/WebKit/LayoutTests/TestExpectations
,
Jul 18 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b5683a83ccd74dbad544ce2fc0bc1af910352aa3 commit b5683a83ccd74dbad544ce2fc0bc1af910352aa3 Author: jgruber <jgruber@chromium.org> Date: Mon Jul 18 14:44:11 2016 Update test expectations for inspector/console/console-stack-overflow BUG=624285 Review-Url: https://codereview.chromium.org/2147213003 Cr-Commit-Position: refs/heads/master@{#405987} [modify] https://crrev.com/b5683a83ccd74dbad544ce2fc0bc1af910352aa3/third_party/WebKit/LayoutTests/TestExpectations
,
Jul 18 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f7f66e7d92f73574bced45ad6bbb10162308d84f commit f7f66e7d92f73574bced45ad6bbb10162308d84f Author: Rebaseline Bot <blink-rebaseline-bot@chromium.org> Date: Mon Jul 18 16:24:00 2016 Auto-rebaseline for r405987 https://chromium.googlesource.com/chromium/src/+/b5683a83c BUG=624285 TBR=jgruber@chromium.org Review URL: https://codereview.chromium.org/2156233002 . Cr-Commit-Position: refs/heads/master@{#406012} [modify] https://crrev.com/f7f66e7d92f73574bced45ad6bbb10162308d84f/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/f7f66e7d92f73574bced45ad6bbb10162308d84f/third_party/WebKit/LayoutTests/inspector/console/console-stack-overflow-expected.txt [add] https://crrev.com/f7f66e7d92f73574bced45ad6bbb10162308d84f/third_party/WebKit/LayoutTests/platform/android/inspector/console/console-stack-overflow-expected.txt |
||
►
Sign in to add a comment |
||
Comment 1 by yangguo@chromium.org
, Jun 29 2016