1453493 - Capture console log output when users click Report Site Issue

Status: RESOLVED DUPLICATE of bug 1451485

(Web Compatibility :: Tooling & Investigations, enhancement, P1)

Description

[Mike Taylor [:miketaylr]]

Possibly this should be opt-in at report-time (or def. opt-out), but it would be nice if we can capture the state of the console when a user click Report Site Issue, to be embedded in the bug that gets filed.

Comment 1

[Mike Taylor [:miketaylr]]

Tom, can you do some investigation on how we might be able to get at console logs to be included in the `details` param at report time?

https://dxr.mozilla.org/mozilla-central/source/browser/extensions/webcompat-reporter/content/WebCompatReporter.jsm#94

(ignore `prefs` there, it will be changed to something more general)

Comment 2

[Mike Taylor [:miketaylr]]

We might just want to start with errors.

Comment 3

[Mike Taylor [:miketaylr]]

Brian, Harald mentioned to me that you would know the most about this -- can you give some pointers on reading console errors for a given tab? Thanks.

Comment 4

[Brian Grinstead [:bgrins]]

(In reply to Mike Taylor [:miketaylr] from comment #3)
> Brian, Harald mentioned to me that you would know the most about this -- can
> you give some pointers on reading console errors for a given tab? Thanks.

Do you care only about errors on the page or also care about console API messages (i.e. if the page did `console.log`)?

Because if it's the former only, then errors can be fetched with `Services.console.getMessageArray()` and filtering for Ci.nsIScriptError messages with the correct innerWindowID for the window. Depending on if you care about errors from iframes on the page (or service workers, etc) you would want to include those window IDs as well.

Here's what the webconsole does for example: https://searchfox.org/mozilla-central/rev/78dbe34925f04975f16cb9a5d4938be714d41897/devtools/server/actors/webconsole/listeners.js#135.

Comment 5

[Mike Taylor [:miketaylr]]

Awesome, thanks Brian!

Comment 6

[Thomas Wisniewski]

Brian, is it expected that the innerWindowID property for each message returned by `Services.console.getMessageArray()` would be 0 when accessed by an addon in browser/extensions? (I see that the devtools console is getting the expected innerWindowID for the same messages that I'm getting 0 for in a call made from browser/extensions/webcompat-reporter/content/WebCompatReporter.jsm)

Comment 7

[Brian Grinstead [:bgrins]]

(In reply to Thomas Wisniewski from comment #6)
> Brian, is it expected that the innerWindowID property for each message
> returned by `Services.console.getMessageArray()` would be 0 when accessed by
> an addon in browser/extensions? (I see that the devtools console is getting
> the expected innerWindowID for the same messages that I'm getting 0 for in a
> call made from
> browser/extensions/webcompat-reporter/content/WebCompatReporter.jsm)

No, I would expect to see the same innerWindowID from any caller. Do you see the correct value if you run the `Services.console.getMessageArray()` call in the Browser Console input?

If you could put up a patch and STR to show the issue in WebCompatReporter.jsm I'll take a look.

Comment 8

[Thomas Wisniewski]

Sorry for the delay here.

But no, running this in the browser console:

>Services.console.getMessageArray().map(m => [m.innerWindowID, m.message])

Returns either 0 or undefined for the innerWindowID for all of the messages (with the ones for the window I'm looking for all returning 0). (And other messages from other windows are also using 0, so it's not just the window ID).

My patch is getting the same 0/undefineds from that call.

I've attached the WIP patch here, just in case. (I'm testing by reporting a simple local HTML page that accesses window.event, which raises an obvious console error, so I can distinguish that one from other errors. Reporting just means clicking on the three-dot page action menu in the address bar and picking "report issue", which will open a new tab after logging my patch's debug messages to console.)

And for the record, I'm currently expecting the ID to be the same as what I get from running this code fragment in the addon (which logs an unsafe CPOW access, though I don't know how to otherwise get the value):

>let contentWin = gBrowser.selectedBrowser._contentWindow;
>let id = contentWin.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDOMWindowUtils).currentInnerWindowID;

Perhaps this is happening because the innerWindowID can't be computed properly by getMessageArray, due to cross-process restrictions (by either my addon or the browser console)?

Comment 9

[Thomas Wisniewski]

Actually, I figured out the problem. It turns out that I wasn't querying the messages as nsIScriptErrors (for example with instanceof) before trying to access their nsIScriptErrors-specific properties (including innerWindowID).

I'll shortly attach a fresh patch for review which should add the same messages that are displayed in the web console into details["log"]. That includes both script errors and calls to console.log and friends, as an array of strings like this (we can tweak the specifics as desired):

  [
    "[console.log([\"OK\"]) http://localhost/testcase.html:14:1]",
    "[JavaScript Error: \"TypeError: window.event is undefined\" {file: \"http://localhost/testcase.html\" line: 15}]\n@http://localhost/testcase.html:15:1\n",
    "[JavaScript Error: \"The character encoding of the HTML document was not declared. The document will render with garbled text in some browser configurations if the document contains characters from outside the US-ASCII range. The character encoding of the page must be declared in the document or in the transfer protocol.\" {file: \"http://localhost/testcase.html\" line: 0}]"
  ]

Comment 10

[Thomas Wisniewski]

Attachment: Bug 1453493 - Capture console log output when users click Report Site Issue.

Review commit: https://reviewboard.mozilla.org/r/250576/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/250576/

Comment 11

[Thomas Wisniewski [:twisniewski]]

Mike, I just realized that if there are enough messages in the console log, our current approach of sending everything over GET will result in a 414 error. What should we do about that?

Comment 12

[Mike Taylor [:miketaylr]]

(In reply to twisniewski from comment #11)
> Mike, I just realized that if there are enough messages in the console log,
> our current approach of sending everything over GET will result in a 414
> error. What should we do about that?

Ah yeah. Using GET params won't work here. Karl filed https://github.com/webcompat/webcompat.com/issues/2024 a while back for this purpose. We should probably block on getting something like that implemented.

Comment 13

[Brian Grinstead [:bgrins]]

Comment on attachment 8984786 [details]
Bug 1453493 - Capture console log output when users click Report Site Issue.

https://reviewboard.mozilla.org/r/250576/#review259446

This looks good to me from the console side. I think this will miss Service Worker messages - is that important to include them?

A bit unfortunate that we require this much boilerplate to fetch messages that match a window and all subframes. I actually wonder if we could expose functions on ConsoleAPIStorage and Services.console where you pass in a window ID and it includes and message from subframes and matching Service Workers. Would require some thinking to see if it would work for both the webconsole and this use case, but it seems like it should.

Comment 14

[Brian Grinstead [:bgrins]]

(In reply to Brian Grinstead [:bgrins] from comment #13)
> A bit unfortunate that we require this much boilerplate to fetch messages
> that match a window and all subframes. I actually wonder if we could expose
> functions on ConsoleAPIStorage and Services.console where you pass in a
> window ID and it includes and message from subframes and matching Service
> Workers. Would require some thinking to see if it would work for both the
> webconsole and this use case, but it seems like it should.

Although I guess we'd need to still have isMessageRelevant handling when observing new messages for the console frontend, so that helper may not save us a ton of code on the webconsole side unless if we also exposed a helper for dealing with observed messages.

Comment 15

[Thomas Wisniewski [:twisniewski]]

We decided to just land this in the webextension version of the reporter in bug 1451485, rather than do so first in the bootstrapped version. As such I'm closing this as a dupe of that bug.

If we ever need to add ServiceWorker messages or do some refactoring (as per comment 13), let's file new bugs against the reporter.