From 2b880dd4e060f8f7f95afe9ff2a3e2e6d540c922 Mon Sep 17 00:00:00 2001 From: kumavis Date: Thu, 5 Apr 2018 13:15:08 -0700 Subject: [PATCH 1/3] migrations - report migrations errors to sentry with vault structure --- app/scripts/background.js | 20 +++++++++++++++++- app/scripts/lib/getObjStructure.js | 33 ++++++++++++++++++++++++++++++ app/scripts/lib/migrator/index.js | 24 ++++++++++++++++++---- app/scripts/migrations/024.js | 14 +++++-------- 4 files changed, 77 insertions(+), 14 deletions(-) create mode 100644 app/scripts/lib/getObjStructure.js diff --git a/app/scripts/background.js b/app/scripts/background.js index 3ad0a7863..ec586f642 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -20,6 +20,7 @@ const reportFailedTxToSentry = require('./lib/reportFailedTxToSentry') const setupMetamaskMeshMetrics = require('./lib/setupMetamaskMeshMetrics') const EdgeEncryptor = require('./edge-encryptor') const getFirstPreferredLangCode = require('./lib/get-first-preferred-lang-code') +const getObjStructure = require('./lib/getObjStructure') const STORAGE_KEY = 'metamask-config' const METAMASK_DEBUG = process.env.METAMASK_DEBUG @@ -77,6 +78,16 @@ async function loadStateFromPersistence () { diskStore.getState() || migrator.generateInitialState(firstTimeState) + // report migration errors to sentry + migrator.on('error', (err) => { + // get vault structure without secrets + const vaultStructure = getObjStructure(versionedData) + raven.captureException(err, { + // "extra" key is required by Sentry + extra: { vaultStructure }, + }) + }) + // migrate data versionedData = await migrator.migrateData(versionedData) if (!versionedData) { @@ -84,7 +95,14 @@ async function loadStateFromPersistence () { } // write to disk - if (localStore.isSupported) localStore.set(versionedData) + if (localStore.isSupported) { + localStore.set(versionedData) + } else { + // throw in setTimeout so as to not block boot + setTimeout(() => { + throw new Error('MetaMask - Localstore not supported') + }) + } // return just the data return versionedData.data diff --git a/app/scripts/lib/getObjStructure.js b/app/scripts/lib/getObjStructure.js new file mode 100644 index 000000000..3db389507 --- /dev/null +++ b/app/scripts/lib/getObjStructure.js @@ -0,0 +1,33 @@ +const clone = require('clone') + +module.exports = getObjStructure + +// This will create an object that represents the structure of the given object +// it replaces all values with the result of their type + +// { +// "data": { +// "CurrencyController": { +// "conversionDate": "number", +// "conversionRate": "number", +// "currentCurrency": "string" +// } +// } + +function getObjStructure(obj) { + const structure = clone(obj) + return deepMap(structure, (value) => { + return value === null ? 'null' : typeof value + }) +} + +function deepMap(target = {}, visit) { + Object.entries(target).forEach(([key, value]) => { + if (typeof value === 'object' && value !== null) { + target[key] = deepMap(value, visit) + } else { + target[key] = visit(value) + } + }) + return target +} diff --git a/app/scripts/lib/migrator/index.js b/app/scripts/lib/migrator/index.js index 4fd2cae92..203b2ddc9 100644 --- a/app/scripts/lib/migrator/index.js +++ b/app/scripts/lib/migrator/index.js @@ -1,6 +1,9 @@ -class Migrator { +const EventEmitter = require('events') + +class Migrator extends EventEmitter { constructor (opts = {}) { + super() const migrations = opts.migrations || [] // sort migrations by version this.migrations = migrations.sort((a, b) => a.version - b.version) @@ -12,13 +15,26 @@ class Migrator { // run all pending migrations on meta in place async migrateData (versionedData = this.generateInitialState()) { + // get all migrations that have not yet been run const pendingMigrations = this.migrations.filter(migrationIsPending) + // perform each migration for (const index in pendingMigrations) { const migration = pendingMigrations[index] - versionedData = await migration.migrate(versionedData) - if (!versionedData.data) throw new Error('Migrator - migration returned empty data') - if (versionedData.version !== undefined && versionedData.meta.version !== migration.version) throw new Error('Migrator - Migration did not update version number correctly') + try { + // attempt migration and validate + const migratedData = await migration.migrate(versionedData) + if (!migratedData.data) throw new Error('Migrator - migration returned empty data') + if (migratedData.version !== undefined && migratedData.meta.version !== migration.version) throw new Error('Migrator - Migration did not update version number correctly') + // accept the migration as good + versionedData = migratedData + } catch (err) { + // emit error instead of throw so as to not break the run (gracefully fail) + const error = new Error(`MetaMask Migration Error #${version}:\n${err.stack}`) + this.emit('error', error) + // stop migrating and use state as is + return versionedData + } } return versionedData diff --git a/app/scripts/migrations/024.js b/app/scripts/migrations/024.js index 7a0391805..c917a167c 100644 --- a/app/scripts/migrations/024.js +++ b/app/scripts/migrations/024.js @@ -13,17 +13,13 @@ const clone = require('clone') module.exports = { version, - migrate: function (originalVersionedData) { + migrate: async function (originalVersionedData) { const versionedData = clone(originalVersionedData) versionedData.meta.version = version - try { - const state = versionedData.data - const newState = transformState(state) - versionedData.data = newState - } catch (err) { - console.warn(`MetaMask Migration #${version}` + err.stack) - } - return Promise.resolve(versionedData) + const state = versionedData.data + const newState = transformState(state) + versionedData.data = newState + return versionedData }, } From 7fdf663ea7ae4b3c6bb5cdefb1f33729f5cf4422 Mon Sep 17 00:00:00 2001 From: kumavis Date: Thu, 5 Apr 2018 13:21:00 -0700 Subject: [PATCH 2/3] migrator - fix typo --- app/scripts/lib/migrator/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/scripts/lib/migrator/index.js b/app/scripts/lib/migrator/index.js index 203b2ddc9..ea9af3c80 100644 --- a/app/scripts/lib/migrator/index.js +++ b/app/scripts/lib/migrator/index.js @@ -30,7 +30,7 @@ class Migrator extends EventEmitter { versionedData = migratedData } catch (err) { // emit error instead of throw so as to not break the run (gracefully fail) - const error = new Error(`MetaMask Migration Error #${version}:\n${err.stack}`) + const error = new Error(`MetaMask Migration Error #${migration.version}:\n${err.stack}`) this.emit('error', error) // stop migrating and use state as is return versionedData From ffc71ff7d2c27d419bff4ca127ed5219bf9261c3 Mon Sep 17 00:00:00 2001 From: kumavis Date: Thu, 5 Apr 2018 13:38:34 -0700 Subject: [PATCH 3/3] migrator - dont overwrite error stack and warn to console --- app/scripts/lib/migrator/index.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/app/scripts/lib/migrator/index.js b/app/scripts/lib/migrator/index.js index ea9af3c80..85c2717ea 100644 --- a/app/scripts/lib/migrator/index.js +++ b/app/scripts/lib/migrator/index.js @@ -29,9 +29,12 @@ class Migrator extends EventEmitter { // accept the migration as good versionedData = migratedData } catch (err) { + // rewrite error message to add context without clobbering stack + const originalErrorMessage = err.message + err.message = `MetaMask Migration Error #${migration.version}: ${originalErrorMessage}` + console.warn(err.stack) // emit error instead of throw so as to not break the run (gracefully fail) - const error = new Error(`MetaMask Migration Error #${migration.version}:\n${err.stack}`) - this.emit('error', error) + this.emit('error', err) // stop migrating and use state as is return versionedData }