1445814 - Each error should only cause Log.error to be called once

Status: RESOLVED FIXED

(Firefox :: Normandy Client, enhancement)

Description

[Michael Cooper [:mythmon]]

Thanks to the trial of browser error collection, I've learned some things about how errors are reported. For example, log that happens with level > Log.warn (for example, Log.error), gets automatically passed along to Cu.reportError (and then on to Sentry). Further, repeated calls to Cu.reportError cannot be (easily) correlated.

Because of this, we should only call log.error once per error we detect. For example, here is a case of where we do not follow this rule:

    try {
      const result = await FilterExpressions.eval(recipe.filter_expression, context);
      return !!result;
    } catch (err) {
      log.error(`Error checking filter for "${recipe.name}"`);
      log.error(`Filter: "${recipe.filter_expression}"`);
      log.error(`Error: "${err}"`);
      return false;
    }

Further, we should think carefully about our use of log.error, keeping in mind that it triggers Cu.reportError.

[0]: https://wiki.mozilla.org/Firefox/BrowserErrorCollection

Comment 1

[Michael Cooper [:mythmon]]

Attachment: Bug 1445814 - Only call log.error and Cu.reportError once per error

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

Comment 2

[:Gijs (he/him)]

Comment on attachment 8959028 [details]
Bug 1445814 - Only call log.error and Cu.reportError once per error

https://reviewboard.mozilla.org/r/227898/#review233854

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/52612b2979b1
Only call log.error and Cu.reportError once per error r=Gijs

Comment 4

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/52612b2979b1