From 590b17ae6f9dd6517caba6ce42debfd8920595af Mon Sep 17 00:00:00 2001 From: Juan Tejada Date: Fri, 31 Mar 2017 10:17:51 -0700 Subject: [PATCH] [client-app] Don't imeout removing sync worker when removing an account Summary: Most of the times when removing a sync worker (i.e. when removing an account), we would see database errors in the console. This happened because more often than not we would interrupt in the middle of message processing, but `processMessage` is not interruptible, which means that the worker would be interrupted right after processing its current message. However, if `processMessage` would take more than 500ms (the current timeout for stopping the worker), we would destroy the database before processing was done and it would throw a bunch of errors. To fix this, we just don't set a timeout when removing the worker as a consequence of removing an account. However, when we are removing the worker when we detect that it is stuck, we set a time out of 5 seconds. Test Plan: manually test removing accounts, verify that it doesn't error Reviewers: mark, halla, evan Reviewed By: halla, evan Differential Revision: https://phab.nylas.com/D4308 --- .../src/local-sync-worker/sync-process-manager.es6 | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/packages/client-sync/src/local-sync-worker/sync-process-manager.es6 b/packages/client-sync/src/local-sync-worker/sync-process-manager.es6 index 931cfb9d5..93b0e829f 100644 --- a/packages/client-sync/src/local-sync-worker/sync-process-manager.es6 +++ b/packages/client-sync/src/local-sync-worker/sync-process-manager.es6 @@ -109,7 +109,7 @@ class SyncProcessManager { }) global.Logger.log(`SyncProcessManager: Detected stuck worker for account ${accountId}`, activity, time) - await this.removeWorkerForAccountId(accountId) + await this.removeWorkerForAccountId(accountId, {timeout: 5 * 1000}) const {Account} = await LocalDatabaseConnector.forShared(); const account = await Account.findById(accountId) await this.addWorkerForAccount(account) @@ -175,14 +175,9 @@ class SyncProcessManager { } } - async removeWorkerForAccountId(accountId) { + async removeWorkerForAccountId(accountId, {timeout} = {}) { if (this._workersByAccountId[accountId]) { - try { - await this._workersByAccountId[accountId].destroy({timeout: 500}) - } catch (err) { - err.message = `Error while cleaning up sync worker: ${err.message}` - NylasEnv.reportError(err) - } + await this._workersByAccountId[accountId].destroy({timeout}) this._workersByAccountId[accountId] = null; }