Closed Bug 1446450 Opened 2 years ago Closed 9 months ago

Error message "Extension is invalid" is not helpful, not enough details

Categories

(DevTools :: about:debugging, enhancement, P2)

57 Branch
enhancement

Tracking

(firefox67 fixed)

RESOLVED FIXED
Firefox 67
Tracking Status
firefox67 --- fixed

People

(Reporter: jaws, Assigned: jdescottes)

References

Details

Attachments

(6 files)

Attached file extension.zip
Loading the attached manifest.json in about:debugging shows an error with the message "Extension is invalid". The webpage console doens't have anything more helpful either, only repeating the same message:

Error: Extension is invalid
Stack trace:
loadManifestFromWebManifest@resource://gre/modules/addons/XPIInstall.jsm:333:11
 Controls.js:85:9

Loading the Browser Console shows:
1521218691408	addons.webextension.<unknown>	ERROR	Loading extension 'null': Reading manifest: Error processing content_scripts.0.matches: Expected array instead of "*://*.foo.com/*"

We shouldn't have to force extension developers to open the Browser Console to find the message. This error message should be included in the about:debugging page.
I think this is a regression since we used to show manifest errors.
Mark, I think you implemented that originally can you take a look?
Flags: needinfo?(mstriemer)
Product: Firefox → DevTools
Displaying a error messages in about:debugging was added in https://bugzilla.mozilla.org/show_bug.cgi?id=1330732 
The featured is covered by a test, so this should not be a regression. The feature is catching startup issues, but this is an installation issue.

Here, the installation fails with the following stack trace: 

Error: "Extension is invalid"
  loadManifestFromWebManifestresource://gre/modules/addons/XPIInstall.jsm:441:11
  loadManifestresource://gre/modules/addons/XPIInstall.jsm:807:21
  loadManifestFromFileresource://gre/modules/addons/XPIInstall.jsm:854:23
  installTemporaryAddonresource://gre/modules/addons/XPIInstall.jsm:3817:23
  methresource://gre/modules/addons/XPIProvider.jsm:2719:29
  installTemporaryAddonresource://gre/modules/AddonManager.jsm:2052:12
  installTemporaryAddonresource://gre/modules/AddonManager.jsm:3344:12
  installAddonresource://devtools/client/aboutdebugging/components/addons/Controls.js:80:5
  loadAddonFromFileresource://devtools/client/aboutdebugging/components/addons/Controls.js:70:7

Simply changing https://searchfox.org/mozilla-central/source/toolkit/mozapps/extensions/internal/XPIInstall.jsm#441 to throw (extension.errors[0]) (or a concatenation of all error messages...) would allow about:debugging to display a proper error message.

Is there any concern with leaking errors to the users? I guess not, since the error is visible in the browser console anyway.
If not we can proceed with this change, and probably add a new test.
Severity: normal → enhancement
Flags: needinfo?(aswan)
Priority: -- → P2
(In reply to Julian Descottes [:jdescottes][:julian] from comment #2)
> Displaying a error messages in about:debugging was added in
> https://bugzilla.mozilla.org/show_bug.cgi?id=1330732 
> The featured is covered by a test, so this should not be a regression. The
> feature is catching startup issues, but this is an installation issue.

Ah, right.

> Simply changing
> https://searchfox.org/mozilla-central/source/toolkit/mozapps/extensions/
> internal/XPIInstall.jsm#441 to throw (extension.errors[0]) (or a
> concatenation of all error messages...) would allow about:debugging to
> display a proper error message.

I would prefer to put the errors into a property on the Error that is thrown rather than cramming them into the message.  (that would let the front-end format them nicely as well...)

> Is there any concern with leaking errors to the users? I guess not, since
> the error is visible in the browser console anyway.
> If not we can proceed with this change, and probably add a new test.

It was a deliberate decision that the regular add-on install flow (ie, not from about:debugging) doesn't show detailed errors about invalid extensions, but displaying them here is of course desirable.
Flags: needinfo?(aswan)

Any progress here?

I'm willing to write a patch that will simply call console.error with the contents of error. Right now, this message is simply useless.

Flags: needinfo?(aswan)

(In reply to David Teller [:Yoric] (please use "needinfo") from comment #4)

Any progress here?

I'm not working on this. The devtools team assigned priority P2, which in theory means they intend to work on it relatively soon?

Flags: needinfo?(aswan)

(In reply to David Teller [:Yoric] (please use "needinfo") from comment #4)

Any progress here?

Sorry this fell out of the radar. I will submit some patches for this later today, it's easy enough to address.

I'm willing to write a patch that will simply call console.error with the contents of error. Right now, this message is simply useless.

With the scenario logged here, the detailed error message is already in the Browser Console, but not surfaced in about:debugging. Calling console.error would not change this? I wonder if you have a different scenario where you don't even see the message in the console? Can you check if you don't see the message in the console already, and if that's the case share the addon you are using here?

Flags: needinfo?(mstriemer) → needinfo?(dteller)
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED

I'm willing to write a patch that will simply call console.error with the contents of error. Right now, this message is simply useless.

With the scenario logged here, the detailed error message is already in the Browser Console, but not surfaced in about:debugging. Calling console.error would not change this? I wonder if you have a different scenario where you don't even see the message in the console? Can you check if you don't see the message in the console already, and if that's the case share the addon you are using here?

In my case, this was an invalid key in manifest.json. No error message other than Extension is invalid in any kind of console.

Flags: needinfo?(dteller)

Depends on D20651

Addressing suggestion from D20648 :)

Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/53b94170e100
Forward detailed errors for "Extension is invalid" errors;r=aswan
https://hg.mozilla.org/integration/autoland/rev/f1993f5194af
Show additional error messages in about:debugging;r=daisuke
https://hg.mozilla.org/integration/autoland/rev/55f4f862586c
Show additional error messages in about:debugging-new;r=daisuke
https://hg.mozilla.org/integration/autoland/rev/6ab8ccf59cae
Add test to check the display of additional addon install error messages;r=daisuke
https://hg.mozilla.org/integration/autoland/rev/c5e6c33c5737
Add webextensions unit test for additionalErrors;r=aswan
Duplicate of this bug: 1536900
Duplicate of this bug: 1289181
Duplicate of this bug: 1407087
You need to log in before you can comment on or make changes to this bug.