mirror of
https://github.com/Foundry376/Mailspring.git
synced 2025-03-16 02:03:42 +08:00
fix(config): Write config.json atomically and handle errors (#2518)
Summary: It seems that #2518 was being caused because `fs.writeFileSync` ocasionally failed and ended up writing an empty config.json file, effectively blowing away your N1 settings. The most common cause of failure for `writeFileSync` seems to be EBUSY error on win32, according to Sentry. The reason as to why EBUSY is happening so frequently on win32 is still unclear. This commit: - Adds an `atomicWriteFileSync` helper which writes to a temporary file first, and then renames the file. If writing to the temp file fails, we wont blow away our actual config.json. Also, renaming seems to be atomic across all platforms. - Adds more robust error handling and messaging to both loading and writing the file, and report them to Sentry - Automatically retries saving the file 3 times before telling the user. - Fixes https://sentry.nylas.com/sentry/edgehill/group/43112/ Test Plan: Manual Reviewers: evan, bengotow Reviewed By: bengotow Differential Revision: https://phab.nylas.com/D3245
This commit is contained in:
parent
95aa6386fc
commit
0ea35896b0
3 changed files with 51 additions and 4 deletions
|
@ -2,16 +2,21 @@ import path from 'path';
|
|||
import pathWatcher from 'pathwatcher';
|
||||
import fs from 'fs-plus';
|
||||
import {BrowserWindow, dialog, app} from 'electron';
|
||||
import {atomicWriteFileSync} from '../fs-utils'
|
||||
|
||||
let _ = require('underscore');
|
||||
_ = _.extend(_, require('../config-utils'));
|
||||
|
||||
const RETRY_SAVES = 3
|
||||
|
||||
|
||||
export default class ConfigPersistenceManager {
|
||||
constructor({configDirPath, resourcePath} = {}) {
|
||||
this.configDirPath = configDirPath;
|
||||
this.resourcePath = resourcePath;
|
||||
|
||||
this.userWantsToPreserveErrors = false
|
||||
this.saveRetries = 0
|
||||
this.configFilePath = path.join(this.configDirPath, 'config.json')
|
||||
this.settings = {};
|
||||
|
||||
|
@ -46,9 +51,15 @@ export default class ConfigPersistenceManager {
|
|||
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;
|
||||
detail += `\n\nFix the formatting of ${this.configFilePath} to resolve this error, or reset your settings to continue using N1.`
|
||||
|
||||
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',
|
||||
|
@ -88,11 +99,17 @@ export default class ConfigPersistenceManager {
|
|||
}
|
||||
})
|
||||
} catch (error) {
|
||||
this.notifyFailure("Configuration Error", `
|
||||
global.errorLogger.reportError(error)
|
||||
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'],
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -103,7 +120,27 @@ export default class ConfigPersistenceManager {
|
|||
const allSettings = {'*': this.settings};
|
||||
const allSettingsJSON = JSON.stringify(allSettings, null, 2);
|
||||
this.lastSaveTimstamp = Date.now();
|
||||
fs.writeFileSync(this.configFilePath, allSettingsJSON);
|
||||
try {
|
||||
atomicWriteFileSync(this.configFilePath, allSettingsJSON)
|
||||
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()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
saveSoon = () => {
|
||||
|
|
9
src/fs-utils.es6
Normal file
9
src/fs-utils.es6
Normal file
|
@ -0,0 +1,9 @@
|
|||
import fs from 'fs'
|
||||
import {Utils} from 'nylas-exports'
|
||||
|
||||
export function atomicWriteFileSync(filepath, content) {
|
||||
const randomId = Utils.generateTempId()
|
||||
const backupPath = `${filepath}.${randomId}.bak`
|
||||
fs.writeFileSync(backupPath, content)
|
||||
fs.renameSync(backupPath, filepath)
|
||||
}
|
|
@ -168,6 +168,7 @@ class NylasExports
|
|||
@lazyLoad "Utils", 'flux/models/utils'
|
||||
@lazyLoad "DOMUtils", 'dom-utils'
|
||||
@lazyLoad "DateUtils", 'date-utils'
|
||||
@lazyLoad "FsUtils", 'fs-utils'
|
||||
@lazyLoad "CanvasUtils", 'canvas-utils'
|
||||
@lazyLoad "RegExpUtils", 'regexp-utils'
|
||||
@lazyLoad "MenuHelpers", 'menu-helpers'
|
||||
|
|
Loading…
Reference in a new issue