-
Notifications
You must be signed in to change notification settings - Fork 42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Include stacktrace into log messages #807
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we are fine with the extra traffic, as long as the user is opting-in for the log messages.
@@ -10854,7 +10854,7 @@ Define the following [=console steps=] with |method|, |args|, and | |||
|
|||
1. Let |source| be the result of [=get the source=] given [=current Realm Record=]. | |||
|
|||
1. If |method| is "<code>assert</code>", "<code>error</code>", | |||
1. If |method| is "<code>assert</code>", "<code>error</code>", "<code>log</code>", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we perhaps do it for all methods?
The Browser Testing and Tools Working Group just discussed The full IRC log of that discussion<AutomatedTester> topic: Include stacktrace into log messages<AutomatedTester> github: https://github.com//pull/807 <orkon> q+ <AutomatedTester> ack next <AutomatedTester> orkon: this is a PR/issue about stacktrace not being part of the log messages <AutomatedTester> ... this was originally excluded but they have come up with a use case to ignore log messages if it comes from a "third party script". <whimboo> q+ <AutomatedTester> ... I am now wondering if we should include stacktraces to all console logs? <AutomatedTester> ack next <jgraham> q+ <sadym> q+ <AutomatedTester> whimboo: I think it would be fine to add it. I think it would add it to console logging? I wonder if we want to all people to opt into this extra info as it could add a lot of info <AutomatedTester> ack next <AutomatedTester> jgraham: I was going to say something similar. I don't know if it is expensive to compute a stacktrace as it could be expensive <AutomatedTester> ... this could lead into the subscription discussion later <AutomatedTester> ... I think it would be good to have a way of config if you don't want debug messages you can exclude them and do we want to opt into stacktraces. It merits a discussion <AutomatedTester> ack next <AutomatedTester> sadym: I didn't mean that we add configuration to the subscription. I don't want to send them by default, just if the user subscribes it <jdescottes> q+ <AutomatedTester> ... and users can limit the info being passed back with preload script <AutomatedTester> ack next <jgraham> q+ <AutomatedTester> jdescottes: I think that stacktraces are there for most things, have a way to add it already and support adding it in this scenario <AutomatedTester> ack next <AutomatedTester> jgraham: Let's add it by default and if we need filtering we can add that at a later stage <whimboo> q+ <AutomatedTester> ack next <orkon> q+ <AutomatedTester> whimboo: could someone submit a PR that updates and adds stacktrace <AutomatedTester> ... or I can try do it next week <AutomatedTester> ack next <AutomatedTester> orkon: this is in the console spec already <AutomatedTester> whimboo: we don't have this in firefox yet so I think we may need the spec changes <AutomatedTester> q? <whimboo> see also https://github.com/whatwg/console/issues/203 <AutomatedTester> whimboo: I have put a link to the issue above <AutomatedTester> RRSAgent, make minutes <AutomatedTester> q? <RRSAgent> I have made the request to generate https://www.w3.org/2024/11/13-webdriver-minutes.html AutomatedTester <AutomatedTester> sadym: we already send stacktrace for logEntry |
As mentioned by @whimboo during the meeting, in Firefox we indeed only have stacktraces available for a few console message types. For |
This would align behavior in Chrome and the spec. Related discussions: - puppeteer/puppeteer#13264 - whatwg/console#203
de6782b
to
a86f3e6
Compare
So I think this spec change does not depend on the behavior of the main Console spec because we are defining WebDriver BiDi-specific steps here. It should be possible to only compute stacktraces if there is a WebDriver BiDi connection and report them as WebDriver BiDi events without necessarily affecting the behavior in other instances of console use. |
This would align behavior in Chrome and the spec and support the following use case:
As a user I want to filter out console messages from third party libraries. For this, I could use the URLs in the stack trace.
Related discussions:
Low-priority and we can discuss later.
Preview | Diff