Closed Bug 1442427 Opened 3 years ago Closed 3 years ago

Errors thrown in async listeners are not logged

Categories

(WebExtensions :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 1398672

People

(Reporter: Oriol, Unassigned)

Details

Attachments

(2 files)

Attached file test.xpi
Consider a webextension with

  browser.browserAction.onClicked.addListener(async function () {
    await new Promise(r => setTimeout(r, 1e3));
    browser.browserAction2.error;
  });

browser.browserAction2 does not exist, so attempting to access a property will asynchronously throw, i.e. reject the promise.

But this error does not appear in the browser console, so debugging typos is so difficult.
Comment on attachment 8955325 [details]
Bug 1442427 - Log rejected promises from webextension listeners

https://reviewboard.mozilla.org/r/224492/#review230434


Code analysis found 3 defects in this patch:
 - 3 defects found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: browser/components/extensions/ext-browserAction.js:600
(Diff revision 1)
>        browserAction: {
>          onClicked: new InputEventManager(context, "browserAction.onClicked", fire => {
> -          let listener = (event, browser) => {
> +          let listener = (event, browser) =>
>              context.withPendingBrowser(browser, () =>
> -              fire.sync(tabManager.convert(tabTracker.activeTab)));
> -          };
> +              fire.sync(tabManager.convert(tabTracker.activeTab))).catch(error => {
> +                Cu.reportError(Object.assign(new Error(""), error));

Error: Expected indentation of 14 spaces but found 16. [eslint: indent]

::: browser/components/extensions/ext-browserAction.js:601
(Diff revision 1)
>          onClicked: new InputEventManager(context, "browserAction.onClicked", fire => {
> -          let listener = (event, browser) => {
> +          let listener = (event, browser) =>
>              context.withPendingBrowser(browser, () =>
> -              fire.sync(tabManager.convert(tabTracker.activeTab)));
> -          };
> +              fire.sync(tabManager.convert(tabTracker.activeTab))).catch(error => {
> +                Cu.reportError(Object.assign(new Error(""), error));
> +                throw error;

Error: Expected indentation of 14 spaces but found 16. [eslint: indent]

::: browser/components/extensions/ext-browserAction.js:602
(Diff revision 1)
> -          let listener = (event, browser) => {
> +          let listener = (event, browser) =>
>              context.withPendingBrowser(browser, () =>
> -              fire.sync(tabManager.convert(tabTracker.activeTab)));
> -          };
> +              fire.sync(tabManager.convert(tabTracker.activeTab))).catch(error => {
> +                Cu.reportError(Object.assign(new Error(""), error));
> +                throw error;
> +              });

Error: Expected indentation of 12 spaces but found 14. [eslint: indent]
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1398672
I'm not sure which is the proper place to log the error, so in the patch I did it in various ones. Choose wherever you prefer, or somewhere else.
Attachment #8955325 - Flags: review?(mixedpuppy)
This one will have to go through Kris.
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.