From 20bdba3d1710a070d06c2a395f92d948b9396d47 Mon Sep 17 00:00:00 2001 From: kumavis Date: Tue, 5 Jun 2018 11:51:27 -0700 Subject: [PATCH] diagnostics - rewrite bug-notifier as diagnostics-reporter --- app/scripts/controllers/preferences.js | 19 ++--- app/scripts/lib/bug-notifier.js | 22 ------ app/scripts/lib/diagnostics-reporter.js | 71 +++++++++++++++++++ app/scripts/metamask-controller.js | 38 +++------- .../controllers/metamask-controller-test.js | 5 -- 5 files changed, 84 insertions(+), 71 deletions(-) delete mode 100644 app/scripts/lib/bug-notifier.js create mode 100644 app/scripts/lib/diagnostics-reporter.js diff --git a/app/scripts/controllers/preferences.js b/app/scripts/controllers/preferences.js index 2fe009f9a..a5d8cc27b 100644 --- a/app/scripts/controllers/preferences.js +++ b/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.notifier = opts.notifier || notifier + this.diagnostics = opts.diagnostics 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' - const firstTimeInfo = this.getFirstTimeInfo ? this.getFirstTimeInfo() : {} - this.notifier.notify(uri, { - accounts: Object.keys(newlyLost), - metadata: { - version, - firstTimeInfo, - }, - }) - .catch(log.error) + if (this.diagnostics) this.diagnostics.reportOrphans(newlyLost) + // store lost accounts for (let key in newlyLost) { lostIdentities[key] = newlyLost[key] } diff --git a/app/scripts/lib/bug-notifier.js b/app/scripts/lib/bug-notifier.js deleted file mode 100644 index 4d305b894..000000000 --- a/app/scripts/lib/bug-notifier.js +++ /dev/null @@ -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 - diff --git a/app/scripts/lib/diagnostics-reporter.js b/app/scripts/lib/diagnostics-reporter.js new file mode 100644 index 000000000..6c77923bd --- /dev/null +++ b/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 diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index ad1d6d6a7..4b0b00306 100644 --- a/app/scripts/metamask-controller.js +++ b/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') - if (nonSimpleKeyrings.length > 1) { - 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) + const nonSimpleKeyrings = this.keyringController.keyrings.filter(keyring => keyring.type !== 'Simple Key Pair') + if (nonSimpleKeyrings.length > 1) { + if (this.diagnostics) await this.reportMultipleKeyrings(nonSimpleKeyrings) } await this.preferencesController.syncAddresses(accounts) diff --git a/test/unit/app/controllers/metamask-controller-test.js b/test/unit/app/controllers/metamask-controller-test.js index 7ec98766a..266c3f258 100644 --- a/test/unit/app/controllers/metamask-controller-test.js +++ b/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)