feat: suppress devtools console logging

[clavin]

Background

Electron app developers often see console errors from the DevTools frontend when running from the command line, for example:

[15676:1226/144806.624:ERROR:CONSOLE:1] "Request Autofill.enable failed. {"code":-32601,"message":"'Autofill.enable' wasn't found"}", source: devtools://devtools/bundled/core/protocol_client/protocol_client.js (1)
[15676:1226/144806.624:ERROR:CONSOLE:1] "Request Autofill.setAddresses failed. {"code":-32601,"message":"'Autofill.setAddresses' wasn't found"}", source: devtools://devtools/bundled/core/protocol_client/protocol_client.js (1)

These originate upstream in Chromium:

1. The DevTools frontend logs failed requests for methods not implemented in Electron's Chromium build.
2. The log message is printed by content::LogConsoleMessage to the native console.

These messages are intended for Chromium/DevTools developers, but they predominantly reach app developers. The console is an impactful surface for dev UX, and routinely seeing unactionable "errors" degrades the experience. (To be clear, it isn't a high priority surface, though.)

Solution

Before content::LogConsoleMessage writes to the native console, the message passes through RenderFrameHostDelegate::DidAddMessageToConsole, which is plumbed to the corresponding WebContents delegate method. That hook can return true to indicate the message was handled and suppress the default logging path.

Electron's DevTools frontend is hosted in electron::InspectableWebContents. This change overrides the delegate DidAddMessageToConsole method to suppress the messages from the DevTools' WebContents.

In case we need an escape hatch to see these log messages again, they are reimplemented as verbose logs. Run with --vmodule=inspectable_web_contents=1 to print out the log messages again. The messages also print as usual in testing builds (which we run in CI).

Fixes #41614.

Checklist

• PR description included and stakeholders cc'd
• npm test passes
• relevant API documentation, tutorials, and examples are updated and follow the documentation style guide
• PR release notes describe the change in a way relevant to app developers, and are capitalized, punctuated, and past tense.

Release Notes

Notes: DevTools errors are no longer printed to console

[erickzhao]

API LGTM

[nikwen]

Thanks for the PR, @clavin!

I've thought about it some more. Would it make more sense to add stub implementations of Autofill.enable and Autofill.setAddresses instead which do nothing? I'm afraid that we (as maintainers) might miss important error messages in the future if we disable devtools logging by default. (Especially cases where certain parts of the devtools are actually broken due to a Chromium update.)

I don't have a strong opinion on this. If you and other maintainers think suppressing is better, I'm happy with merging this. But I thought this was worthwhile bringing this up.

[TheOneTheOnlyJJ]

... to add stub implementations of Autofill.enable and Autofill.setAddresses instead which do nothing? ...

From the little information I have, these 2 functions should serve as form autofill suggestions, right? I may be wrong on this, so please correct me if I am so.

I wonder why these are not implemented at all in the first place? Are there any problems with them and their functionality, or did the team just not get to them yet?

Could they be properly implemented, either as in Chromium or through an Electron-specific patch?

[jkleinsc]

API LGTM

[clavin]

Would it make more sense to add stub implementations of Autofill.enable and Autofill.setAddresses instead which do nothing?

That solution makes sense too! Tbh I haven't looked into what this would look like, nor why these are unimplemented in our build. I'm nervous that stubbing them may require patching, and I'm not convinced that this issue meets the threshold for on-going patch maintenance.

When the solution is this simple, addressing the root of the problem seems better than patching in stubs (if the problem comes up again in the future). Messages that are important to us as maintainers, but not to users, still fall into this trap:

These messages are intended for Chromium/DevTools developers, but they predominantly reach app developers. The console is an impactful surface for dev UX, and routinely seeing unactionable "errors" degrades the experience.

Your concern to not sweep these messages under the rug does persuade me; so, I added another carve-out: in testing builds (which run in CI and that most of us use for development), the messages will still be logged as normal. For release builds, the messages are suppressed (unless verbose logging is enabled).

Thx for the feedback @nikwen 😄

[nikwen]

@clavin I like the testing build logging solution! Thanks! 🙌

---

@TheOneTheOnlyJJ Regarding your question:

I wonder why these are not implemented at all in the first place?

I don't think we say that we officially support the Chrome Devtools Protocol, which those commands are part of. I haven't looked into what the commands do exactly and whether they even make sense in the context of Electron, so I can't comment on whether a patch for them would be accepted. But I don't think they'll do anything unless you use the Chrome Devtools Protocol.

[deepak1556]

That solution makes sense too! Tbh I haven't looked into what this would look like, nor why these are unimplemented in our build. I'm nervous that stubbing them may require patching, and I'm not convinced that this issue meets the threshold for on-going patch maintenance.

The missing apis are needed for autofill view in devtools

We don't support the autofill feature. The protocol handler for autofill apis https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/devtools/protocol/autofill_handler.cc;bpv=0;bpt=1 depends on the relevant components/autofill to be available.

If we want to fix this the proper way it has to be addressed in upstream devtools and it will not be a trivial patch we can maintain,

1. Make the AutofillModel https://source.chromium.org/chromium/chromium/src/+/main:third_party/devtools-frontend/src/front_end/core/sdk/AutofillModel.ts;l=171 not autostart which is what is causing the failing calls to enable and setaddress
2. Enable the model as part of the AutoFillView https://source.chromium.org/chromium/chromium/src/+/main:third_party/devtools-frontend/src/front_end/panels/autofill/AutofillView.ts;bpv=0;bpt=0 similar to other optional models like for example WebAuthnModel that get enabled based on the pane.

[deepak1556]

LGTM, with nit

[release-clerk]

Release Notes Persisted

DevTools errors are no longer printed to console

[trop]

I have automatically backported this PR to "40-x-y", please check out #49359