limit sentry events (#930)

* only allow one instance of Frame, focus instance when trying to run another instance

* improve readability

* add log for requested second instance

* remove capital

* sentry => errors, move exception handling

* fix tests

* limit sentry events

* update for event rate limiting

* tweak names

* rename sentry to errors, move uncaughtExceptionHandler

* Update test/main/sentry/index.test.js

Co-authored-by: Matt Holtzman <matt.holtzman@gmail.com>

* address review comments

* add dialog import back

* move captureException to correct place

* Update index.js

* Update index.js

* Update index.js

* Update index.test.js

Co-authored-by: Matt Holtzman <matt.holtzman@gmail.com>
Co-authored-by: goosewobbler <goosewobbler@pm.me>

main/sentry/index.ts → main/errors/index.ts

@@ -1,7 +1,10 @@
 import * as Sentry from '@sentry/electron'
 import type { Event } from '@sentry/types'
+
 import store from '../store'
 
+const EVENT_RATE_LIMIT = 5
+
 function getCrashReportFields () {
   const fields = ['networks', 'networksMeta', 'tokens']
 
@@ -27,17 +30,37 @@ function getSentryExceptions (event: Event) {
   return safeExceptions
 }
 
+export function captureException (e: NodeJS.ErrnoException) {
+  captureException(e)
+}
+
 export function init () {
+  let allowedEvents = EVENT_RATE_LIMIT
+
+  setInterval(() => {
+    if (allowedEvents < EVENT_RATE_LIMIT) {
+      allowedEvents++
+    }
+  }, 60_000)
+
   Sentry.init({
     // only use IPC from renderer process, not HTTP
     ipcMode: Sentry.IPCMode.Classic,
     dsn: 'https://7b09a85b26924609bef5882387e2c4dc@o1204372.ingest.sentry.io/6331069',
-    beforeSend: (evt: Event) => ({
-      ...evt,
-      exception: { values: getSentryExceptions(evt) },
-      user: { ...evt.user, ip_address: undefined }, // remove IP address
-      tags: { ...evt.tags, 'frame.instance_id': store('main.instanceId') },
-      extra: getCrashReportFields()
-    })
+    beforeSend: (event: Event) => {
+      if (allowedEvents === 0) {
+        return null
+      }
+
+      allowedEvents--
+
+      return {
+        ...event,
+        exception: { values: getSentryExceptions(event) },
+        user: { ...event.user, ip_address: undefined }, // remove IP address
+        tags: { ...event.tags, 'frame.instance_id': store('main.instanceId') },
+        extra: getCrashReportFields()
+      }
+    }
   })
 }

main/index.js

@@ -54,10 +54,10 @@ const accounts = require('./accounts').default
 
 const launch = require('./launch')
 const updater = require('./updater')
-const sentry = require('./sentry')
+const errors = require('./errors')
 
 require('./rpc')
-sentry.init()
+errors.init()
 
 // const clients = require('./clients')
 const signers = require('./signers').default
@@ -71,11 +71,11 @@ log.info('Node: v' + process.versions.node)
 // prevent showing the exit dialog more than once
 let closing = false
 
-process.on('uncaughtException', e => {
+process.on('uncaughtException', (e) => {
   log.error('uncaughtException', e)
 
-  Sentry.captureException(e)
-
+  errors.captureException(e)
+  
   if (e.code === 'EPIPE') {
     log.error('uncaught EPIPE error', e)
     return
@@ -86,6 +86,7 @@ process.on('uncaughtException', e => {
 
     showUnhandledExceptionDialog(e.message, e.code)
   }
+
 })
 
 const externalWhitelist = [
@@ -326,6 +327,10 @@ ipcMain.on('tray:action', (e, action, ...args) => {
   log.info('Tray sent unrecognized action: ', action)
 })
 
+app.on('second-instance', (event, argv, workingDirectory) => {
+  log.info(`second instance requested from directory: ${workingDirectory}`)
+  windows.showTray()
+})
 app.on('activate', () => windows.activate())
 app.on('will-quit', () => app.quit())
 app.on('quit', async () => {

test/main/sentry/index.test.js → test/main/errors/index.test.js

@@ -1,11 +1,36 @@
 import * as Sentry from '@sentry/electron'
-import { init as initSentry } from '../../../main/sentry'
+import { initSentry } from '../../../main/errors'
 
 jest.mock('@sentry/electron', () => ({ init: jest.fn(), IPCMode: { Classic: 'test-ipcmode' } }))
 jest.mock('../../../main/store')
 
+beforeAll(() => {
+  jest.useFakeTimers()
+})
+
+afterAll(() => {
+  jest.useRealTimers()
+})
+
 describe('sentry', () => {
   const mockEvent = (event) => Sentry.init.mock.calls[0][0].beforeSend(event)
+  const mockEvents = (events) => events.map(mockEvent)
+  const validEvent = {
+    exception: {
+      values: [],
+    },
+    extra: {
+      networks: '{}',
+      networksMeta: '{}',
+      tokens: '{}',
+    },
+    tags: {
+      "frame.instance_id": undefined,
+    },
+    user: {
+      ip_address: undefined,
+    }
+  }
 
   it('should initialize sentry with the expected object', () => {
     initSentry()
@@ -17,7 +42,7 @@ describe('sentry', () => {
   })
 
   it('should strip asar paths from stackframe modules', () => {
-    initSentry();
+    initSentry()
     const sentryEvent = mockEvent({
       exception: {
         values: [{
@@ -42,4 +67,39 @@ describe('sentry', () => {
       "{asar}/compiled/main/signers/lattice/Lattice/index"
     ])
   })
+
+  it('should drop events once the rate limit has been reached', () => {
+    initSentry()
+    const sentEvents = mockEvents(Array(10).fill({}))
+
+    // after the limit is reached, this function will return a falsy value rather than the actual event
+    const reportedEvents = sentEvents.filter(evt => !!evt)
+    expect(reportedEvents).toStrictEqual(Array(5).fill(validEvent))
+  })
+
+  it('should send events after the rate limit recovery period has elapsed', () => {
+    const events = Array(5).fill({})
+    initSentry()
+    mockEvents(events)
+    jest.advanceTimersByTime(60_000)
+    expect(mockEvents(events)).toStrictEqual(
+      [validEvent, null, null, null, null]
+    )
+    jest.advanceTimersByTime(2 * 60_000)
+    expect(mockEvents(events)).toStrictEqual(
+      [validEvent, validEvent, null, null, null]
+    )
+    jest.advanceTimersByTime(3 * 60_000)
+    expect(mockEvents(events)).toStrictEqual(
+      [validEvent, validEvent, validEvent, null, null]
+    )
+    jest.advanceTimersByTime(4 * 60_000)
+    expect(mockEvents(events)).toStrictEqual(
+      [validEvent, validEvent, validEvent, validEvent, null]
+    )
+    jest.advanceTimersByTime(100 * 60_000)
+    expect(mockEvents(events)).toStrictEqual(
+      [validEvent, validEvent, validEvent, validEvent, validEvent]
+    )
+  })
 })