Merge pull request #3423 from overleaf/as-ta-sentry-fixes

Improve loading of Sentry reporter in frontend

GitOrigin-RevId: fc05aa48ad0e816b4ef0f5dafb6cf00525a28223

services/web/.eslintrc

@@ -84,7 +84,6 @@
         "angular": true,
         "ace": true,
         "ga": true,
-        "Raven": true, // Backwards compat for Sentry reporting
         "sl_console": true,
         "sl_debugging": true,
         // Injected in layout.pug

services/web/frontend/js/base.js

@@ -17,7 +17,7 @@
  */
 
 import './libraries'
-import './utils/sentry'
+import './infrastructure/error-reporter'
 import './modules/recursionHelper'
 import './modules/errorCatcher'
 import './modules/localStorage'

services/web/frontend/js/ide/chat/services/chatMessages.js

@@ -1,5 +1,3 @@
-/* global Raven */
-
 /* eslint-disable
     max-len,
     no-return-assign,
@@ -16,6 +14,7 @@
  * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
  */
 import App from '../../../base'
+import { captureException } from '../../../infrastructure/error-reporter'
 
 export default App.factory('chatMessages', function($http, ide) {
   const MESSAGES_URL = `/project/${ide.project_id}/messages`
@@ -81,20 +80,17 @@ export default App.factory('chatMessages', function($http, ide) {
         chat.state.atEnd = true
       }
       if (messages.reverse == null) {
-        if (typeof Raven !== 'undefined' && Raven !== null) {
-          Raven.captureException(
-            new Error(`messages has no reverse property ${typeof messages}`)
-          )
-        }
+        captureException(
+          new Error(`messages has no reverse property ${typeof messages}`)
+        )
       }
       if (typeof messages.reverse !== 'function') {
-        if (typeof Raven !== 'undefined' && Raven !== null) {
-          Raven.captureException(
-            new Error(
-              `messages.reverse not a function ${typeof messages.reverse} ${typeof messages}`
-            )
+        captureException(
+          new Error(
+            `messages.reverse not a function ${typeof messages.reverse} ${typeof messages}`
           )
-        }
+        )
+
         chat.state.errored = true
       } else {
         messages.reverse()

services/web/frontend/js/ide/pdfng/directives/pdfRenderer.js

@@ -19,6 +19,7 @@
  */
 import App from '../../../base'
 import PDFJS from './pdfJsLoader'
+import { captureMessage } from '../../../infrastructure/error-reporter'
 
 export default App.factory('PDFRenderer', function(
   $timeout,
@@ -336,11 +337,10 @@ export default App.factory('PDFRenderer', function(
           if (loadTask.cancelled) {
             return
           } // return from cancelled page load
-          __guardMethod__(window.Raven, 'captureMessage', o =>
-            o.captureMessage(
-              `pdfng page load timed out after ${this.PAGE_LOAD_TIMEOUT}ms`
-            )
+          captureMessage(
+            `pdfng page load timed out after ${this.PAGE_LOAD_TIMEOUT}ms`
           )
+
           timedOut = true
           this.clearIndicator(page)
           // @jobs = @jobs - 1

services/web/frontend/js/ide/pdfng/directives/pdfViewer.js

@@ -19,6 +19,7 @@ import _ from 'lodash'
  * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
  */
 import App from '../../../base'
+import { captureMessage } from '../../../infrastructure/error-reporter'
 import pdfTextLayer from './pdfTextLayer'
 import pdfAnnotations from './pdfAnnotations'
 import pdfHighlights from './pdfHighlights'
@@ -66,16 +67,14 @@ App.controller('pdfViewerController', function(
           return $scope.$emit('loaded')
         },
         errorCallback(error) {
-          if (window.Raven) {
-            // MissingPDFException is "expected" as the pdf file can be on a
-            // CLSI server that has been cycled out.
-            // Currently, there is NO error handling to handle this situation,
-            // but we plan to add this in the future
-            // (https://github.com/overleaf/issues/issues/2985) and this error
-            // is causing noise in Sentry so ignore it
-            if (!error.name === 'MissingPDFException') {
-              window.Raven.captureMessage(`pdfng error ${error}`)
-            }
+          // MissingPDFException is "expected" as the pdf file can be on a
+          // CLSI server that has been cycled out.
+          // Currently, there is NO error handling to handle this situation,
+          // but we plan to add this in the future
+          // (https://github.com/overleaf/issues/issues/2985) and this error
+          // is causing noise in Sentry so ignore it
+          if (!error || error.name !== 'MissingPDFException') {
+            captureMessage(`pdfng error ${error}`)
           }
           return $scope.$emit('pdf:error', error)
         },

services/web/frontend/js/infrastructure/error-boundary.js

@@ -1,16 +1,13 @@
 import React from 'react'
+import { captureException } from './error-reporter'
 import { ErrorBoundary } from 'react-error-boundary'
 
 function errorHandler(error, componentStack) {
-  if (window.Raven) {
-    Raven.captureException(error, scope => {
-      scope.setExtra('componentStack', componentStack)
-      scope.setTag('handler', 'react-error-boundary')
-      return scope
-    })
-  } else {
-    console.error(error)
-  }
+  captureException(error, scope => {
+    scope.setExtra('componentStack', componentStack)
+    scope.setTag('handler', 'react-error-boundary')
+    return scope
+  })
 }
 
 function DefaultFallbackComponent() {

services/web/frontend/js/infrastructure/error-reporter.js

@@ -0,0 +1,73 @@
+// Conditionally enable Sentry based on whether the DSN token is set
+const reporterPromise = window.ExposedSettings.sentryDsn
+  ? sentryReporter()
+  : nullReporter()
+
+function sentryReporter() {
+  return (
+    import(/* webpackMode: "eager" */ '@sentry/browser')
+      .then(Sentry => {
+        let eventCount = 0
+
+        Sentry.init({
+          dsn: window.ExposedSettings.sentryDsn,
+          release: window.ExposedSettings.sentryRelease,
+
+          // Ignore errors unless they come from our origins
+          // Adapted from: https://docs.sentry.io/platforms/javascript/#decluttering-sentry
+          whitelistUrls: [
+            new RegExp(window.ExposedSettings.sentryAllowedOriginRegex)
+          ],
+
+          ignoreErrors: [
+            // Ignore very noisy error
+            'SecurityError: Permission denied to access property "pathname" on cross-origin object',
+            // Ignore unhandled error that is "expected" - see https://github.com/overleaf/issues/issues/3321
+            /^Missing PDF/,
+            // Ignore "expected" error from aborted fetch - see https://github.com/overleaf/issues/issues/3321
+            /^AbortError/,
+            // Ignore spurious error from Ace internals - see https://github.com/overleaf/issues/issues/3321
+            'ResizeObserver loop limit exceeded',
+            'ResizeObserver loop completed with undelivered notifications.'
+          ],
+
+          beforeSend(event) {
+            // Limit number of events sent to Sentry to 100 events "per page load",
+            // (i.e. the cap will be reset if the page is reloaded). This prevent
+            // hitting their server-side event cap.
+            eventCount++
+            if (eventCount > 100) {
+              return null // Block the event from sending
+            } else {
+              return event
+            }
+          }
+        })
+
+        Sentry.setUser({ id: window.user_id })
+
+        return Sentry
+      })
+      // If Sentry fails to load, use the null reporter instead
+      .catch(nullReporter)
+  )
+}
+
+function nullReporter() {
+  return Promise.resolve({
+    captureException: error => {
+      console.error(error)
+    },
+    captureMessage: error => {
+      console.error(error)
+    }
+  })
+}
+
+export function captureException(...args) {
+  reporterPromise.then(reporter => reporter.captureException(...args))
+}
+
+export function captureMessage(...args) {
+  reporterPromise.then(reporter => reporter.captureMessage(...args))
+}

services/web/frontend/js/modules/errorCatcher.js

@@ -9,17 +9,8 @@
  * DS207: Consider shorter variations of null checks
  * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
  */
-/* eslint-disable
-    max-len,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
+import { captureException } from '../infrastructure/error-reporter'
+
 const app = angular.module('ErrorCatcher', [])
 const UNHANDLED_REJECTION_ERR_MSG = 'Possibly unhandled rejection: canceled'
 
@@ -37,16 +28,12 @@ app.config([
           ) {
             return
           }
-          if (
-            (typeof Raven !== 'undefined' && Raven !== null
-              ? Raven.captureException
-              : undefined) != null
-          ) {
-            Raven.captureException(exception, scope => {
-              scope.setTag('handler', 'angular-exception-handler')
-              return scope
-            })
-          }
+
+          captureException(exception, scope => {
+            scope.setTag('handler', 'angular-exception-handler')
+            return scope
+          })
+
           return $delegate(exception, cause)
         }
     ])

services/web/frontend/js/utils/sentry.js

@@ -1,48 +0,0 @@
-// Conditionally enable Sentry based on whether the DSN token is set
-// Conditionally enable Sentry based on whether the DSN token is set
-if (window.ExposedSettings.sentryDsn) {
-  import(/* webpackChunkName: "sentry" */ '@sentry/browser').then(Sentry => {
-    let eventCount = 0
-
-    Sentry.init({
-      dsn: window.ExposedSettings.sentryDsn,
-      release: window.ExposedSettings.sentryRelease,
-      environment: window.ExposedSettings.sentryEnvironment,
-
-      // Ignore errors unless they come from our origins
-      // Adapted from: https://docs.sentry.io/platforms/javascript/#decluttering-sentry
-      whitelistUrls: [
-        new RegExp(window.ExposedSettings.sentryAllowedOriginRegex)
-      ],
-
-      ignoreErrors: [
-        // Ignore very noisy error
-        'SecurityError: Permission denied to access property "pathname" on cross-origin object',
-        // Ignore unhandled error that is "expected" - see https://github.com/overleaf/issues/issues/3321
-        /^Missing PDF/,
-        // Ignore "expected" error from aborted fetch - see https://github.com/overleaf/issues/issues/3321
-        /^AbortError/,
-        // Ignore spurious error from Ace internals - see https://github.com/overleaf/issues/issues/3321
-        'ResizeObserver loop limit exceeded',
-        'ResizeObserver loop completed with undelivered notifications.'
-      ],
-
-      beforeSend(event) {
-        // Limit number of events sent to Sentry to 100 events "per page load",
-        // (i.e. the cap will be reset if the page is reloaded). This prevent
-        // hitting their server-side event cap.
-        eventCount++
-        if (eventCount > 100) {
-          return null // Block the event from sending
-        } else {
-          return event
-        }
-      }
-    })
-
-    Sentry.setUser({ id: window.user_id })
-
-    // Previously Raven added itself as a global, so we mimic that old behaviour
-    window.Raven = Sentry
-  })
-}

services/web/test/frontend/bootstrap.js

@@ -27,3 +27,6 @@ moment.updateLocale('en', {
 // node-fetch doesn't accept relative URL's: https://github.com/node-fetch/node-fetch/blob/master/docs/v2-LIMITS.md#known-differences
 const fetch = require('node-fetch')
 global.fetch = (url, ...options) => fetch('http://localhost' + url, ...options)
+
+// Mock global settings
+window.ExposedSettings = {}