Foundry376/Mailspring
1
1
Fork 0
mirror of https://github.com/Foundry376/Mailspring.git synced 2025-09-21 20:15:52 +08:00
Code Issues Projects Releases Packages Wiki Activity

[config] Refactor config to fail faster

Browse source 
Summary:
We're having some strange issues regarding corruption of the config
settings at runtime. This diff refactors the code to be more
straightforward and to be louder when things don't go as expected.

Test Plan: Run locally

Reviewers: juan, evan

Reviewed By: juan, evan

Differential Revision: https://phab.nylas.com/D3828
This commit is contained in:
Mark Hahnenberg 2017-02-02 14:12:08 -08:00
parent 5dea41dc34
commit 51f5ca5f9d
2 changed files with 114 additions and 135 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

191
src/browser/config-persistence-manager.es6
View file

@ -1,5 +1,4 @@
import path from 'path';
import pathWatcher from 'pathwatcher';
import fs from 'fs-plus';
import {BrowserWindow, dialog, app} from 'electron';
import {atomicWriteFileSync} from '../fs-utils'
@ -22,7 +21,6 @@ export default class ConfigPersistenceManager {
this.initializeConfigDirectory();
this.load();
this.observe();
}
initializeConfigDirectory() {
@ -43,106 +41,78 @@ export default class ConfigPersistenceManager {
fs.writeFileSync(this.configFilePath, templateConfig);
}
_showLoadErrorDialog(error) {
const message = `Failed to load "${path.basename(this.configFilePath)}"`;
let detail = (error.location) ? error.stack : error.message;
if (error instanceof SyntaxError) {
detail += `\n\nThe file ${this.configFilePath} has incorrect JSON formatting or is empty. Fix the formatting to resolve this error, or reset your settings to continue using N1.`
} else {
detail += `\n\nWe were unable to read the file ${this.configFilePath}. Make sure you have permissions to access this file, and check that the file is not open or being edited and try again.`
}
const clickedIndex = dialog.showMessageBox({
type: 'error',
message,
detail,
buttons: ['Quit', 'Try Again', 'Reset Configuration'],
});
switch (clickedIndex) {
case 0: return 'quit';
case 1: return 'tryagain';
case 2: return 'reset';
default:
throw new Error('Unknown button clicked');
}
}
load() {
this.userWantsToPreserveErrors = false;
try {
const json = JSON.parse(fs.readFileSync(this.configFilePath)) || {};
const json = JSON.parse(fs.readFileSync(this.configFilePath));
if (!json || !json['*']) {
throw new Error('config json appears empty');
}
this.settings = json['*'];
this.emitChangeEvent();
} catch (error) {
global.errorLogger.reportError(error, {event: 'Failed to load config.json'})
const message = `Failed to load "${path.basename(this.configFilePath)}"`;
let detail = (error.location) ? error.stack : error.message;
error.message = `Failed to load config.json: ${error.message}`;
global.errorLogger.reportError(error);
if (error instanceof SyntaxError) {
detail += `\n\nThe file ${this.configFilePath} has incorrect JSON formatting or is empty. Fix the formatting to resolve this error, or reset your settings to continue using N1.`
} else {
detail += `\n\nWe were unable to read the file ${this.configFilePath}. Make sure you have permissions to access this file, and check that the file is not open or being edited and try again.`
}
const clickedIndex = dialog.showMessageBox({
type: 'error',
message,
detail,
buttons: ['Quit', 'Try Again', 'Reset Configuration'],
});
if (clickedIndex === 0) {
const action = this._showLoadErrorDialog(error);
if (action === 'quit') {
this.userWantsToPreserveErrors = true;
app.quit();
} else if (clickedIndex === 1) {
this.load();
} else {
if (fs.existsSync(this.configFilePath)) {
fs.unlinkSync(this.configFilePath);
}
this.writeTemplateConfigFile();
this.load();
return;
}
}
}
loadSoon = () => {
this._loadDebounced = this._loadDebounced || _.debounce(this.load, 100);
this._loadDebounced();
}
observe() {
// watch the config file for edits. This observer needs to be
// replaced if the config file is deleted.
let watcher = null;
const watchCurrentConfigFile = () => {
try {
if (watcher) {
watcher.close();
}
watcher = pathWatcher.watch(this.configFilePath, (e) => {
if (e === 'change') {
this.loadSoon();
}
});
} catch (error) {
this.observeErrorOccurred(error);
if (action === 'tryagain') {
this.load();
return;
}
}
watchCurrentConfigFile();
// watch the config directory (non-recursive) to catch the config file
// being deleted and replaced or atomically edited.
try {
let lastctime = null;
pathWatcher.watch(this.configDirPath, () => {
fs.stat(this.configFilePath, (err, stats) => {
if (err) { return; }
if (action !== 'reset') {
throw new Error(`Unknown action: ${action}`);
}
const ctime = stats.ctime.getTime();
if (ctime !== lastctime) {
if (Math.abs(ctime - this.lastSaveTimestamp) > 2000) {
this.loadSoon();
}
watchCurrentConfigFile();
lastctime = ctime;
}
});
})
} catch (error) {
this.observeErrorOccurred(error);
if (fs.existsSync(this.configFilePath)) {
fs.unlinkSync(this.configFilePath);
}
this.writeTemplateConfigFile();
this.load();
}
}
observeErrorOccurred = (error) => {
global.errorLogger.reportError(error)
dialog.showMessageBox({
_showSaveErrorDialog() {
const clickedIndex = dialog.showMessageBox({
type: 'error',
message: 'Configuration Error',
detail: `
Unable to watch path: ${path.basename(this.configFilePath)}. Make sure you have permissions to
${this.configFilePath}. On linux there are currently problems with watch
sizes.
`,
buttons: ['Okay'],
message: `Failed to save "${path.basename(this.configFilePath)}"`,
detail: `\n\nWe were unable to save the file ${this.configFilePath}. Make sure you have permissions to access this file, and check that the file is not open or being edited and try again.`,
buttons: ['Okay', 'Try again'],
})
return ['ignore', 'retry'][clickedIndex];
}
save = () => {
@ -158,43 +128,42 @@ export default class ConfigPersistenceManager {
this.saveRetries = 0
} catch (error) {
if (this.saveRetries >= RETRY_SAVES) {
global.errorLogger.reportError(error, {event: 'Failed to save config.json'})
const clickedIndex = dialog.showMessageBox({
type: 'error',
message: `Failed to save "${path.basename(this.configFilePath)}"`,
detail: `\n\nWe were unable to save the file ${this.configFilePath}. Make sure you have permissions to access this file, and check that the file is not open or being edited and try again.`,
buttons: ['Okay', 'Try again'],
})
this.saveRetries = 0
if (clickedIndex === 1) {
this.saveSoon()
}
} else {
this.saveRetries++
this.saveSoon()
}
}
}
error.message = `Failed to save config.json: ${error.message}`
global.errorLogger.reportError(error);
const action = this._showSaveErrorDialog();
this.saveRetries = 0;
saveSoon = () => {
this._saveThrottled = this._saveThrottled || _.throttle(this.save, 100);
this._saveThrottled();
if (action === 'retry') {
this.save()
}
return;
}
this.saveRetries++
this.save()
}
}
getRawValuesString = () => {
if (!this.settings || _.isEmpty(this.settings)) {
throw new Error('this.settings is empty');
}
return JSON.stringify(this.settings);
}
setRawValue = (keyPath, value, sourceWebcontentsId) => {
if (keyPath) {
_.setValueForKeyPath(this.settings, keyPath, value);
} else {
this.settings = value;
}
setSettings = (value, sourceWebcontentsId) => {
this.settings = value;
this.emitChangeEvent({sourceWebcontentsId});
this.saveSoon();
return null;
this.save();
}
setRawValue = (keyPath, value, sourceWebcontentsId) => {
if (!keyPath) {
throw new Error('keyPath must not be false-y!');
}
_.setValueForKeyPath(this.settings, keyPath, value);
this.emitChangeEvent({sourceWebcontentsId});
this.save();
}
emitChangeEvent = ({sourceWebcontentsId} = {}) => {

58
src/config.coffee
View file

@ -10,9 +10,11 @@ Color = require './color'
if process.type is 'renderer'
app = remote.getGlobal('application')
webContentsId = remote.getCurrentWebContents().getId()
errorLogger = NylasEnv.errorLogger
else
app = global.application
webContentsId = null
errorLogger = global.errorLogger
# Essential: Used to access all of N1's configuration details.
#
@ -338,6 +340,11 @@ class Config
ipcRenderer.on 'on-config-reloaded', (event, settings) =>
@updateSettings(settings)
_logError: (prefix, error) ->
error.message = "#{prefix}: #{error.message}"
console.error(error.message)
errorLogger.reportError(error)
load: ->
@updateSettings(@getRawValues())
@ -429,22 +436,23 @@ class Config
# * `true` if the value was set.
# * `false` if the value was not able to be coerced to the type specified in the setting's schema.
set: (keyPath, value) ->
unless value is undefined
if value is undefined
value = _.valueForKeyPath(@defaultSettings, keyPath)
else
try
value = @makeValueConformToSchema(keyPath, value)
catch e
@_logError("Failed to set keyPath: #{keyPath} = #{value}", e)
return false
defaultValue = _.valueForKeyPath(this.defaultSettings, keyPath)
if (_.isEqual(defaultValue, value))
value = undefined
# Ensure that we never send anything but plain javascript objects through remote.
# Ensure that we never send anything but plain javascript objects through
# remote. Specifically, we don't want to serialize and send function bodies
# across the IPC bridge.
if _.isObject(value)
value = JSON.parse(JSON.stringify(value))
@setRawValue(keyPath, value)
true
return true
# Essential: Restore the setting at `keyPath` to its default value.
#
@ -535,7 +543,9 @@ class Config
###
updateSettings: (newSettings) =>
@settings = newSettings || {}
if !newSettings or _.isEmpty(newSettings)
throw new Error("Tried to update settings with false-y value: #{newSettings}")
@settings = newSettings
@emitChangeEvent()
getRawValue: (keyPath) ->
@ -580,7 +590,7 @@ class Config
defaults = @makeValueConformToSchema(keyPath, defaults)
@setRawDefault(keyPath, defaults)
catch e
console.warn("'#{keyPath}' could not set the default. Attempted default: #{JSON.stringify(defaults)}; Schema: #{JSON.stringify(@getSchema(keyPath))}")
@_logError("Failed to set keypath to default: #{keyPath} = #{JSON.stringify(defaults)}", e)
deepClone: (object) ->
if object instanceof Color
@ -601,23 +611,19 @@ class Config
defaults[key] = @extractDefaultsFromSchema(value) for key, value of properties
defaults
makeValueConformToSchema: (keyPath, value, options) ->
if options?.suppressException
try
@makeValueConformToSchema(keyPath, value)
catch e
undefined
else
value = @constructor.executeSchemaEnforcers(keyPath, value, schema) if schema = @getSchema(keyPath)
value
makeValueConformToSchema: (keyPath, value) ->
value = @constructor.executeSchemaEnforcers(keyPath, value, schema) if schema = @getSchema(keyPath)
value
# When the schema is changed / added, there may be values set in the config
# that do not conform to the schema. This will reset make them conform.
# that do not conform to the schema. This reset will make them conform.
resetSettingsForSchemaChange: ->
@transact =>
settings = @getRawValues()
settings = @makeValueConformToSchema(null, settings, suppressException: true)
@setRawValue(null, settings)
settings = @makeValueConformToSchema(null, settings)
if !settings or _.isEmpty(settings)
throw new Error("settings is falsey or empty")
app.configPersistenceManager.setSettings(settings, webContentsId)
return
emitChangeEvent: ->
@ -625,9 +631,13 @@ class Config
getRawValues: ->
try
return JSON.parse(app.configPersistenceManager.getRawValuesString())
catch
return {}
result = JSON.parse(app.configPersistenceManager.getRawValuesString())
if !result or _.isEmpty(result)
throw new Error('settings is falsey or empty')
return result
catch error
@_logError('Failed to parse response from getRawValuesString', error)
throw error
setRawValue: (keyPath, value) ->
app.configPersistenceManager.setRawValue(keyPath, value, webContentsId)