From 0f2896446ef6787c7aeb3e57dc94b8669c456efe Mon Sep 17 00:00:00 2001 From: Juan Tejada Date: Thu, 9 Mar 2017 00:18:56 -0800 Subject: [PATCH] [client-sync] Bump sequelize version - Ensure it doesn't halt sync Summary: This commit bumps the version of sequelize to point to the latest version in our fork. This version bump also includes latest updates from upstream sequelize since the last time we bumped the version. Notably, it includes a critical fix that will prevent the sync loop from getting stuck on rare occasions. Specifically, sequelize's `Query.prototype.run` could in some cases return a Promise that would /never/ resolve or reject, effectively halting the execution of any code that was waiting for that promise. This was due to a lack of error handling inside a the query's `afterExecute` function; if an error was thrown there, the enclosing Promise would never reject, and the error would just remain uncaught. An example of such an error that could cause this scenario is: T7742 - https://sentry.io/nylas/nylas-mail/issues/230016155/ Sync getting halted in this way can produce a variety of user facing bugs like not being able to send or it taking an absurd amount of time (hours), tasks never finishing or taking an absurd amount of time to complete, new mail not arriving, among others The commit that fixes this is: https://github.com/nylas/sequelize/commit/0deeda9e1ae058c6ce8d5bb29f4eea1a590f17f3 The full set of changes introduced by this version bump are: https://github.com/nylas/sequelize/compare/nylas-3.30.0...nylas-3.40.0 **Note:** It is important to note that sequelize might still halt the sync loop if there are other places in the code that can return Promises that never resolve or reject under certain circumstances. I can consistently reproduce this scenario when an error is thrown inside `afterExecute`, and I've seen it happen in the wild, but I've also ran into this type of sync loop halting (db promises never resolve) without any sequelize errors being thrown, which suggests that there are other places in the sequelize code that might end up returning a Promise that will halt sync. Unfortunately, we don't have a good way to detect and report when this happens yet, but we are adding one in upcoming diffs in order have data on how many people are running into this, and/or if this patch completely fixes the issue. Otherwise, we'd need to audit sequelize's code. Should resolve T7837 and T7767 Test Plan: manual Reviewers: spang, mark, halla, khamidou, evan Reviewed By: evan Subscribers: mg, tomasz Maniphest Tasks: T7767, T7837 Differential Revision: https://phab.nylas.com/D4152 --- packages/client-app/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/client-app/package.json b/packages/client-app/package.json index a65208d46..332934dd7 100644 --- a/packages/client-app/package.json +++ b/packages/client-app/package.json @@ -102,7 +102,7 @@ "sanitize-html": "1.9.0", "season": "^5.1", "semver": "^4.2", - "sequelize": "github:nylas/sequelize#nylas-3.30.0", + "sequelize": "nylas/sequelize#nylas-3.40.0", "simplemde": "jstejada/simplemde-markdown-editor#input-style-support", "source-map-support": "^0.3.2", "sqlite3": "https://github.com/bengotow/node-sqlite3/archive/bengotow/usleep-v3.1.4.tar.gz",