Gather console.warn, console.error messages

[warrengm]

Add gatherer for user console messages.

Feature request summary

I'm interested in inspecting console.warn, console.error etc. messages in a custom plugin audit and I mistakenly thought that the ConsoleMessages artifact included these messages but discovered that it does not. (It only includes warnings about violations and deprecated APIs)

I can see a few possible solutions:

1. Collect these events in the existing ConsoleMessages gatherer using Runtime.consoleAPICalled events
2. Implement an entirely new gather with just console API events
3. Just stick the logs in artifacts.devtoolsLogs without a new gatherer

I don't think (1) is a good solution due to differences in the event interfaces between Log.entryAdded and Runtime.consoleAPICalled. My main consideration is to avoid increasing the LHR size with too many console messages on some sites. We can mitigate this issue by omitting the custom gatherer from the main config (when no audits require it), to omit console.log events from the output (as I'm only interested in warnings/errors really), or omitting duplicate log messages. How does that sound? Any other considerations that I might have missed?

I'd be happy to work on this myself. Personally I'd lean toward implementing (2) but omit console.log events to reduce size (or only include error, warning, and assert events).

What is the motivation or use case for changing this?

My motivation is for some custom audits in our plugin.

How is this beneficial to Lighthouse?

I can see this benefitting lighthouse core if we have audits for script errors, similar to the user timings audits.

[patrickhulce]

Thanks for filing @warrengm!

My primary concern with option 2 is confusion when there's another gatherer named ConsoleMessages, as you point out one would definitely expect it to include this sort of thing :)

Would you be willing to wait for a 7.0 release (ref #11207) to get this functionality? (timeline not decided, but presumably sometime in the next 2 months?) If so, then a more useful ConsoleMessages with modified interface is on the table and much more attractive to me than adding a third attempt at ConsoleMessages/RuntimeExceptions/ConsoleLogMessages 😅

[warrengm]

I definitely want to avoid confusion. If we keep ConsoleMessages as a separate gatherer then I'd favor naming it something else. But merging them as one gatherer makes sense to me too.

Do you mean to wait to make the change until after v7.0 is out or to wait until 7.0 for this change to be deployed (and depended up for plugins)?

[patrickhulce]

Do you mean to wait to make the change until after v7.0 is out or to wait until 7.0 for this change to be deployed (and depended up for plugins)?

I mean waiting to do this work so that the first gatherer that has this information is released as part of 7.0 (and it doesn't release under a 6.x version). Plugins that need it would have to depend on 7.0 or later.

[warrengm]

Sure, that sounds good. Is v7 the next release or will there be another 6.x release before then? If v7 is the next release we should be able to begin work as soon as we agree on a design, as I understand.

[patrickhulce]

Is v7 the next release or will there be another 6.x release before then?

I'm guessing we'll do at least one more 6.x release before we start making breaking changes for 7.0, but I don't think there's been a decision yet.

[warrengm]

Hi Patrick,

SG. For next steps, I can look into design of a new gatherer that would encapsulate the consoleAPICalled events and the existing ConsoleMessages and RuntimeExceptions gatherer. But we can block merging in the new gatherer until 7.0 is ready to cut.

If that approach sounds good, I imagine the new gatherer's artifacts might look something like the following:

export interface PublicGathererArtifacts {
  ConsoleMessages: ConsoleMessage[];
}

interface ConsoleMessage {
   // Borrowed from EntryAddedEvent.source with a few console and exception added
  source: 'violation', ..., 'console', 'exception';
  // Union of EntryAddedEvent.level, RuntimeAPICalledEvent.type, and 'exception'?
  level: 'log', 'warning', ..., 'exception', 'other', ...;
  // Text needs to be processed from args on Runtime.consoleAPICalledEvent
  text: string;
  timestamp: number;
  stackTrace?: Crdp.Runtime.StackTrace;
  url?: string;
  event: Crdp.RuntimeAPICalledEvent | Crdp.Runtime.ExceptionThrownEvent | Crdp.EntryAddedEvent;
}

How does that look? I expect those main fields are of interest to most consumers. Let's discuss the details after agreeing on a high-level sketch.

[patrickhulce]

That sounds good to me at a high-level 👍

The only thing I'd want to ensure is for this to still be inline with #11313. That means everything is still arbitrary timespan compatible and doesn't rely on page navigations. The only immediate sticking point I can think of is resolution of the text might be dependent on the current navigation context, which is tricky, but not insurmountable and more of a detail really.

[brendankenny]

How would remoteObjects work with consoleAPICalled? I assume in DevTools they aren't automatically fetched over the protocol until the user interacts with them in the console, so would we proactively fetch them to construct the text field in your proposed artifact, @warrengm? We may end up with very very large artifacts in that case.

Are you after specific types of warned/error messages? Maybe this gatherer could limit not only console.log calls but also some types of args for the other console methods?

[patrickhulce]

so would we proactively fetch them to construct the text field in your proposed artifact

I assumed we would only resolve one layer or two tops similar to how logs in node are elided. Given the liberty we've taken with other element truncation, I don't anticipate this being that large.

but a good issue to iron out if others didn't have this expectation :) @warrengm did you have expectations for very large object serialization support here?

[brendankenny]

Yeah, I was just curious and thinking about what needs to be supported and what we can cut off. What's important to get in for near-term use cases, does JSON still need to be valid JSON, what do we do with things like logged DOM elements and circular references, etc.

[warrengm]

Regarding FraggleRock, I'd have to check the time origin and run some tests for CRDP timestamps, but that doesn't sound like an insurmountable problem.

Personally, I'm only interested in text logs for console.warn and console.error calls--but I don't want to overly fit to my use case. Limiting the log types seems like a reasonable trade-off for. (Plus some sites might emit thousands of console.log messages but I think warn/error messages are less common.) We can also trim the artifacts size by, say, only storing the ObjectPreview for non-primitive types. Do you have any strong feelings for what should be kept vs. omitted?

I'll do some more testing on my side and see what works well. I haven't played around with console API events much yet

[warrengm]

We could rename the gatherer to something along the lines of ConsoleWarningsAndErrors for clarity

[connorjclark]

I like just Console, but ConsoleMessages seems fine even after refactor. Although a name change might be ideal if breaking.

[paulirish]

@warrengm we have our cut for v7 coming up, and this would be a breaking change (as the public artifact shape changes). do you have time to work on this in the next 2 weeks?

also, we discussed a bit more and are happy to collect very basic details. You said you only wanted the strings, and not resolved object ids, etc.. that seems great to us. If we ever want more comprehensive data, we can add it later. KISS for now. 💋

[warrengm]

I'll keep it to basic warnings and errors for now. I plan to get started on this now and will have the first draft up soon!