Closed Bug 1440816 Opened 2 years ago Closed Last year

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

Categories

(Core :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: osmose, Assigned: kmag)

References

Details

(Keywords: leave-open)

Attachments

(2 files)

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.
Assignee: nobody → kmaglione+bmo
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.
Attachment #8953695 - Flags: review?(bobbyholley) → review?(nfitzgerald)
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
Attachment #8953695 - Flags: review?(nfitzgerald) → review+
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?
Keywords: leave-open
No longer depends on: 1442992
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?
Flags: needinfo?(amarchesini)
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
Attachment #8953696 - Flags: review?(amarchesini) → review+
Hooray, thanks :D
Flags: needinfo?(amarchesini)
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
We've been getting stacks in sentry from errors in child processes thanks to this bug for a few weeks now.
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.