1442427 - Errors thrown in async listeners are not logged

Status: RESOLVED DUPLICATE of bug 1398672

(WebExtensions :: General, defect)

Description

[Oriol Brufau [:Oriol]]

Attachment: 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 1

[Oriol Brufau [:Oriol]]

Attachment: Bug 1442427 - Log rejected promises from webextension listeners

Review commit: https://reviewboard.mozilla.org/r/224492/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/224492/

Comment 2

[Code Review Bot [:reviewbot]]

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]

Comment 4

[Oriol Brufau [:Oriol]]

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.

Comment 5

[Shane Caraveo (:mixedpuppy)]

This one will have to go through Kris.