[isomorphic-core] Ensure IMAPConnPool uses updated account credentials

Summary:
Previously, we would initialize an `AccountPool` with an account
instance and reference the account as an instance variable. Then,
every time we created a connection, we used the account we had a
reference to obtain the credentials.

However, if the credentials on the account changed, e.g. when we refresh
the access token for gmail accounts, the connection pool would never
realize that the account had new credentials and always try to create
the connection with the old credentials, causing the sync worker to
enter an error loop of `Invalid credentials` errors

To fix this, we could reload the account every time we try to create a
connection, but it seems that the fresh credentials should just be
passed every time we create connections without the pool having to be
aware of potential changes under the rug.

This commit makes it so the pool no longer holds a reference to the
account, but rather it is passed in every time we check out a connection

Test Plan: manual. how did this even work before? :(

Reviewers: evan, halla, spang, mark

Reviewed By: mark

Differential Revision: https://phab.nylas.com/D4370
This commit is contained in:
Juan Tejada 2017-04-05 22:35:21 -07:00
parent bb03d3c78e
commit 8633058488

View file

@ -8,15 +8,14 @@ const MAX_ICLOUD_CONNECTIONS = 5;
const MAX_IMAP_CONNECTIONS = 5;
class AccountConnectionPool {
constructor(account, maxConnections) {
this._account = account;
constructor(maxConnections) {
this._availableConns = new Array(maxConnections).fill(null);
this._queue = [];
}
async _genConnection(socketTimeout, logger) {
const settings = this._account.connectionSettings;
const credentials = this._account.decryptedCredentials();
async _genConnection(account, socketTimeout, logger) {
const settings = account.connectionSettings;
const credentials = account.decryptedCredentials();
if (!settings || !settings.imap_host) {
throw new Error("_genConnection: There are no IMAP connection settings for this account.");
@ -29,13 +28,13 @@ class AccountConnectionPool {
db: null,
settings: Object.assign({}, settings, credentials, {socketTimeout}),
logger,
account: this._account,
account,
});
return conn.connect();
}
async withConnections({desiredCount, logger, socketTimeout, onConnected}) {
async withConnections({account, desiredCount, logger, socketTimeout, onConnected}) {
// If we wake up from the first await but don't have enough connections in
// the pool then we need to prepend ourselves to the queue until there are
// enough. This guarantees that the queue is fair.
@ -74,7 +73,9 @@ class AccountConnectionPool {
for (let i = 0; i < desiredCount; ++i) {
conns.push(this._availableConns.shift());
}
conns = await Promise.all(conns.map(() => this._genConnection(socketTimeout, logger)));
conns = await Promise.all(
conns.map(() => this._genConnection(account, socketTimeout, logger))
);
keepOpen = await onConnected(conns, onDone);
if (!keepOpen) {
@ -108,11 +109,11 @@ class IMAPConnectionPool {
async withConnectionsForAccount(account, {desiredCount, logger, socketTimeout, onConnected}) {
if (!this._poolMap[account.id]) {
this._poolMap[account.id] = new AccountConnectionPool(account, this._maxConnectionsForAccount(account));
this._poolMap[account.id] = new AccountConnectionPool(this._maxConnectionsForAccount(account));
}
const pool = this._poolMap[account.id];
await pool.withConnections({desiredCount, logger, socketTimeout, onConnected});
await pool.withConnections({account, desiredCount, logger, socketTimeout, onConnected});
}
}