996037 - Telemetry experiments: Improve error messages for addon install failures

Status: RESOLVED WONTFIX

(Firefox Health Report Graveyard :: Client: Desktop, defect, P5)

Description

[Georg Fritzsche [:gfritzsche]]

Addon install failures are too cryptic in our logging, e.g.:

> ExperimentEntry #0::_installAddon() - AddonInstall onDownloadFailed for [e-mail], state=4, error=-2

We should at least give that one (STATE_DOWNLOAD_FAILED, ERROR_INCORRECT_HASH) a more descriptive message, if not all.
Otherwise QA and experiment writers can lose a lot of time on this.

Comment 1

[Georg Fritzsche [:gfritzsche]]

Blair, do we have functionality anywhere to stringify those error codes? Or am i overlooking some error message property?
I'd love to not re-list them for more descript error messages than "state=4, error-2".

Comment 2

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Hmm, no, there isn't. Good point. Best way forward is probably to just change the constants on the AddonManager object from numericals to more useful strings.

Only obvious downside of that is any add-on doing the obviously bad thing of hard-coding the constant value instead of using the constant name.... I don't specifically remember seeing any add-on doing that though (I've looked through the code of add-ons using AddonManager APIs numerous times before).


This should cover it:
http://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/AddonManager.jsm#2360,2362,2364,2366,2368,2370,2372,2374,2376,2381,2383,2385,2387,2391,2393,2395,2397,2399,2401,2403

Bonus points if, instead of updating the values in AddonUpdateChecker.jsm, you update that file to directly use the constants from the AddonManager object.

Comment 3

[Benjamin Smedberg]

I'm not sure that the risk associated with changing the API constants is worth the reward here. Also, low-priority in general. Make it a mentored bug if you can describe in some detail what should actually be changed here.

Comment 4

[Georg Fritzsche [:gfritzsche]]

This doesn't seem very important and nothing happened here in a while, lets be realistic and close it.

Comment 5

[Kamil Jozwiak [:kjozwiak]]

> This doesn't seem very important and nothing happened here in a while, lets
> be realistic and close it.

Is this the correct piece of code [1] that defines the state and error codes? I'll just add these definitions into the telemetry experiment wiki to help out the SV team that's taking over QAing.

[1] https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/AddonManager.jsm#2930

Comment 6

[Georg Fritzsche [:gfritzsche]]

Yes, that looks right. Might be enough to link it for future reference to avoid outdating.

Comment 7

[Kamil Jozwiak [:kjozwiak]]

Thanks! Created a quick document [1] and also linked it to the main page [2].

[1] https://wiki.mozilla.org/QA/Telemetry/State%26ErrorCodes#State_and_Error_Codes
[2] https://wiki.mozilla.org/QA/Telemetry#Telemetry_Experiments.2FFHR_Documentation