Closed
Bug 1233438
Opened 7 years ago
Closed 7 years ago
Addon data missing in environment if there is an addon with undefined .description
Categories
(Toolkit :: Telemetry, defect, P3)
Toolkit
Telemetry
Tracking
()
VERIFIED
FIXED
mozilla46
People
(Reporter: Dexter, Assigned: hu.eric, Mentored)
References
(Blocks 1 open bug)
Details
(Keywords: regression, Whiteboard: [measurement:client])
Attachments
(1 file)
I just noticed that mochitest-browser-chrome bc4 tests logs are cluttered with this errors: 06:16:35 INFO - ************************* 06:16:35 INFO - A coding exception was thrown and uncaught in a Task. 06:16:35 INFO - Full message: TypeError: aString is undefined 06:16:35 INFO - Full stack: limitStringToLength@resource://gre/modules/TelemetryEnvironment.jsm:267:3 06:16:35 INFO - EnvironmentAddonBuilder.prototype._getActiveAddons<@resource://gre/modules/TelemetryEnvironment.jsm:521:22 06:16:35 INFO - EnvironmentAddonBuilder.prototype._updateAddons<@resource://gre/modules/TelemetryEnvironment.jsm:479:27 06:16:35 INFO - TaskImpl_run@resource://gre/modules/Task.jsm:315:40 06:16:35 INFO - TaskImpl@resource://gre/modules/Task.jsm:276:3 06:16:35 INFO - createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:250:14 06:16:35 INFO - EnvironmentAddonBuilder.prototype._checkForChanges@resource://gre/modules/TelemetryEnvironment.jsm:437:25 06:16:35 INFO - EnvironmentAddonBuilder.prototype._onAddonChange@resource://gre/modules/TelemetryEnvironment.jsm:422:5 06:16:35 INFO - EnvironmentAddonBuilder.prototype.onInstalled@resource://gre/modules/TelemetryEnvironment.jsm:414:5 06:16:35 INFO - AddonManagerInternal.callAddonListeners@resource://gre/modules/AddonManager.jsm:1801:11 06:16:35 INFO - this.AddonManagerPrivate.callAddonListeners@resource://gre/modules/AddonManager.jsm:2842:5 06:16:35 INFO - _setCurrentTheme@resource://gre/modules/LightweightThemeManager.jsm:700:8 06:16:35 INFO - this.LightweightThemeManager.currentTheme@resource://gre/modules/LightweightThemeManager.jsm:147:12 06:16:35 INFO - @chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser/browser_bug591465.js:255:3 06:16:35 INFO - log_exceptions@chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser/head.js:185:12 06:16:35 INFO - run_next_test/<@chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser/head.js:227:21 06:16:35 INFO - testScope/test_executeSoon/<.run@chrome://mochikit/content/browser-test.js:968:9 06:16:35 INFO - ************************* We should check in [0] that aString is both non-null and non-undefined. [0] - https://dxr.mozilla.org/mozilla-central/rev/0babaa3edcf908c393b68a3dc2d1c2a2450c31ed/toolkit/components/telemetry/TelemetryEnvironment.jsm#264
Reporter | ||
Updated•7 years ago
|
Points: --- → 1
Priority: -- → P3
Whiteboard: [measurement:client]
Comment 1•7 years ago
|
||
This can be reproduced by looking at the output of: mach test browser/base/content/test/social/browser_aboutHome_activation.js
Updated•7 years ago
|
Assignee: nobody → pineapple.rice
Updated•7 years ago
|
Mentor: gfritzsche, alessio.placitelli
Updated•7 years ago
|
Keywords: regression
Comment 2•7 years ago
|
||
[Tracking Requested - why for this release]: We broke our addons data collection here: If users have any addons without a description field installed, we don't collect any addon data. This is a regression from bug 1211404, which sadly went out on release with 43.
status-firefox43:
--- → wontfix
status-firefox44:
--- → affected
status-firefox45:
--- → affected
tracking-firefox44:
--- → ?
Comment 3•7 years ago
|
||
So, this might not be a large-scale issue, we are currently looking into the impact here. Normal extensions appear to always have a null .description if there is none provided in the .rdf. The cause in tests here is the SocialProvider and its AddonWrapper: https://dxr.mozilla.org/mozilla-central/rev/66fb852962c0d5f6f5fe0604204da4f5d17763c9/toolkit/components/social/SocialService.jsm#1176 I assume some of those test manifests simply don't have a description, which might not be an issue on release.
Attachment #8699933 -
Flags: review?(gfritzsche)
Comment 5•7 years ago
|
||
Comment on attachment 8699933 [details] [diff] [review] Add safety checking for undefined value of aString. Prevents error on snippet code attempting to add an addon without a description. Review of attachment 8699933 [details] [diff] [review]: ----------------------------------------------------------------- Thanks!
Attachment #8699933 -
Flags: review?(gfritzsche) → review+
Comment 6•7 years ago
|
||
We took this through good testing here locally, it looks like the main issue is with the fake addons the SocialProvider testing sets up. Pushed this to try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=db545b100c21
Comment 7•7 years ago
|
||
Alexandra, can you verify this when it hits Nightly? I would love to get this verified quickly so it can get uplifted to 44 & 45 soon.
Flags: qe-verify+
Flags: needinfo?(alexandra.lucinet)
QA Contact: alexandra.lucinet
Comment 9•7 years ago
|
||
ni?dexter for filing a follow-up for test-coverage on this.
Flags: needinfo?(alessio.placitelli)
Updated•7 years ago
|
Summary: "TypeError: aString is undefined" in browser tests at TelemetryEnvironment.jsm:267:3 → Addon data missing in environment if there is an addon with undefined .description
Comment 10•7 years ago
|
||
Related: bug 1190346 mentions malware addons etc. having missing .name, .version, .description
Comment 11•7 years ago
|
||
Follow-up on fhr-dev: https://mail.mozilla.org/pipermail/fhr-dev/2015-December/000745.html
Comment 12•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/01a10d89e8a1
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Reporter | ||
Comment 13•7 years ago
|
||
(In reply to Georg Fritzsche [:gfritzsche] [away Dec 19 - Jan 3] from comment #11) > Follow-up on fhr-dev: > https://mail.mozilla.org/pipermail/fhr-dev/2015-December/000745.html A larger sample [0] confirmed that the percentage of clients using addons/themes/plugins with no description or name is still the 14%. [0] - https://gist.github.com/Dexterp37/20d2631e0b0090494500
Reporter | ||
Comment 14•7 years ago
|
||
Filed bug 1234197 for adding test coverage.
Flags: needinfo?(alessio.placitelli)
Comment 15•7 years ago
|
||
Successfully reproduced and verified this issue on builds provided by Alessio (thanks!), under Windows 7 64-bit; also, some additional exploratory testing was performed with latest 46.0a1 (from 2015-12-21), across platforms [1] [1] Windows 10 32-bit, Ubuntu 12.04 32-bit and Mac OS X 10.11
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Flags: needinfo?(alexandra.lucinet)
Reporter | ||
Comment 16•7 years ago
|
||
Comment on attachment 8699933 [details] [diff] [review] Add safety checking for undefined value of aString. Prevents error on snippet code attempting to add an addon without a description. Approval Request Comment [Feature/regressing bug #]: bug 1211404 [User impact if declined]: Users with broken addons (e.g. malware, etc.) will produce Telemetry pings with no addons, plugins and themes in the related environment's section. Moreover, the about:telemetry will not contain any data for addons, plugins and themes. [Describe test coverage new/current, TreeHerder]: Manual QA verification. [Risks and why]: Low risk, as this would impact on Telemetry environment's data collection for addons, plugins and themes, which is already broken for those clients. [String/UUID change made/needed]: None
Attachment #8699933 -
Flags: approval-mozilla-beta?
Attachment #8699933 -
Flags: approval-mozilla-aurora?
Comment on attachment 8699933 [details] [diff] [review] Add safety checking for undefined value of aString. Prevents error on snippet code attempting to add an addon without a description. This seems to address data quality issues in Telemetry for broken addon scenarios. Beta44+, Aurora45+
Attachment #8699933 -
Flags: approval-mozilla-beta?
Attachment #8699933 -
Flags: approval-mozilla-beta+
Attachment #8699933 -
Flags: approval-mozilla-aurora?
Attachment #8699933 -
Flags: approval-mozilla-aurora+
I don't feel the need to track this bug as I would not block the FF44 release on this anyway.
Comment 19•7 years ago
|
||
bugherderuplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/cee07fcf6676
Comment 20•7 years ago
|
||
bugherderuplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/47c9ab8f0eb6
Comment 21•7 years ago
|
||
bugherderuplift |
https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/47c9ab8f0eb6
status-b2g-v2.5:
--- → fixed
Comment 23•7 years ago
|
||
Confirming this fix with a 44 beta build (Build ID: 20160104173512) provided by :Dexter, under Windows 7 64-bit, along with some additional exploratory testing performed with Firefox 44.0b4 (Build ID: 20151228134903), across platforms [1] [1] Mac OS X 10.11, Ubuntu 14.04 32-bit, Windows 7 x64 and Windows 10 x64
Flags: qe-verify+
You need to log in
before you can comment on or make changes to this bug.
Description
•