Add condition to Debugger.setPauseOnException
Reported by
thejames...@gmail.com,
Jul 10
|
||||||||||
Issue description
UserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/67.0.3396.99 Safari/537.36
Steps to reproduce the problem:
1. Add an event listener for error/unhandledrejection onto window and use event.preventDefault e.g.:
class Cancellation extends Error {}
window.addEventListener('unhandledrejection', event => {
if (event.object instanceof Cancellation) {
event.preventDefault()
}
})
2. Enable Pause on Uncaught Exceptions in devtools
3. Just throw an error e.g. in the example rejecting a Promise with a new Cancellation() will continue to cause the pause on uncaught exceptions to trigger
What is the expected behavior?
I would've expected that event.preventDefault() would make devtools treat it as a caught exception. Basically I wouldn't expect *any* exception that doesn't appear in the console to cause the pause on uncaught exceptions. (I would probably still expect them to cause pause on caught exceptions though).
What went wrong?
Devtools continues to pause on these exceptions that I would otherwise consider caught given that they're explicitly handled.
This has become rather annoying for pretty much exactly the example above, while using a CancelToken a lot like in the prex (https://github.com/rbuckton/prex) library this has become an issue with using the pause on caught exceptions as cancellation is sufficiently common it causes many false positives that it's not particularly useful in significant parts of some pages.
Did this work before? No
Chrome version: 69.0.3472.3 Channel: dev
OS Version:
Flash Version:
,
Jul 12
thejamesernator@ - Thanks for filing the issue...!! Could you please provide a sample test file/url to test the issue from TE-end. This will help us in triaging the issue further. Thanks...!!
,
Jul 12
Yeah here's a plunk with some buttons that throw some errors that are handled in `window.onerror`/`window.onunhandledrejection`, the text within should explain the expected outcomes when `Pause on Uncaught Exceptions` is currently enabled in the Sources tab. https://plnkr.co/edit/e1bKwAMrbJE5nICJci2p?p=preview
,
Jul 12
Thank you for providing more feedback. Adding the requester to the cc list. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 13
Tested the issue on Ubuntu 17.10 using chrome latest dev #69.0.3486.0. Attached a screen cast for reference. Following are the steps followed to reproduce the issue. ------------ 1. Navigated to url: https://plnkr.co/edit/e1bKwAMrbJE5nICJci2p?p=preview 2. Clicked on 1st two buttons i.e "Throw Cancellation" and "Reject promise with cancellation". Observed that no error was shown in the console. 3. Clicked on 2nd two buttons i.e "Throw Other error" and "Reject promise with other error". Observed that errors were shown in the console. thejamesernator@ - Could you please check the attached screen cast and please let us know if it is the issue mentioned in comment #0. Thanks...!!
,
Jul 13
No I'm meaning when Pause on Uncaught Exception is enabled it shouldn't continue to pause when the top two buttons are clicked because `event.preventDefault()` is used to handle the error. I attached an image to show what I mean should be enabled. When clicking the second two buttons the errors appear in the console normally so it's not surprising they cause the Pause on Uncaught exception to trigger. However for the top two buttons it doesn't appear in the console (as it's intentionally handled in both of the listeners) so I would not expect a pause on exception.
,
Jul 13
Thank you for providing more feedback. Adding the requester to the cc list. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 18
As per comment #6 we have tested this issue on Ubuntu 17.10 using chrome latest dev #69.0.3486.0. Steps: ------- 1. Launched chrome 2. Navigated to given URL ""https://plnkr.co/edit/e1bKwAMrbJE5nICJci2p?p=preview"" 3. opened Dev_Tools >> Sources >> Double clicked on ""e1bKwaMJE5nCJCi2p?p=preview"" 4. Enabled Pause on caught (II) As per screen shot on comment #6 5. Clicked on Throw Cancellation As we are observed Paused on exception as per screen-cast. @Reporter: Could you please review the attached screen-cast and confirm if anything being missed here. Thanks...!
,
Jul 19
Yes I can confirm that that is the unexpected behaviour, I would've expected it to not Pause as I've explicitly added a `event.preventDefault()` to say that I've seen and handled the error. I actually had a search through the source code to see how the pausing works, and I think solving this may not be particularly trivial as in order to get the Pause on Exception to work it inspects the entire Javascript stack to see if there's `catch ()` statement somewhere in the stack. If there is then it is treated as "caught" otherwise it gets treated as "uncaught" and it triggers the Pause on Exception handler. (https://cs.chromium.org/chromium/src/v8/src/isolate.cc?dr=C&g=0&l=1527) Because `window.onerror`/`window.onunhandledrejection` handlers are *not* part of the error stack Pause on Exception is unable to know that it might be caught using `event.preventDefault()` inside a `window.onerror` handler. This behaviour is actually noticeable in a completely different way, because of the way Pause on Exception works when throwing an error it will actually pause on the *closest* `throw` statement to where it eventually becomes an uncaught exception rather than where it originally was thrown from. This can be demonstrated by another Plunk: https://plnkr.co/edit/n3l8N3Bk6igMDmStM9LE?p=preview Given the fact this issue is essentially equivalent to determining if somewhere high up in the stack some arbitrary code will *rethrow* the error I don't think it's actually directly actionable. Alternative idea: Provide a way to mark an exception such that debuggers do not need to consider it when using features like Pause on (Uncaught) Exception. e.g. Maybe a special function in the devtools REPL like `ignoreExceptions(exception => exception instanceof Cancellation)` which Pause on Exception calls to check if it should bother to actually pause for that exception.
,
Jul 19
Thank you for providing more feedback. Adding the requester to the cc list. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 27
Able to reproduce the issue on reported chromium version 69.0.3472.3 also on latest chrome 70.0.3504.0 using Mac 10.13.5, Ubuntu 17.10 and Windows 10. Note: After refreshing the page we have seen the issue on M-60 Same behavior is seen on M60(60.0.3112.113) hence considering it as non-regression and marking it as Untriaged. Thanks!
,
Aug 18
Thank you for the report. I agree with the main assessment in c#9, that more data would need to be plumbed into V8 for it to be aware of "handling via preventDefault". Another alternative we could explore is to try to make "Never pause here" prevent DevTools from pausing on a line that has an uncaught exception of this form.
,
Sep 24
As was mentioned in #9: there is no trivial way to implement this feature since we make decision about pause or not pause at moment when exception is thrown. We can not continue code execution and at the same time we do not know will exception be prevented as default or not. I can see three different options here: - use existing "Never pause here", it works but it requires adding "Never pause here" to each line where exception is thrown, - adding condition to Debugger.setPauseOnExceptions, condition will take exception as argument and allow to check anything about this object, it will resolve this problem but from UI perspective: it is not clear where condition text box should be located, - "magic": pause and use callFunctionOn to rethrow exception with throwOnSideEffect flag to make an attempt to check that exception was prevented. Most realistic is second option.
,
Sep 27
|
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by krajshree@chromium.org
, Jul 11