Pause reason incorrect for promise rejection
Reported by
roblour...@gmail.com,
Aug 15 2017
|
||||
Issue description
UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.90 Safari/537.36
Steps to reproduce the problem:
In the debug protocol, the "Debugger.paused" event has a "reason", which can be "exception", "promiseRejection", among other values. In some cases when breaking on a promise rejection, it's incorrectly set to "exception".
For example, given this code:
let x = Promise.reject(new Error('x'));
let y = new Promise((_, reject) => {
reject(new Error('error'));
}).catch(e => {
});
let z = Promise.resolve().then(() => {
return Promise.reject(new Error('sldk'));
}).catch(e => {
});
What is the expected behavior?
It should be "promiseRejection" for all of these cases.
What went wrong?
"reason" is set to "exception" for `x`, but "promiseRejection" for `y` and the Promise.reject inside `z`. This is surfaced in the Chrome devtools UI, see attached screenshots.
Did this work before? N/A
Chrome version: 62.0.3186.0 Channel: n/a
OS Version: OS X 10.12.4
Flash Version:
,
Aug 16 2017
In Node 7.5 with V8 5.4.500.48, it never pauses on the Promise.reject, even with "Pause on caught exceptions". In Node 7.6 with V8 5.5.372.40, it has the behavior reported above.
,
Aug 17 2017
Thanks for the report. I'm able to reproduce on 59.0.3071.115, 60.0.3112.0 and 62.0.3187.0. It looks like the first 'x' case used to not throw an exception in the past. The 'debug exception' seems intentionally introduced to enable pausing on rejected promises: https://bugs.chromium.org/p/chromium/issues/detail?id=393913 https://codereview.chromium.org/440773004 It sounds reasonable to have this 'x' case throw a "promiseRejection" instead of a generic OnException, to match the other cases with the same reason. case 'x' and 'z' call v8::internal::Runtime_PromiseRejectEventFromStack() case 'y' calls v8::internal::Runtime_DebugPromiseReject() kozy@, would you like to take a look?
,
Oct 16 2017
,
Sep 28
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/3f99afc93c9ba1ba5df19f123b93cc3079893c9b commit 3f99afc93c9ba1ba5df19f123b93cc3079893c9b Author: Alexey Kozyatinskiy <kozyatinskiy@chromium.org> Date: Fri Sep 28 15:53:34 2018 inspector: mark all pauses on promise rejection with proper reason Sometimes we do not have promise on stack, e.g. Promise.reject call, but we need to attribute this pause with promise rejection. TBR=yangguo@chromium.org Bug: chromium:755728 Cq-Include-Trybots: luci.chromium.try:linux_chromium_headless_rel;master.tryserver.blink:linux_trusty_blink_rel Change-Id: I03ca1e1cd6c21677f0a12ece626e2c8a1938437b Reviewed-on: https://chromium-review.googlesource.com/1249942 Reviewed-by: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org> Commit-Queue: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org> Cr-Commit-Position: refs/heads/master@{#56293} [modify] https://crrev.com/3f99afc93c9ba1ba5df19f123b93cc3079893c9b/src/debug/debug-interface.h [modify] https://crrev.com/3f99afc93c9ba1ba5df19f123b93cc3079893c9b/src/debug/debug.cc [modify] https://crrev.com/3f99afc93c9ba1ba5df19f123b93cc3079893c9b/src/debug/debug.h [modify] https://crrev.com/3f99afc93c9ba1ba5df19f123b93cc3079893c9b/src/inspector/v8-debugger-agent-impl.cc [modify] https://crrev.com/3f99afc93c9ba1ba5df19f123b93cc3079893c9b/src/inspector/v8-debugger-agent-impl.h [modify] https://crrev.com/3f99afc93c9ba1ba5df19f123b93cc3079893c9b/src/inspector/v8-debugger.cc [modify] https://crrev.com/3f99afc93c9ba1ba5df19f123b93cc3079893c9b/src/inspector/v8-debugger.h [modify] https://crrev.com/3f99afc93c9ba1ba5df19f123b93cc3079893c9b/test/cctest/test-debug.cc [add] https://crrev.com/3f99afc93c9ba1ba5df19f123b93cc3079893c9b/test/inspector/debugger/pause-on-promise-rejections-expected.txt [add] https://crrev.com/3f99afc93c9ba1ba5df19f123b93cc3079893c9b/test/inspector/debugger/pause-on-promise-rejections.js
,
Sep 28
|
||||
►
Sign in to add a comment |
||||
Comment 1 Deleted