1440816 - Errors from child/content processes do not have stacks in nsIConsoleService listeners

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[Osmose [:osmose, :mkelly]]

STR:

1. Open https://www.mozilla.org/en-US/
2. Open the Browser Content Toolbox
3. Run `Cu.reportError("test")`
4. Open the Browser Console

The error logged in the browser console (and dispatched by nsIConsoleService) doesn't have a stack trace. The error logged in the Browser Content Toolbox does, however.

I can also replicate this by dropping reportError calls in content process code and registering my own listeners on the console service, but the steps above are easier for replication.

We want stack traces to help with reporting and diagnosing these errors (bug 1426479). The top 3 errors that we're currently collecting all have no stack trace due to this issue.

Comment 1

[Kris Maglione [:kmag]]

Attachment: Bug 1440816: Part 1 - Set readPrincipals callback on JS context.

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

Comment 2

[Kris Maglione [:kmag]]

Attachment: Bug 1440816: Part 2 - Clone stacks when sending console messages to the parent process.

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

Comment 3

[Bobby Holley (:bholley)]

Comment on attachment 8953695 [details]
Bug 1440816: Part 1 - Set readPrincipals callback on JS context.

I don't understand the history of this API. It looks like it was added in bug 1209263, but non non-test consumers? Delegating to fitzgen.

Comment 4

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Comment on attachment 8953695 [details]
Bug 1440816: Part 1 - Set readPrincipals callback on JS context.

https://reviewboard.mozilla.org/r/222928/#review229300

LGTM.

I think in the devtools we only ever cloned SavedFrame stacks with reconstructed SavedFrame principals, so we never hit a code path that would have required the callback.

https://searchfox.org/mozilla-central/source/js/src/vm/StructuredClone.cpp#2513-2534

Comment 5

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8953696 [details]
Bug 1440816: Part 2 - Clone stacks when sending console messages to the parent process.

https://reviewboard.mozilla.org/r/222930/#review229700

::: dom/ipc/ContentChild.cpp:479
(Diff revision 1)
> +        ClonedMessageData cloned;
> +        if (!data.BuildClonedMessageDataForChild(mChild, cloned)) {
> +          return NS_ERROR_FAILURE;
> +        }
> +
> +        mChild->SendScriptErrorWithStack(msg, sourceName, sourceLine,

I think we want to return after this, otherwise we would send the error twice?

Comment 6

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4e681b986b5cdd36dbb8b50c232b8b5d4db8e4f3
Bug 1440816: Part 1 - Set readPrincipals callback on JS context. r=fitzgen

Comment 7

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/4e681b986b5c

Comment 8

[Osmose [:osmose, :mkelly]]

baku: This is blocking me from granting access to JS errors on Sentry to more Firefox devs, would you be able to look at this soon?

Comment 9

[Andrea Marchesini [:baku]]

Comment on attachment 8953696 [details]
Bug 1440816: Part 2 - Clone stacks when sending console messages to the parent process.

https://reviewboard.mozilla.org/r/222930/#review231968

Comment 10

[Osmose [:osmose, :mkelly]]

Hooray, thanks :D

Comment 11

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b93a9c630a0c9ce75b5fc1a0ab91f3a9adbbab87
Bug 1440816: Part 2 - Clone stacks when sending console messages to the parent process. r=baku

Comment 12

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/b93a9c630a0c

Comment 13

[Osmose [:osmose, :mkelly]]

We've been getting stacks in sentry from errors in child processes thanks to this bug for a few weeks now.