From 1fe7fdf4b1c2886ae3b64cd7494e9bd38221311b Mon Sep 17 00:00:00 2001 From: Juan Tejada Date: Mon, 27 Jun 2016 10:27:38 -0700 Subject: [PATCH] More sync error handling WIP --- packages/nylas-core/database-connector.js | 2 +- packages/nylas-core/extendable-error.js | 18 +++++++ packages/nylas-core/imap-connection.js | 4 +- packages/nylas-core/index.js | 1 + packages/nylas-core/models/account/message.js | 3 +- packages/nylas-core/models/shared/account.js | 8 ++- .../imap/fetch-messages-in-category.js | 1 - packages/nylas-sync/sync-worker.js | 51 ++++++++++++++----- 8 files changed, 70 insertions(+), 18 deletions(-) create mode 100644 packages/nylas-core/extendable-error.js diff --git a/packages/nylas-core/database-connector.js b/packages/nylas-core/database-connector.js index ef09e43b0..fc93922bc 100644 --- a/packages/nylas-core/database-connector.js +++ b/packages/nylas-core/database-connector.js @@ -35,7 +35,7 @@ class DatabaseConnector { _sequelizeForAccount(accountId) { if (!accountId) { - throw new Error(`You need to pass an accountId to init the database!`) + return Promise.reject(new Error(`You need to pass an accountId to init the database!`)) } const sequelize = new Sequelize(accountId, '', '', { storage: path.join(STORAGE_DIR, `a-${accountId}.sqlite`), diff --git a/packages/nylas-core/extendable-error.js b/packages/nylas-core/extendable-error.js new file mode 100644 index 000000000..b64144f32 --- /dev/null +++ b/packages/nylas-core/extendable-error.js @@ -0,0 +1,18 @@ +class ExtendableError extends Error { + constructor(message) { + super(message); + this.name = this.constructor.name; + this.message = message; + Error.captureStackTrace(this, this.constructor); + } + + toJSON() { + const obj = {} + Object.getOwnPropertyNames(this).forEach((key) => { + obj[key] = this[key]; + }); + return obj + } +} + +module.exports = ExtendableError diff --git a/packages/nylas-core/imap-connection.js b/packages/nylas-core/imap-connection.js index e1fabd2a6..70f5a0647 100644 --- a/packages/nylas-core/imap-connection.js +++ b/packages/nylas-core/imap-connection.js @@ -1,7 +1,7 @@ const Rx = require('rx') const Imap = require('imap'); const _ = require('underscore'); -const xoauth2 = require("xoauth2"); +const xoauth2 = require('xoauth2'); const EventEmitter = require('events'); @@ -273,7 +273,7 @@ class IMAPConnection extends EventEmitter { }) .catch((err) => { this._currentOperation = null; - console.error(err); + console.log(`Task errored: ${operation.description()}`) reject(err); }) .finally(() => { diff --git a/packages/nylas-core/index.js b/packages/nylas-core/index.js index 59733f444..f666976a7 100644 --- a/packages/nylas-core/index.js +++ b/packages/nylas-core/index.js @@ -7,4 +7,5 @@ module.exports = { SyncPolicy: require('./sync-policy'), SchedulerUtils: require('./scheduler-utils'), Config: require(`./config/${process.env.ENV || 'development'}`), + ExtendableError: require('./extendable-error'), } diff --git a/packages/nylas-core/models/account/message.js b/packages/nylas-core/models/account/message.js index b533870d6..5f0b27e27 100644 --- a/packages/nylas-core/models/account/message.js +++ b/packages/nylas-core/models/account/message.js @@ -39,7 +39,7 @@ module.exports = (sequelize, Sequelize) => { }, }, instanceMethods: { - fetchRaw({account, db}) { + fetchRaw: function fetchRaw({account, db}) { const settings = Object.assign({}, account.connectionSettings, account.decryptedCredentials()) return Promise.props({ category: this.getCategory(), @@ -57,6 +57,7 @@ module.exports = (sequelize, Sequelize) => { .finally(() => connection.end()) }) }, + toJSON: function toJSON() { return { id: this.id, diff --git a/packages/nylas-core/models/shared/account.js b/packages/nylas-core/models/shared/account.js index fff33f885..533f46c57 100644 --- a/packages/nylas-core/models/shared/account.js +++ b/packages/nylas-core/models/shared/account.js @@ -1,5 +1,5 @@ const crypto = require('crypto'); -const {JSONType} = require('../../database-types'); +const {JSONType, JSONARRAYType} = require('../../database-types'); const algorithm = 'aes-256-ctr'; const password = 'd6F3Efeq'; @@ -11,6 +11,7 @@ module.exports = (sequelize, Sequelize) => { connectionSettings: JSONType('connectionSettings'), connectionCredentials: Sequelize.STRING, syncPolicy: JSONType('syncPolicy'), + syncErrors: JSONARRAYType('syncErrors'), }, { classMethods: { associate: ({AccountToken}) => { @@ -24,9 +25,14 @@ module.exports = (sequelize, Sequelize) => { email_address: this.emailAddress, connection_settings: this.connectionSettings, sync_policy: this.syncPolicy, + sync_errors: this.syncErrors, } }, + errored: function errored() { + return this.syncErrors.length > 0 + }, + setCredentials: function setCredentials(json) { if (!(json instanceof Object)) { throw new Error("Call setCredentials with JSON!") diff --git a/packages/nylas-sync/imap/fetch-messages-in-category.js b/packages/nylas-sync/imap/fetch-messages-in-category.js index dbb1ddcd0..600a6adb1 100644 --- a/packages/nylas-sync/imap/fetch-messages-in-category.js +++ b/packages/nylas-sync/imap/fetch-messages-in-category.js @@ -114,7 +114,6 @@ class FetchMessagesInCategory { _fetchMessagesAndQueueForProcessing(range) { const messagesObservable = this._box.fetch(range) - // TODO Error handling here messagesObservable.subscribe(this._processMessage.bind(this)) return messagesObservable.toPromise() } diff --git a/packages/nylas-sync/sync-worker.js b/packages/nylas-sync/sync-worker.js index 11b8cd03d..1d2b8137e 100644 --- a/packages/nylas-sync/sync-worker.js +++ b/packages/nylas-sync/sync-worker.js @@ -3,12 +3,20 @@ const { IMAPConnection, PubsubConnector, DatabaseConnector, + ExtendableError, } = require('nylas-core'); const FetchCategoryList = require('./imap/fetch-category-list') const FetchMessagesInCategory = require('./imap/fetch-messages-in-category') const SyncbackTaskFactory = require('./syncback-task-factory') +class SyncAllCategoriesError extends ExtendableError { + constructor(message, failures) { + super(message) + this.failures = failures + } +} + class SyncWorker { constructor(account, db) { this._db = db; @@ -47,11 +55,8 @@ class SyncWorker { if (afterSync === 'idle') { return this.getInboxCategory() - .then((inboxCategory) => { - this._conn.openBox(inboxCategory.name).then(() => { - console.log("SyncWorker: - Idling on inbox category"); - }) - }); + .then((inboxCategory) => this._conn.openBox(inboxCategory.name)) + .then(() => console.log("SyncWorker: - Idling on inbox category")) } else if (afterSync === 'close') { console.log("SyncWorker: - Closing connection"); this._conn.end(); @@ -95,7 +100,7 @@ class SyncWorker { }); this._conn = conn; - resolve(this._conn.connect()); + return resolve(this._conn.connect()); }); } @@ -118,7 +123,7 @@ class SyncWorker { }); } - fetchMessagesInCategory() { + syncAllCategories() { const {Category} = this._db; const {folderSyncOptions} = this._account.syncPolicy; @@ -132,25 +137,47 @@ class SyncWorker { // ['[Gmail]/All Mail', '[Gmail]/Trash', '[Gmail]/Spam'].includes(cat.name) // ) + // TODO Don't accumulate errors, just bail on the first error and clear + // the queue and the connection + const failures = [] return Promise.all(categoriesToSync.map((cat) => this._conn.runOperation(new FetchMessagesInCategory(cat, folderSyncOptions)) + .catch((error) => failures.push({error, category: cat.name})) )) + .then(() => { + if (failures.length > 0) { + const error = new SyncAllCategoriesError( + `Failed to sync all categories for ${this._account.emailAddress}`, failures + ) + return Promise.reject(error) + } + return Promise.resolve() + }) }); } syncNow() { clearTimeout(this._syncTimer); - this.ensureConnection() .then(this.fetchCategoryList.bind(this)) .then(this.syncbackMessageActions.bind(this)) - .then(this.fetchMessagesInCategory.bind(this)) - // TODO Update account sync state in this error handler - .catch(console.error) + .then(this.syncAllCategories.bind(this)) + .catch((error) => { + // TODO + // Distinguish between temporary and critical errors + // Update account sync state for critical errors + // Handle connection errors separately + console.log('----------------------------------') + console.log('Erroring where you are supposed to') + console.log(error) + console.log('----------------------------------') + }) .finally(() => { this._lastSyncTime = Date.now() this.onSyncDidComplete() - .catch((error) => console.error('SyncWorker.syncNow: Unhandled error while cleaning up sync: ', error)) + .catch((error) => ( + console.error('SyncWorker.syncNow: Unhandled error while cleaning up after sync: ', error) + )) .finally(() => this.scheduleNextSync()) }); }