299085 - EM should handle install errors instead of just throwing to xpinstall

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Robert Strong (they/them - no direct email)]

If the EM throws during an install xpinstall will display an Unexpected error
-203 alert which has little value for diagnosing the problem and can have the
affect of morphing an existin platform specific bug, etc. as can be seen in bug
295732. Also, xpinstall does not display this error when a theme fails to
install so no error is displayed to the user in the case of themes.

Comment 1

[Robert Strong (they/them - no direct email)]

I'm not sure when I will have the time to look at this so reassigning to default
owner.

Comment 2

[Robert Strong (they/them - no direct email)]

Attachment: patch

This patch changes the generic "Unexpected error: -203" message to "Unexpected
installation error". It also adds the line "Review the JavaScript Console log
for more details" to the error displayed. The reason for not displaying the
-203 is to lessen one bug being confused for another bug since error numbers
are usually thought to point to a specific problem.

These errors should and have only occured when an extension author makes a
mistake packaging an extension or when there is a serious bug in the EM. These
error messages should never be seen with releases except by extension
developers unless an extension developer releases an extension that they didn't
try to install.

Comment 3

[Robert Strong (they/them - no direct email)]

Attachment: simpler [checked in]

Just add a string for handling -203 errors.

Comment 4

[Robert Strong (they/them - no direct email)]

Benjamin - I think this would be a good thing for 1.8b4. As is, whenever someone
gets a generic failure they end up thinking they are experiencing a bug that has
already been fixed due to the -203 in the message. Though this won't clear up
why the error occured it does prevent it from being associated with -203.

Comment 5

[:Gavin Sharp [email: [e-mail]]]

Checking in xpinstall.properties;
/cvsroot/mozilla/toolkit/locales/en-US/chrome/global/xpinstall/xpinstall.properties,v
 <--  xpinstall.properties
new revision: 1.5; previous revision: 1.4
done

Comment 6

[:Gavin Sharp [email: [e-mail]]]

Sorry Rob, I hadn't realized you'd gotten your CVS account, and I went ahead and
checked this in.

Comment 7

[Robert Strong (they/them - no direct email)]

Gavin - this is as simple of a fix as possible so no worries... thanks for the
checkin.

I haven't decided if more still needs to be done here so I'm going to leave this
open for now.

Comment 8

[Robert Strong (they/them - no direct email)]

In thinking on this I believe detailed error messages that would under normal
circumstances only be seen by or be useful to extension authors / nightly
testers should reside in the JS console and a simple alert error message should
be displayed. This avoids the addition of strings which then need to be reviewed
etc. (see bug 302527 for one example) and this sort of detail doesn't provide
value to the average user... if anything it confuses them.

Resolved -> Fixed