diagnostics - rewrite bug-notifier as diagnostics-reporter

app/scripts/controllers/preferences.js

@@ -1,9 +1,7 @@
 const ObservableStore = require('obs-store')
 const normalizeAddress = require('eth-sig-util').normalize
 const extend = require('xtend')
-const notifier = require('../lib/bug-notifier')
+
-const log = require('loglevel')
-const { version } = require('../../manifest.json')
 
 class PreferencesController {
 
@@ -34,8 +32,7 @@ class PreferencesController {
       lostIdentities: {},
     }, opts.initState)
 
-    this.getFirstTimeInfo = opts.getFirstTimeInfo || null
+    this.diagnostics = opts.diagnostics
-    this.notifier = opts.notifier || notifier
 
     this.store = new ObservableStore(initState)
   }
@@ -128,17 +125,9 @@ class PreferencesController {
     if (Object.keys(newlyLost).length > 0) {
 
       // Notify our servers:
-      const uri = 'https://diagnostics.metamask.io/v1/orphanedAccounts'
+      if (this.diagnostics) this.diagnostics.reportOrphans(newlyLost)
-      const firstTimeInfo = this.getFirstTimeInfo ? this.getFirstTimeInfo() : {}
-      this.notifier.notify(uri, {
-        accounts: Object.keys(newlyLost),
-        metadata: {
-          version,
-          firstTimeInfo,
-        },
-      })
-      .catch(log.error)
 
+      // store lost accounts
       for (let key in newlyLost) {
         lostIdentities[key] = newlyLost[key]
       }

app/scripts/lib/bug-notifier.js

@@ -1,22 +0,0 @@
-class BugNotifier {
-  notify (uri, message) {
-    return postData(uri, message)
-  }
-}
-
-function postData(uri, data) {
-  return fetch(uri, {
-    body: JSON.stringify(data), // must match 'Content-Type' header
-    credentials: 'same-origin', // include, same-origin, *omit
-    headers: {
-      'content-type': 'application/json',
-    },
-    method: 'POST', // *GET, POST, PUT, DELETE, etc.
-    mode: 'cors', // no-cors, cors, *same-origin
-  })
-}
-
-const notifier = new BugNotifier()
-
-module.exports = notifier
-

app/scripts/lib/diagnostics-reporter.js

@@ -0,0 +1,71 @@
+class DiagnosticsReporter {
+
+  constructor ({ firstTimeInfo, version }) {
+    this.firstTimeInfo = firstTimeInfo
+    this.version = version
+  }
+
+  async reportOrphans(orphans) {
+    try {
+      await this.submit({
+        accounts: Object.keys(orphans),
+        metadata: {
+          type: 'orphans'
+        },
+      })
+    } catch (err) {
+      console.error('DiagnosticsReporter - "reportOrphans" encountered an error:')
+      console.error(err)
+    }
+  }
+
+  async reportMultipleKeyrings(rawKeyrings) {
+    try {
+      const keyrings = await Promise.all(rawKeyrings.map(async (keyring, index) => {
+        return {
+          index,
+          type: keyring.type,
+          accounts: await keyring.getAccounts(),
+        }
+      }))
+      await this.submit({
+        accounts: [],
+        metadata: {
+          type: 'keyrings',
+          keyrings,
+        },
+      })
+    } catch (err) {
+      console.error('DiagnosticsReporter - "reportMultipleKeyrings" encountered an error:')
+      console.error(err)
+    }
+  }
+
+  async submit (message) {
+    try {
+      // add metadata
+      message.metadata.version = this.version
+      message.metadata.firstTimeInfo = this.firstTimeInfo
+      return await postData(message)
+    } catch (err) {
+      console.error('DiagnosticsReporter - "submit" encountered an error:')
+      console.error(err)
+    }
+  }
+
+}
+
+function postData(data) {
+  const uri = 'https://diagnostics.metamask.io/v1/orphanedAccounts'
+  return fetch(uri, {
+    body: JSON.stringify(data), // must match 'Content-Type' header
+    credentials: 'same-origin', // include, same-origin, *omit
+    headers: {
+      'content-type': 'application/json',
+    },
+    method: 'POST', // *GET, POST, PUT, DELETE, etc.
+    mode: 'cors', // no-cors, cors, *same-origin
+  })
+}
+
+module.exports = DiagnosticsReporter

app/scripts/metamask-controller.js

@@ -46,7 +46,7 @@ const GWEI_BN = new BN('1000000000')
 const percentile = require('percentile')
 const seedPhraseVerifier = require('./lib/seed-phrase-verifier')
 const cleanErrorStack = require('./lib/cleanErrorStack')
-const notifier = require('./lib/bug-notifier')
+const DiagnosticsReporter = require('./lib/diagnostics-reporter')
 const log = require('loglevel')
 
 module.exports = class MetamaskController extends EventEmitter {
@@ -66,7 +66,10 @@ module.exports = class MetamaskController extends EventEmitter {
     this.recordFirstTimeInfo(initState)
 
     // metamask diagnostics reporter
-    this.notifier = opts.notifier || notifier
+    this.diagnostics = opts.diagnostics || new DiagnosticsReporter({
+      firstTimeInfo: initState.firstTimeInfo,
+      version,
+    })
 
     // platform-specific api
     this.platform = opts.platform
@@ -89,7 +92,7 @@ module.exports = class MetamaskController extends EventEmitter {
     this.preferencesController = new PreferencesController({
       initState: initState.PreferencesController,
       initLangCode: opts.initLangCode,
-      getFirstTimeInfo: () => initState.firstTimeInfo,
+      diagnostics: this.diagnostics,
     })
 
     // currency controller
@@ -492,32 +495,9 @@ module.exports = class MetamaskController extends EventEmitter {
     const accounts = await this.keyringController.getAccounts()
 
     // verify keyrings
-    try {
+    const nonSimpleKeyrings = this.keyringController.keyrings.filter(keyring => keyring.type !== 'Simple Key Pair')
-      const nonSimpleKeyrings = this.keyringController.keyrings.filter(keyring => keyring.type !== 'Simple Key Pair')
+    if (nonSimpleKeyrings.length > 1) {
-      if (nonSimpleKeyrings.length > 1) {
+      if (this.diagnostics) await this.reportMultipleKeyrings(nonSimpleKeyrings)
-        const keyrings = await Promise.all(nonSimpleKeyrings.map(async (keyring, index) => {
-          return {
-            index,
-            type: keyring.type,
-            accounts: await keyring.getAccounts()
-          }
-        }))
-        // unexpected number of keyrings, report to diagnostics
-        const uri = 'https://diagnostics.metamask.io/v1/orphanedAccounts'
-        const firstTimeInfo = this.getFirstTimeInfo ? this.getFirstTimeInfo() : {}
-        await this.notifier.notify(uri, {
-          accounts: [],
-          metadata: {
-            type: 'keyrings',
-            keyrings,
-            version,
-            firstTimeInfo,
-          },
-        })
-      }
-    } catch (err) {
-      console.error('Keyring validation error:')
-      console.error(err)
     }
 
     await this.preferencesController.syncAddresses(accounts)

test/unit/app/controllers/metamask-controller-test.js

@@ -72,11 +72,6 @@ describe('MetaMaskController', function () {
     it('removes any identities that do not correspond to known accounts.', async function () {
       const fakeAddress = '0xbad0'
       metamaskController.preferencesController.addAddresses([fakeAddress])
-      metamaskController.preferencesController.notifier = {
-        notify: async () => {
-          return true
-        },
-      }
       await metamaskController.submitPassword(password)
 
       const identities = Object.keys(metamaskController.preferencesController.store.getState().identities)