"use strict"; violations only logged at LOG level from AddonManager.jsm callback

NEW
Unassigned

Status

()

Toolkit
Add-ons Manager
3 years ago
a month ago

People

(Reporter: Peter Kehl, Unassigned, Mentored)

Tracking

({good-first-bug})

33 Branch
x86_64
Windows 7
good-first-bug
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

860 bytes, application/vnd.mozilla.xul+xml
Details
(Reporter)

Description

3 years ago
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:32.0) Gecko/20100101 Firefox/32.0
Build ID: 20140923175406

Steps to reproduce:

Have an XUL with "use strict"; Javascript which calls AddonManager.getAllAddons(..). Pass a callback function that breaks strict Javascript. XUL attached


Actual results:

No log in Browser Console at ERROR or WARNING level. Only a log at LOG level:

1415588570479	addons.manager	WARN	Exception calling callback: ReferenceError: assignment to undeclared variable x (chrome://selite-extension-sequencer/content/extensions/test.xul:9:12) JS Stack trace: @test.xul:9:13 < safeCall@AddonManager.jsm:168:5 < getAddonsByTypes_noMoreObjects@AddonManager.jsm:2061:9 < AOC_callNext@AddonManager.jsm:263:7 < getAddonsByTypes_concatAddons@AddonManager.jsm:2056:11 < SocialAddonProvider.getAddonsByTypes@SocialService.jsm:1051:5 < callProvider@AddonManager.jsm:194:12 < getAddonsByTypes_nextObject@AddonManager.jsm:2053:1 < AOC_callNext@AddonManager.jsm:269:7 < getAddonsByTypes_concatAddons@AddonManager.jsm:2056:11 < PL_getAddonsByTypes@PluginProvider.jsm:148:5 < callProvider@AddonManager.jsm:194:12 < getAddonsByTypes_nextObject@AddonManager.jsm:2053:1 < AOC_callNext@AddonManager.jsm:269:7 < getAddonsByTypes_concatAddons@AddonManager.jsm:2056:11 < OpenH264Provider.getAddonsByTypes@OpenH264Provider.jsm:354:5 < callProvider@AddonManager.jsm:194:12 < getAddonsByTypes_nextObject@AddonManager.jsm:2053:1 < AOC_callNext@AddonManager.jsm:269:7 < getAddonsByTypes_concatAddons@AddonManager.jsm:2056:11 < LightweightThemeManager_getAddonsByTypes@LightweightThemeManager.jsm:390:5 < callProvider@AddonManager.jsm:194:12 < getAddonsByTypes_nextObject@AddonManager.jsm:2053:1 < AOC_callNext@AddonManager.jsm:269:7 < getAddonsByTypes_concatAddons@AddonManager.jsm:2056:11 < getAddonsByTypes_getVisibleAddons@XPIProvider.jsm:3689:7 < makeSafe/<@XPIProviderUtils.js:146:17 < asyncMap_gotValue@XPIProviderUtils.js:181:7 < asyncMap_callback@XPIProviderUtils.js:188:9 < completeAddon@XPIProviderUtils.js:135:5 < getAddon@AddonRepository.jsm:585:7 < this.AddonRepository.getCachedAddonByID<@AddonRepository.jsm:617:5 < TaskImpl_run@Task.jsm:314:40 < TaskImpl@Task.jsm:275:3 < createAsyncFunction/asyncFunction@Task.jsm:249:14 < getRepositoryAddon@XPIProviderUtils.js:137:3 < asyncMap_each@XPIProviderUtils.js:187:7 < asyncMap@XPIProviderUtils.js:185:3 < this.XPIDatabase.getAddonList/<@XPIProviderUtils.js:1071:9 < Handler.prototype.process@Promise-backend.js:866:23 < this.PromiseWalker.walkerLoop@Promise-backend.js:745:7



Expected results:

Since the callback stops executing (fatal violation of JS strict mode), it should generate ERROR and not WARNING.

This may not be specific to AddonManager. The issue may be related to https://bugzilla.mozilla.org/show_bug.cgi?id=1094057.
(Reporter)

Comment 1

3 years ago
Created attachment 8519661 [details]
test.xul

Updated

3 years ago
Component: Untriaged → Add-ons Manager
Product: Firefox → Toolkit
See Also: → bug 1094057

Comment 2

5 months ago
Using the provided test.xul file I can see a different warning (see the screenshot https://www.screencast.com/t/ts3UpBoyI ) in the browser console on Firefox 53.0a1 (2017-01-17) under Wind 7 64-bit.

What do you think about this Kris?
Flags: needinfo?(kmaglione+bmo)

Comment 3

5 months ago
We should probably just Cu.reportError those exceptions. I'm not sure if using the AOM logger has any utility there. But Log.jsm should also probably log warnings at the warning rather than message level.
Flags: needinfo?(rhelmer)
Flags: needinfo?(kmaglione+bmo)
Flags: needinfo?(aswan)

Comment 4

5 months ago
It would be nice to distinguish where the exception is coming from.  Cu.reportError makes perfect sense if the exception is coming from front-end code.  If an exception is coming from an extension, it ought to be associated with that extension in the debugger, but I don't think that's all that practical so Cu.reportError everywhere is probably a reasonable thing to do.
Flags: needinfo?(aswan)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(rhelmer)
Keywords: good-first-bug

Comment 5

2 months ago
AddonManager.getAllAddons(..) is not going to be possible in Firefox 57 when we move to WebExtensions, does that mean its still going to be relevant. Rob, if this a good first bug, we'd need a bit more detail about what the assignee of this bug should do.
(In reply to Andy McKay [:andym] from comment #5)
> AddonManager.getAllAddons(..) is not going to be possible in Firefox 57 when
> we move to WebExtensions, does that mean its still going to be relevant.
> Rob, if this a good first bug, we'd need a bit more detail about what the
> assignee of this bug should do.

AddonManager APIs will still be used by internal code (such as the WebExtensions implementation itself) and also things like privileged add-ons including webext experiments.

The goal here would be to use `Cu.reportError` for exceptions specifically, instead of simply logging them which is the norm right now. This includes but is not limited to `getAllAddons`.

Bonus points for figuring out how to associate exceptions coming from add-ons in the debugger somehow (comment 4) but just reporting the exceptions with reportError would be an improvement.

Updated

a month ago
Mentor: rhelmer@mozilla.com
You need to log in before you can comment on or make changes to this bug.