Closed Bug 1140055 Opened 5 years ago Closed 5 years ago

Exception in TelemetryEnvironment: "TypeError: theme.installDate is null"

Categories

(Toolkit :: Telemetry, defect)

39 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox39 --- fixed

People

(Reporter: vladan, Assigned: Dexter)

References

Details

Attachments

(1 file)

With browser.dom.window.dump.enabled set, I saw the following exception repeatedly in stdout:

*************************
A coding exception was thrown and uncaught in a Task.

Full message: TypeError: theme.installDate is null
Full stack: this.TelemetryEnvironment._getActiveTheme<@resource://gre/modules/TelemetryEnvironment.jsm:737:9
TaskImpl_run@resource://gre/modules/Task.jsm:314:40

*************************

Note, that I'm using a default theme.
Attached patch bug1140055.patchSplinter Review
That's odd, that doesn't happen on my system with the default theme. Environment should definitely be more robust about that, so I've added some checks with this patch and updated the documentation.
Assignee: nobody → alessio.placitelli
Status: NEW → ASSIGNED
Attachment #8573806 - Flags: review?(vdjeric)
Comment on attachment 8573806 [details] [diff] [review]
bug1140055.patch

Review of attachment 8573806 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/telemetry/TelemetryEnvironment.jsm
@@ +720,5 @@
>          type: addon.type,
>          foreignInstall: addon.foreignInstall,
>          hasBinaryComponents: addon.hasBinaryComponents,
> +        installDay: truncateToDays(installDate.getTime()),
> +        updateDay: truncateToDays(updateDate.getTime()),

maybe report 0 if the date is null? doesn't really matter, but the server analysis code will have to check for 1970-01-01 instead of 0

Your call
Attachment #8573806 - Flags: review?(vdjeric) → review+
Thanks for the review! We already report 0 as |Date.getTime()| returns 0 for |new Date(0)|, so we should be covered.

Try-push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8ab780527844
Keywords: checkin-needed
(In reply to Alessio Placitelli [:Dexter] from comment #3)
> Thanks for the review! We already report 0 as |Date.getTime()| returns 0 for
> |new Date(0)|, so we should be covered.

Hmm, what happens when truncateTodays gets passed 0? Did you test that scenario?
Flags: needinfo?(alessio.placitelli)
Yes, I've tested it: |truncateToDays| returns 0 when the passed argument is 0. I can explicitly add some more tests in test_TelemetryEnvironment.js for that if you think that's better!
Flags: needinfo?(alessio.placitelli)
(In reply to Alessio Placitelli [:Dexter] from comment #6)
> Yes, I've tested it: |truncateToDays| returns 0 when the passed argument is
> 0. I can explicitly add some more tests in test_TelemetryEnvironment.js for
> that if you think that's better!

Oic, I was looking at the TelemtrySession's version of truncateToDays which takes a Date as a parameter
https://hg.mozilla.org/mozilla-central/rev/93a6c9f5b4bd
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla39
You need to log in before you can comment on or make changes to this bug.