1467829 - Telemetry JS errors have no stack information, making them hard to track down

Status: NEW

(Toolkit :: Telemetry, enhancement, P3)

Description

[:Gijs (he/him)]

E.g. https://dxr.mozilla.org/mozilla-central/rev/199a085199815cc99daa658956a7c9436e1d436b/toolkit/components/telemetry/TelemetryHistogram.cpp#1689

There's a few others in the same file.

I think it should be fairly easy to use nsContentUtils to report something that at least has some caller information.

Comment 1

[Georg Fritzsche [:gfritzsche]]

(In reply to :Gijs (he/him) from comment #0)
> I think it should be fairly easy to use nsContentUtils to report something
> that at least has some caller information.

Are you thinking about a specific function?
From looking around, e.g. `nsContentUtils::LogSimpleConsoleError()` doesn't print any caller information.

Comment 2

[:Gijs (he/him)]

(In reply to Georg Fritzsche [:gfritzsche] from comment #1)
> (In reply to :Gijs (he/him) from comment #0)
> > I think it should be fairly easy to use nsContentUtils to report something
> > that at least has some caller information.
> 
> Are you thinking about a specific function?
> From looking around, e.g. `nsContentUtils::LogSimpleConsoleError()` doesn't
> print any caller information.

Ugh, yeah, so, that's bug 1438553. It may be possible to use the code from that patch here directly, assuming the same issues that have stopped that patch from landing don't apply to these callsites, which I'm not sure about off-hand...

Comment 3

[Alessio Placitelli [:Dexter]]

(In reply to :Gijs (he/him) from comment #2)
> (In reply to Georg Fritzsche [:gfritzsche] from comment #1)
> > (In reply to :Gijs (he/him) from comment #0)
> > > I think it should be fairly easy to use nsContentUtils to report something
> > > that at least has some caller information.
> > 
> > Are you thinking about a specific function?
> > From looking around, e.g. `nsContentUtils::LogSimpleConsoleError()` doesn't
> > print any caller information.
> 
> Ugh, yeah, so, that's bug 1438553. It may be possible to use the code from
> that patch here directly, assuming the same issues that have stopped that
> patch from landing don't apply to these callsites, which I'm not sure about
> off-hand...

Let's wait for the discussion on that bug to settle, I'm adding it as a dependency.

Comment 4

[Boris Zbarsky [:bzbarsky]]

So this is specifically about stack information, not just line/file information?  JS_ReportErrorASCII should have the latter.

Comment 5

[Alessio Placitelli [:Dexter]]

(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #4)
> So this is specifically about stack information, not just line/file
> information?  JS_ReportErrorASCII should have the latter.

This is what we're using there. Mh, leaving myself a ni? to verify if the stack gets reported there or not.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Maybe I was unclear.  JS_ReportErrorASCII will generally _not_ have a stack.  It _will_ have the file and line for the topmost stack frame only on the error report.

Comment 7

[Alessio Placitelli [:Dexter]]

(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #6)
> Maybe I was unclear.  JS_ReportErrorASCII will generally _not_ have a stack.
> It _will_ have the file and line for the topmost stack frame only on the
> error report.

Ouch, sorry, I misread comment 4. Yes, this is specifically about stack information.