Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: catch and log electron-store.set errors #2547

Merged
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions src/automatic-gc.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const store = require('./common/store')
const { AUTO_GARBAGE_COLLECTOR: CONFIG_KEY } = require('./common/config-keys')
const { ipcMain } = require('electron')
const ipcMainEvents = require('./common/ipc-main-events')
const safeStoreSet = require('./utils/safe-store-set')

const gcFlag = '--enable-gc'
const isEnabled = flags => flags.some(f => f === gcFlag)
Expand All @@ -24,9 +25,14 @@ function disable () {
}
}

/**
*
* @param {string[]} newFlags
*/
function applyConfig (newFlags) {
store.set('ipfsConfig.flags', newFlags)
ipcMain.emit(ipcMainEvents.IPFS_CONFIG_CHANGED) // trigger node restart
safeStoreSet('ipfsConfig.flags', newFlags, () => {
ipcMain.emit(ipcMainEvents.IPFS_CONFIG_CHANGED) // trigger node restart
})
}

module.exports = async function () {
Expand Down
2 changes: 1 addition & 1 deletion src/common/config-keys.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/**
* Configuration key constants for the features in IPFS Desktop.
*
* @constant
* @type {Record<import('../types').CONFIG_KEYS, string>}
*/
const CONFIG_KEYS = {
Expand Down
40 changes: 30 additions & 10 deletions src/common/logger.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ logger.info(`[meta] logs can be found on ${logsPath}`)

/**
*
* @param {AnalyticsTimeOptions & import('countly-sdk-nodejs').CountlyAddEventOptions} args
* @param {AnalyticsTimeOptions & Omit<import('countly-sdk-nodejs').CountlyAddEventOptions, 'key'>} opts
*
* @returns {void} void
*/
Expand All @@ -79,11 +79,11 @@ module.exports = Object.freeze({
/**
*
* @param {string} msg
* @param {AnalyticsTimeOptions} opts
* @param {AnalyticsTimeOptions & Omit<import('countly-sdk-nodejs').CountlyAddEventOptions, 'key'>} opts
*
* @returns {AnalyticsTimeReturnValue} AnalyticsTimeReturnValue
*/
start: (msg, opts = {}) => {
start: (msg, opts) => {
const start = performance.now()
logger.info(`${msg} STARTED`)

Expand All @@ -107,19 +107,35 @@ module.exports = Object.freeze({
/**
*
* @param {string} msg
* @param {AnalyticsTimeOptions & import('countly-sdk-nodejs').CountlyAddEventOptions} opts
* @param {AnalyticsTimeOptions & Omit<import('countly-sdk-nodejs').CountlyAddEventOptions, 'key'>} [opts]
*
* @returns {void} void
*/
info: (msg, opts = {}) => {
addAnalyticsEvent({ count: 1, ...opts })
info: (msg, opts) => {
if (opts) {
addAnalyticsEvent({ count: 1, ...opts })
}

logger.info(msg)
},

error: (err) => {
Countly.log_error(err)
logger.error(err)
/**
*
* @param {Error|string} errMsg
* @param {Error|unknown} [error]
*/
error: (errMsg, error) => {
if (errMsg instanceof Error) {
Countly.log_error(errMsg)
logger.error(errMsg)
} else if (error != null && error instanceof Error) {
// errorMessage is not an instance of an error, but error is
Countly.log_error(error)
logger.error(errMsg, error)
} else {
Countly.log_error(new Error(errMsg))
logger.error(errMsg, error)
}
Comment on lines +127 to +138
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix error logging

},

warn: (msg, meta) => {
Expand All @@ -131,5 +147,9 @@ module.exports = Object.freeze({
},

logsPath,
addAnalyticsEvent
addAnalyticsEvent,
/**
* For when you want to log something without potentially emitting it to countly
*/
fileLogger: logger
})
17 changes: 13 additions & 4 deletions src/common/store.js
Original file line number Diff line number Diff line change
@@ -1,22 +1,28 @@
const electron = require('electron')
const { app } = require('electron')
const Store = require('electron-store')

const { fileLogger } = require('./logger')

/**
* @type {import('./types').DesktopPersistentStore}
*/
const defaults = {
ipfsConfig: {
type: 'go',
path: '',
flags: [
'--agent-version-suffix=desktop',
'--migrate',
'--enable-gc'
]
},
language: (electron.app || electron.remote.app).getLocale(),
experiments: {}
language: app.getLocale(),
experiments: {},
binaryPath: ''
}

const migrations = {
'>=0.11.0': store => {
fileLogger.info('Running migration: >=0.11.0')
store.delete('version')

const flags = store.get('ipfsConfig.flags', [])
Expand All @@ -26,6 +32,7 @@ const migrations = {
}
},
'>0.13.2': store => {
fileLogger.info('Running migration: >0.13.2')
const flags = store.get('ipfsConfig.flags', [])
const automaticGC = store.get('automaticGC', false)
// ensure checkbox follows cli flag config
Expand All @@ -34,6 +41,7 @@ const migrations = {
}
},
'>=0.17.0': store => {
fileLogger.info('Running migration: >=0.17.0')
let flags = store.get('ipfsConfig.flags', [])

// make sure version suffix is always present and normalized
Expand All @@ -53,6 +61,7 @@ const migrations = {
}
},
'>=0.20.6': store => {
fileLogger.info('Running migration: >=0.20.6')
let flags = store.get('ipfsConfig.flags', [])

// use default instead of hard-coded dhtclient
Expand Down
12 changes: 12 additions & 0 deletions src/common/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
export interface DesktopPersistentStore_IpfsdConfig {
path: string,
flags: string[]
}

export interface DesktopPersistentStore {

ipfsConfig: DesktopPersistentStore_IpfsdConfig,
language: string,
experiments: Record<string, boolean>,
binaryPath?: string
}
31 changes: 16 additions & 15 deletions src/custom-ipfs-binary.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const { showDialog } = require('./dialogs')
const logger = require('./common/logger')
const store = require('./common/store')
const dock = require('./utils/dock')
const safeStoreSet = require('./utils/safe-store-set')
const getCtx = require('./context')

const SETTINGS_KEY = 'binaryPath'
Expand Down Expand Up @@ -40,23 +41,23 @@ async function setCustomBinary () {
return
}

store.set(SETTINGS_KEY, filePaths[0])
safeStoreSet(SETTINGS_KEY, filePaths[0], () => {
opt = showDialog({
showDock: false,
title: i18n.t('setCustomIpfsBinarySuccess.title'),
message: i18n.t('setCustomIpfsBinarySuccess.message', { path: filePaths[0] }),
buttons: [
i18n.t('restart'),
i18n.t('close')
]
})

opt = showDialog({
showDock: false,
title: i18n.t('setCustomIpfsBinarySuccess.title'),
message: i18n.t('setCustomIpfsBinarySuccess.message', { path: filePaths[0] }),
buttons: [
i18n.t('restart'),
i18n.t('close')
]
})
logger.info(`[custom binary] updated to ${filePaths[0]}`)

logger.info(`[custom binary] updated to ${filePaths[0]}`)

if (opt === 0) {
restartIpfs()
}
if (opt === 0) {
restartIpfs()
}
})
})
}

Expand Down
5 changes: 3 additions & 2 deletions src/daemon/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const { shell } = require('electron')
const store = require('../common/store')
const logger = require('../common/logger')
const dialogs = require('./dialogs')
const safeStoreSet = require('../utils/safe-store-set')

/**
* Get repository configuration file path.
Expand Down Expand Up @@ -234,13 +235,13 @@ function migrateConfig (ipfsd) {
if (changed) {
try {
writeConfigFile(ipfsd, config)
store.set(REVISION_KEY, REVISION)
safeStoreSet(REVISION_KEY, REVISION)
} catch (err) {
logger.error(`[daemon] migrateConfig: error writing config file: ${err.message || err}`)
return
}
}
store.set(REVISION_KEY, REVISION)
safeStoreSet(REVISION_KEY, REVISION)
}

/**
Expand Down
3 changes: 2 additions & 1 deletion src/daemon/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const { STATUS } = require('./consts')
const createDaemon = require('./daemon')
const ipcMainEvents = require('../common/ipc-main-events')
const { analyticsKeys } = require('../analytics/keys')
const safeStoreSet = require('../utils/safe-store-set')
const getCtx = require('../context')

async function setupDaemon () {
Expand Down Expand Up @@ -64,7 +65,7 @@ async function setupDaemon () {
// not set.
if (!config.path || typeof config.path !== 'string') {
config.path = ipfsd.path
store.set('ipfsConfig', config)
safeStoreSet('ipfsConfig', config)
}

log.end()
Expand Down
8 changes: 5 additions & 3 deletions src/enable-namesys-pubsub.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const store = require('./common/store')
const { EXPERIMENT_PUBSUB_NAMESYS: CONFIG_KEY } = require('./common/config-keys')
const { ipcMain } = require('electron')
const ipcMainEvents = require('./common/ipc-main-events')
const safeStoreSet = require('./utils/safe-store-set')

const namesysPubsubFlag = '--enable-namesys-pubsub'
const isEnabled = flags => flags.some(f => f === namesysPubsubFlag)
Expand All @@ -25,12 +26,13 @@ function disable () {
}

function applyConfig (newFlags) {
store.set('ipfsConfig.flags', newFlags)
ipcMain.emit(ipcMainEvents.IPFS_CONFIG_CHANGED) // trigger node restart
safeStoreSet('ipfsConfig.flags', newFlags, () => {
ipcMain.emit(ipcMainEvents.IPFS_CONFIG_CHANGED) // trigger node restart
})
}

module.exports = async function () {
const activate = ({ newValue, oldValue }) => {
const activate = ({ newValue, oldValue = null }) => {
if (newValue === oldValue) return

try {
Expand Down
8 changes: 5 additions & 3 deletions src/enable-pubsub.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const store = require('./common/store')
const { EXPERIMENT_PUBSUB: CONFIG_KEY } = require('./common/config-keys')
const { ipcMain } = require('electron')
const ipcMainEvents = require('./common/ipc-main-events')
const safeStoreSet = require('./utils/safe-store-set')

const pubsubFlag = '--enable-pubsub-experiment'
const isEnabled = flags => flags.some(f => f === pubsubFlag)
Expand All @@ -25,12 +26,13 @@ function disable () {
}

function applyConfig (newFlags) {
store.set('ipfsConfig.flags', newFlags)
ipcMain.emit(ipcMainEvents.IPFS_CONFIG_CHANGED) // trigger node restart
safeStoreSet('ipfsConfig.flags', newFlags, () => {
ipcMain.emit(ipcMainEvents.IPFS_CONFIG_CHANGED) // trigger node restart
})
}

module.exports = async function () {
const activate = ({ newValue, oldValue }) => {
const activate = ({ newValue, oldValue = null }) => {
if (newValue === oldValue) return

try {
Expand Down
3 changes: 2 additions & 1 deletion src/i18n.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const ICU = require('i18next-icu')
const Backend = require('i18next-fs-backend')
const store = require('./common/store')
const ipcMainEvents = require('./common/ipc-main-events')
const safeStoreSet = require('./utils/safe-store-set')
const logger = require('./common/logger')

module.exports = async function () {
Expand Down Expand Up @@ -32,7 +33,7 @@ module.exports = async function () {
return
}

store.set('language', lang)
safeStoreSet('language', lang)

await i18n.changeLanguage(lang)
ipcMain.emit(ipcMainEvents.LANG_UPDATED, lang)
Expand Down
21 changes: 12 additions & 9 deletions src/move-repository-location.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const logger = require('./common/logger')
const { showDialog, recoverableErrorDialog, selectDirectory } = require('./dialogs')
const dock = require('./utils/dock')
const { analyticsKeys } = require('./analytics/keys')
const safeStoreSet = require('./utils/safe-store-set')

module.exports = function ({ stopIpfs, startIpfs }) {
dock.run(async () => {
Expand Down Expand Up @@ -69,23 +70,25 @@ module.exports = function ({ stopIpfs, startIpfs }) {
await fs.move(currDir, newDir)
logger.info(`[move repository] moved from ${currDir} to ${newDir}`)
} catch (err) {
logger.error(`[move repository] ${err.toString()}`)
logger.error(`[move repository] error moving from '${currDir}' to '${newDir}'`, err)
return recoverableErrorDialog(err, {
title: i18n.t('moveRepositoryFailed.title'),
message: i18n.t('moveRepositoryFailed.message', { currDir, newDir })
})
}

config.path = newDir
store.set('ipfsConfig', config)
logger.info('[move repository] configuration updated', { withAnalytics: analyticsKeys.MOVE_REPOSITORY })

showDialog({
title: i18n.t('moveRepositorySuccessDialog.title'),
message: i18n.t('moveRepositorySuccessDialog.message', { location: newDir }),
showDock: false
})
await safeStoreSet('ipfsConfig', config, async () => {
logger.info('[move repository] configuration updated', { withAnalytics: analyticsKeys.MOVE_REPOSITORY })

showDialog({
title: i18n.t('moveRepositorySuccessDialog.title'),
message: i18n.t('moveRepositorySuccessDialog.message', { location: newDir }),
showDock: false
})

await startIpfs()
await startIpfs()
})
})
}
4 changes: 2 additions & 2 deletions src/tray.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const CONFIG_KEYS = require('./common/config-keys')
const { SHORTCUT: SCREENSHOT_SHORTCUT, takeScreenshot } = require('./take-screenshot')
const { isSupported: supportsLaunchAtLogin } = require('./auto-launch')
const createToggler = require('./utils/create-toggler')
const safeStoreSet = require('./utils/safe-store-set')
const getCtx = require('./context')

function buildCheckbox (key, label) {
Expand Down Expand Up @@ -405,8 +406,7 @@ module.exports = async function () {
await setupMenu()

createToggler(CONFIG_KEYS.MONOCHROME_TRAY_ICON, async ({ newValue }) => {
store.set(CONFIG_KEYS.MONOCHROME_TRAY_ICON, newValue)
return true
return safeStoreSet(CONFIG_KEYS.MONOCHROME_TRAY_ICON, newValue, () => true)
})

ctx.setProp('tray', tray)
Expand Down
Loading