1012750 - defineLazyModuleGetter should log URL if Cu.import call fails

Status: VERIFIED FIXED

(Core :: XPConnect, defect)

Description

[:Gijs (he/him)]

Currently it shows:

NS_ERROR_FILE_NOT_FOUND: Component returned failure code: 0x80520012 (NS_ERROR_FILE_NOT_FOUND) [nsIXPCComponents_Utils.import]

Which is not particularly helpful.

(I see this on every nightly startup, and don't know if it's an add-on or core code, but I aim to find out and improve the platform in the process...)

Comment 1

[:Gijs (he/him)]

Attachment: defineLazyModuleGetter should log failing URL if there was an error loading,

This works. The problematic call was the translation language detection JSM, which isn't built by default, but is invoked if you have the pref toggled, apparently. I'll file a separate bug about that.

Comment 2

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Comment on attachment 8425098 [details] [diff] [review]
defineLazyModuleGetter should log failing URL if there was an error loading,

Review of attachment 8425098 [details] [diff] [review]:
-----------------------------------------------------------------

I'm not a peer here, but this change is trivial and I reviewed a similar trivial change here in the past, so I guess it's ok.

::: js/xpconnect/loader/XPCOMUtils.jsm
@@ +226,5 @@
> +      try {
> +        Cu.import(aResource, temp);
> +      } catch (ex) {
> +        Cu.reportError("Failed to load module " + aResource + " : " + ex);
> +        throw ex;

it seems a bit verbose to append the ex that is about to be thrown, is not the exception already shown?

Comment 3

[:Gijs (he/him)]

remote:   https://hg.mozilla.org/integration/fx-team/rev/c942b71f38d2

And then I realized I missed your nit:

remote:   https://hg.mozilla.org/integration/fx-team/rev/62c6e6a9b6bb

Also, more backlog fodder.

Comment 4

[Marco Mucci [:MarcoM]]

Added to Iteration 32.2

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c942b71f38d2
https://hg.mozilla.org/mozilla-central/rev/62c6e6a9b6bb

Comment 6

[Marco Mucci [:MarcoM]]

Hi Juan, can you review the bug to determine if further verification is required.

Comment 7

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 8425098 [details] [diff] [review]
defineLazyModuleGetter should log failing URL if there was an error loading,

Review of attachment 8425098 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/xpconnect/loader/XPCOMUtils.jsm
@@ +226,5 @@
> +      try {
> +        Cu.import(aResource, temp);
> +      } catch (ex) {
> +        Cu.reportError("Failed to load module " + aResource + " : " + ex);
> +        throw ex;

Shouldn't we display the stack?

Comment 8

[:Gijs (he/him)]

(In reply to David Rajchenbach Teller [:Yoric] (please use "needinfo?" - I'll be away on May 16th-23th) from comment #7)
> Comment on attachment 8425098 [details] [diff] [review]
> defineLazyModuleGetter should log failing URL if there was an error loading,
> 
> Review of attachment 8425098 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: js/xpconnect/loader/XPCOMUtils.jsm
> @@ +226,5 @@
> > +      try {
> > +        Cu.import(aResource, temp);
> > +      } catch (ex) {
> > +        Cu.reportError("Failed to load module " + aResource + " : " + ex);
> > +        throw ex;
> 
> Shouldn't we display the stack?

I don't think that's super-useful, and in part it's the console/debugger's problem (certainly the browser debugger shows stacks for uncaught exceptions, like the re-throw). It would be the stack for when the symbol provided by delayedLazyModuleGetter would first be requested. Providing the loading exception and the url of the module should easily be enough to pinpoint the issue in 99.99% of cases (as opposed to just a generic exception without telling you which module it's about, which it was before this patch...).