Stacktraces are only generated for the first 50 "throw" usage in a realm
Categories
(Remote Protocol :: WebDriver BiDi, defect, P2)
Tracking
(firefox115 fixed)
Tracking | Status | |
---|---|---|
firefox115 | --- | fixed |
People
(Reporter: jdescottes, Assigned: jdescottes)
References
Details
(Whiteboard: [webdriver:m7][webdriver:relnote])
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
Seen while working on Bug 1770754, we stop getting stacktraces after 50 errors have been thrown on a specific window global. This only impacts stacktraces which are normally not observable from JS code, ie uncaught exceptions using throw
.
So in practice, executing a script.evaluate
command with an expression which throws (eg a.b.c.d
with a not defined in the current global), will correctly have a stacktrace even when executed more than 50 times. But with an expression which uses the throw
keyword (eg throw new Error()
) the stacktrace will be missing after 50 executions.
This comes from the following logic:
https://searchfox.org/mozilla-central/rev/b1e5f2c7c96be36974262551978d54f457db2cae/js/src/vm/Realm.cpp#589-606
If the realm is a debuggee, the restriction is lifted so that errors can be inspected if the console is opened.
The first question is if we should force the same behavior for Realms monitored by BiDi, or consider that the stacktrace is optional and is implementation specific.
If we decide that we always want the stacktrace for BiDi monitored Realm, we can either add a dedicated API (similar as to what was done for async stacktraces in Bug 1775207, maybe modifying and reusing that?) or switch from makeGlobalObjectReference
to addDebuggee
.
Given that we want BiDi to offer debugging features in the long run, we will probably need the same features as for devtools, so I'm not sure if we will be able to avoid using addDebuggee
forever. We might want to look at the performance impact of doing that in more details.
Comment 1•2 years ago
|
||
For the behavior differences that come from performance optimization/tuning (that is both this bug and async stack, where the restriction/optimization was based on the assumption that the info won't be necessary, which was not true for BiDi), in my opinion, it should be fine to add per-Realm option to lift the restriction,
so I'm in favor of adding dedicated API, and I can take this if the answer for the first question is "yes".
Assignee | ||
Updated•2 years ago
|
Comment 2•2 years ago
|
||
The severity field is not set for this bug.
:whimboo, could you have a look please?
For more information, please visit auto_nag documentation.
Assignee | ||
Comment 3•2 years ago
|
||
We should check how this impacts performance before enabling it, but otherwise this sounds fine.
Assignee | ||
Updated•2 years ago
|
Comment 4•2 years ago
|
||
Julian, is the performance check something easy that could be done? What specifically we would have to look out for?
Updated•1 years ago
|
Assignee | ||
Comment 5•1 years ago
|
||
I don't have a specific suggestion for a performance test. If we want to do it, it might be something like generating a lot of stacktraces and checking a profile?
Assignee | ||
Comment 6•1 years ago
|
||
Should file a dedicated bug for the platform change
Assignee | ||
Comment 8•1 years ago
|
||
Depends on D179243
Updated•1 years ago
|
Comment 10•1 year ago
|
||
bugherder |
Updated•1 year ago
|
Description
•