core(tsc): add type checking to sentry usage (#5993)

lighthouse-cli/bin.js

@@ -107,7 +107,6 @@ function run() {
       }
     })
     .then(_ => {
-      // @ts-ignore TODO(bckenny): Sentry type checking
       Sentry.init({
         url,
         flags: cliFlags,

lighthouse-core/audits/byte-efficiency/offscreen-images.js

@@ -181,7 +181,6 @@ class OffscreenImages extends ByteEfficiencyAudit {
 
       if (processed instanceof Error) {
         warnings.push(processed.message);
-        // @ts-ignore TODO(bckenny): Sentry type checking
         Sentry.captureException(processed, {tags: {audit: this.meta.id}, level: 'warning'});
         return results;
       }

lighthouse-core/audits/byte-efficiency/uses-responsive-images.js

@@ -105,7 +105,6 @@ class UsesResponsiveImages extends ByteEfficiencyAudit {
 
       if (processed instanceof Error) {
         warnings.push(processed.message);
-        // @ts-ignore TODO(bckenny): Sentry type checking
         Sentry.captureException(processed, {tags: {audit: this.meta.id}, level: 'warning'});
         return;
       }

lighthouse-core/audits/dobetterweb/no-vulnerable-libraries.js

@@ -15,7 +15,6 @@
 const Audit = require('../audit');
 const Sentry = require('../../lib/sentry');
 const semver = require('semver');
-// @ts-ignore - json require
 const snykDatabase = require('../../../third-party/snyk/snapshot.json');
 
 const SEMVER_REGEX = /^(\d+\.\d+\.\d+)[^-0-9]+/;
@@ -93,7 +92,6 @@ class NoVulnerableLibrariesAudit extends Audit {
     } catch (err) {
       err.pkgName = lib.npmPkgName;
       // Report the failure and skip this library if the version was ill-specified
-      // @ts-ignore TODO(bckenny): Sentry type checking
       Sentry.captureException(err, {level: 'warning'});
       return [];
     }

lighthouse-core/gather/computed/trace-of-tab.js

@@ -116,7 +116,6 @@ class TraceOfTab extends ComputedArtifact {
     // However, if no candidates were found (a bogus trace, likely), we fail.
     if (!firstMeaningfulPaint) {
       // Track this with Sentry since it's likely a bug we should investigate.
-      // @ts-ignore TODO(bckenny): Sentry type checking
       Sentry.captureMessage('No firstMeaningfulPaint found, using fallback', {level: 'warning'});
 
       const fmpCand = 'firstMeaningfulPaintCandidate';

lighthouse-core/gather/gatherers/dobetterweb/optimized-images.js

@@ -194,7 +194,6 @@ class OptimizedImages extends Gatherer {
       } catch (err) {
         // Track this with Sentry since these errors aren't surfaced anywhere else, but we don't
         // want to tank the entire run due to a single image.
-        // @ts-ignore TODO(bckenny): Sentry type checking
         Sentry.captureException(err, {
           tags: {gatherer: 'OptimizedImages'},
           extra: {imageUrl: URL.elideDataURI(record.url)},

lighthouse-core/gather/gatherers/dobetterweb/response-compression.js

@@ -103,7 +103,6 @@ class ResponseCompression extends Gatherer {
           });
         });
       }).catch(err => {
-        // @ts-ignore TODO(bckenny): Sentry type checking
         Sentry.captureException(err, {
           tags: {gatherer: 'ResponseCompression'},
           extra: {url: URL.elideDataURI(record.url)},

lighthouse-core/gather/gatherers/fonts.js

@@ -211,18 +211,18 @@ class Fonts extends Gatherer {
     return fontData.then(([loadedFonts, fontsAndErrors]) => {
       // Filter out errors from retrieving data on font faces.
       const fontFaces = /** @type {Array<LH.Artifacts.Font>} */ (fontsAndErrors.filter(
-        fontOrError => {
-          // Abuse the type system a bit since `err` property isn't common between types.
-          const dataError = /** @type {FontGatherError} */ (fontOrError);
-          if (dataError.err) {
-            const err = new Error(dataError.err.message);
-            err.stack = dataError.err.stack || err.stack;
-            // @ts-ignore TODO(bckenny): Sentry type checking
-            Sentry.captureException(err, {tags: {gatherer: 'Fonts'}, level: 'warning'});
-            return false;
-          }
-          return true;
-        }));
+        fontOrError => !('err' in fontOrError)));
+
+      const firstFontError = fontsAndErrors.find(fontOrError => 'err' in fontOrError);
+      if (firstFontError) {
+        // Abuse the type system a bit since `err` property isn't common between types.
+        const dataError = /** @type {FontGatherError} */ (firstFontError);
+        if (dataError.err) {
+          const err = new Error(dataError.err.message);
+          err.stack = dataError.err.stack || err.stack;
+          Sentry.captureException(err, {tags: {gatherer: 'Fonts'}, level: 'warning'});
+        }
+      }
 
       return loadedFonts.map(loadedFont => {
         const fontFaceItem = this._findSameFontFamily(loadedFont, fontFaces);

lighthouse-core/lib/arbitrary-equality-map.js

@@ -5,7 +5,6 @@
  */
 'use strict';
 
-// @ts-ignore
 const isEqual = require('lodash.isequal');
 
 /**

lighthouse-core/lib/i18n.js

@@ -25,9 +25,7 @@ const MESSAGE_INSTANCE_ID_REGEX = /(.* \| .*) # (\d+)$/;
     // In browser environments where we don't need the polyfill, this won't exist
     if (!IntlPolyfill.NumberFormat) return;
 
-    // @ts-ignore
     Intl.NumberFormat = IntlPolyfill.NumberFormat;
-    // @ts-ignore
     Intl.DateTimeFormat = IntlPolyfill.DateTimeFormat;
   } catch (_) {
     log.warn('i18n', 'Failed to install `intl` polyfill');

lighthouse-core/lib/locales/index.js

@@ -18,7 +18,7 @@
 /** @type {Record<LH.Locale, LocaleMessages>} */
 const locales = {
   'en-US': require('./en-US.json'), // The 'source' strings, with descriptions
-  // @ts-ignore - tsc bug, something about en/en-US pointing to same file
+  // @ts-ignore - tsc bug, something about en/en-US pointing to same file. https://github.com/Microsoft/TypeScript/issues/26307
   'en': require('./en-US.json'), // According to CLDR/ICU, 'en' == 'en-US' dates/numbers (Why?!)
 
   // TODO: en-GB has just ~10 messages that are different from en-US. We should only ship those.

lighthouse-core/lib/sentry.js

@@ -3,94 +3,105 @@
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
  * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
  */
-// @ts-nocheck
 'use strict';
 
 const log = require('lighthouse-logger');
 
-// eslint-disable-next-line max-len
+/** @typedef {import('raven').CaptureOptions} CaptureOptions */
+/** @typedef {import('raven').ConstructorOptions} ConstructorOptions */
+
 const SENTRY_URL = 'https://a6bb0da87ee048cc9ae2a345fc09ab2e:63a7029f46f74265981b7e005e0f69f8@sentry.io/174697';
 
-const noop = () => Promise.resolve();
-const sentryApi = {
-  captureMessage: noop,
-  captureException: noop,
-  captureBreadcrumb: noop,
-  mergeContext: noop,
-  getContext: noop,
-};
+// Per-run chance of capturing errors (if enabled).
+const SAMPLE_RATE = 0.01;
 
+/** @type {Array<{pattern: RegExp, rate: number}>} */
 const SAMPLED_ERRORS = [
-  // Error code based sampling
-  {pattern: /DOCUMENT_REQUEST$/, rate: 0.1},
-  {pattern: /(IDLE_PERIOD|FMP_TOO_LATE)/, rate: 0.1},
-  {pattern: /^NO_.*/, rate: 0.1},
-  // Message based sampling
-  {pattern: /Could not load stylesheet/, rate: 0.01},
-  {pattern: /Failed to decode/, rate: 0.1},
-  {pattern: /All image optimizations failed/, rate: 0.1},
-  {pattern: /No.*resource with given/, rate: 0.01},
-  {pattern: /No.*node with given id/, rate: 0.01},
+  // Error code based sampling. Delete if still unused after 2019-01-01.
+  // e.g.: {pattern: /No.*node with given id/, rate: 0.01},
 ];
 
+const noop = () => {};
+
 /**
- * We'll create a delegate for sentry so that environments without error reporting enabled will use
+ * A delegate for sentry so that environments without error reporting enabled will use
  * noop functions and environments with error reporting will call the actual Sentry methods.
  */
-const sentryDelegate = Object.assign({}, sentryApi);
-sentryDelegate.init = function init(opts) {
+const sentryDelegate = {
+  init,
+  /** @type {(message: string, options?: CaptureOptions) => void} */
+  captureMessage: noop,
+  /** @type {(breadcrumb: any) => void} */
+  captureBreadcrumb: noop,
+  /** @type {() => any} */
+  getContext: noop,
+  /** @type {(error: Error, options?: CaptureOptions) => Promise<void>} */
+  captureException: async () => {},
+};
+
+/**
+ * When called, replaces noops with actual Sentry implementation.
+ * @param {{url: string, flags: LH.CliFlags, environmentData: ConstructorOptions}} opts
+ */
+function init(opts) {
+  // If error reporting is disabled, leave the functions as a noop
   if (!opts.flags.enableErrorReporting) {
-    // If error reporting is disabled, leave the functions as a noop
     return;
   }
 
-  const environmentData = opts.environmentData || {};
-  const sentryConfig = Object.assign({}, environmentData, {captureUnhandledRejections: true});
+  // If not selected for samping, leave the functions as a noop.
+  if (SAMPLE_RATE <= Math.random()) {
+    return;
+  }
 
   try {
     const Sentry = require('raven');
+    const sentryConfig = Object.assign({}, opts.environmentData,
+      {captureUnhandledRejections: true});
     Sentry.config(SENTRY_URL, sentryConfig).install();
-    Object.keys(sentryApi).forEach(functionName => {
-      // Have each delegate function call the corresponding sentry function by default
-      sentryDelegate[functionName] = (...args) => Sentry[functionName](...args);
-    });
 
-    // Special case captureException to return a Promise so we don't process.exit too early
-    sentryDelegate.captureException = (err, opts) => {
-      opts = opts || {};
+    // Have each delegate function call the corresponding sentry function by default
+    sentryDelegate.captureMessage = (...args) => Sentry.captureMessage(...args);
+    sentryDelegate.captureBreadcrumb = (...args) => Sentry.captureBreadcrumb(...args);
+    sentryDelegate.getContext = () => Sentry.getContext();
 
-      const empty = Promise.resolve();
+    // Special case captureException to return a Promise so we don't process.exit too early
+    sentryDelegate.captureException = async (err, opts = {}) => {
       // Ignore if there wasn't an error
-      if (!err) return empty;
+      if (!err) return;
+
       // Ignore expected errors
-      if (err.expected) return empty;
-      // Sample known errors that occur at a high frequency
+      // @ts-ignore Non-standard property added to flag error as not needing capturing.
+      if (err.expected) return;
+
+      // Sample known errors that occur at a high frequency.
       const sampledErrorMatch = SAMPLED_ERRORS.find(sample => sample.pattern.test(err.message));
-      if (sampledErrorMatch && sampledErrorMatch.rate <= Math.random()) return empty;
+      if (sampledErrorMatch && sampledErrorMatch.rate <= Math.random()) return;
+
       // Protocol errors all share same stack trace, so add more to fingerprint
+      // @ts-ignore - properties added to protocol method LHErrors.
       if (err.protocolMethod) {
+        // @ts-ignore - properties added to protocol method LHErrors.
         opts.fingerprint = ['{{ default }}', err.protocolMethod, err.protocolError];
       }
 
       return new Promise(resolve => {
         Sentry.captureException(err, opts, () => resolve());
       });
     };
+
+    const context = Object.assign({
+      url: opts.url,
+      deviceEmulation: !opts.flags.disableDeviceEmulation,
+      throttlingMethod: opts.flags.throttlingMethod,
+    }, opts.flags.throttling);
+    Sentry.mergeContext({extra: Object.assign({}, opts.environmentData.extra, context)});
   } catch (e) {
     log.warn(
       'sentry',
       'Could not load raven library, errors will not be reported.'
     );
   }
-
-  const context = Object.assign({
-    url: opts.url,
-    deviceEmulation: !opts.flags.disableDeviceEmulation,
-    throttlingMethod: opts.flags.throttlingMethod,
-  }, opts.flags.throttling);
-
-  sentryDelegate.mergeContext({extra: Object.assign({}, environmentData.extra, context)});
-  return context;
-};
+}
 
 module.exports = sentryDelegate;

lighthouse-core/runner.js

@@ -40,11 +40,9 @@ class Runner {
       const lighthouseRunWarnings = [];
 
       const sentryContext = Sentry.getContext();
-      // @ts-ignore TODO(bckenny): Sentry type checking
       Sentry.captureBreadcrumb({
         message: 'Run started',
         category: 'lifecycle',
-        // @ts-ignore TODO(bckenny): Sentry type checking
         data: sentryContext && sentryContext.extra,
       });
 
@@ -150,7 +148,6 @@ class Runner {
       const report = generateReport(lhr, settings.output);
       return {lhr, artifacts, report};
     } catch (err) {
-      // @ts-ignore TODO(bckenny): Sentry type checking
       await Sentry.captureException(err, {level: 'fatal'});
       throw err;
     }
@@ -249,7 +246,6 @@ class Runner {
           // @ts-ignore An artifact *could* be an Error, but caught here, so ignore elsewhere.
           const artifactError = artifacts[artifactName];
 
-          // @ts-ignore TODO(bckenny): Sentry type checking
           Sentry.captureException(artifactError, {
             tags: {gatherer: artifactName},
             level: 'error',
@@ -283,7 +279,6 @@ class Runner {
         throw err;
       }
 
-      // @ts-ignore TODO(bckenny): Sentry type checking
       Sentry.captureException(err, {tags: {audit: audit.meta.id}, level: 'error'});
       // Non-fatal error become error audit result.
       const errorMessage = err.friendlyMessage ?

lighthouse-viewer/app/src/lighthouse-report-viewer.js

@@ -56,7 +56,6 @@ class LighthouseReportViewer {
    * @private
    */
   _addEventListeners() {
-    // @ts-ignore - tsc thinks document can't listen for `paste`
     document.addEventListener('paste', this._onPaste);
 
     const gistUrlInput = find('.js-gist-url', document);

package.json

@@ -77,6 +77,7 @@
     "@types/lodash.isequal": "^4.5.2",
     "@types/node": "*",
     "@types/opn": "^3.0.28",
+    "@types/raven": "^2.5.1",
     "@types/semver": "^5.5.0",
     "@types/update-notifier": "^1.0.2",
     "@types/ws": "^4.0.1",

yarn.lock

@@ -99,6 +99,13 @@
   dependencies:
     "@types/node" "*"
 
+"@types/raven@^2.5.1":
+  version "2.5.1"
+  resolved "https://registry.yarnpkg.com/@types/raven/-/raven-2.5.1.tgz#62ef0a59e29691945e1f295b62ed199619cbd9b6"
+  dependencies:
+    "@types/events" "*"
+    "@types/node" "*"
+
 "@types/rimraf@^0.0.28":
   version "0.0.28"
   resolved "https://registry.yarnpkg.com/@types/rimraf/-/rimraf-0.0.28.tgz#5562519bc7963caca8abf7f128cae3b594d41d06"