mozilla / testpilot

Test Pilot is a platform for performing controlled tests of new product concepts in Firefox
https://testpilot.firefox.com/
250 stars 123 forks source link

Exception in TestPilot's metrics when disabling TabCenter. #1283

Closed bwinton closed 7 years ago

bwinton commented 8 years ago

Mac OS X, Firefox Developer Edition 50.0a2 (2016-08-10).

TypeError: addon is null (testpilot-addon/lib/metrics.js:159) _(click for details)_ ``` 1472146749799 addons.manager WARN Exception calling callback: TypeError: addon is null (resource://gre/modules/commonjs/toolkit/loader.js -> resource://testpilot-addon/lib/metrics.js:159:9) JS Stack trace: module.exports.onExperimentPing/<@metrics.js:159:9 < safeCall@AddonManager.jsm:187:5 < makeSafe/<@AddonManager.jsm:203:25 < Handler.prototype.process@Promise-backend.js:937:23 < this.PromiseWalker.walkerLoop@Promise-backend.js:816:7 < Promise*this.PromiseWalker.scheduleWalkerLoop@Promise-backend.js:747:11 < this.PromiseWalker.schedulePromise@Promise-backend.js:779:7 < Promise.prototype.then@Promise-backend.js:454:5 < this.XPIDatabase.getAddon@XPIProviderUtils.js:1118:12 < this.XPIDatabase.getVisibleAddonForID@XPIProviderUtils.js:1167:5 < this.XPIProvider.getAddonByID@XPIProvider.jsm:4066:5 < callProviderAsync@AddonManager.jsm:262:12 < promiseCallProvider/<@AddonManager.jsm:287:53 < Promise@Promise-backend.js:388:5 < promiseCallProvider@AddonManager.jsm:286:10 < AddonManagerInternal.getAddonByID/promises<@AddonManager.jsm:2449:12 < AddonManagerInternal.getAddonByID@AddonManager.jsm:2448:20 < this.AddonManager.getAddonByID@AddonManager.jsm:3444:5 < uninstallExperiment@index.js:144:5 < emitOnObject@core.js:112:9 < emit@core.js:89:38 < App<.initialize/

(And don't be trying to move this to the TabCenter repo! It's a TestPilot bug! šŸ˜‰)

lmorchard commented 8 years ago

Error comes from this line in metrics.js. I think we don't expect AddonManager.getAddonByID to get called with the ID of a not-installed add-on here.

Need to check what ID is coming in via the subject of the experiment ping message. That might indeed be a Tab Center issue - unless we're fumbling it on the Test Pilot side, which is also possible.

bwinton commented 8 years ago

Probably the id of the just-uninstalled-TabCenterā€¦

lmorchard commented 8 years ago

I think that counts as a Test Pilot fumble, then - since we should handle a metrics ping on uninstall gracefully. Need to cache those add-on details somewhere on our side, so we still have them when the experiment goes away.

(To be fair, I expected something like that, but I just wanted to be snarky and try to move it to Tab Center anyway šŸ˜ )

bwinton commented 8 years ago

(It'll be less of a problem in the future, since Tab Center will be switching to event-based pings, and won't try to batch them up and send them on uninstallā€¦)