From 21e09840dc989646685546a909461d1bf5054a66 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Thu, 2 Nov 2017 15:59:26 +0000 Subject: [PATCH] Fix multiple requests for publicised groups of given user Previously, a single user could end up in multiple batches, which would have been fine if the logic didn't assume otherwise. If a request took longer than 200ms, multiple batches would occur with intersecting sets of users, deleting promises that were then assumed to exist. The logic now takes all "in flight" users to also not be "pending". Pending now means that the user will be processed in the next batch. "In flight" means the user is part of an ongoing batch. --- src/stores/FlairStore.js | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/src/stores/FlairStore.js b/src/stores/FlairStore.js index d848ca7dda..9424503390 100644 --- a/src/stores/FlairStore.js +++ b/src/stores/FlairStore.js @@ -66,7 +66,7 @@ class FlairStore extends EventEmitter { } // Bulk lookup ongoing, return promise to resolve/reject - if (this._usersPending[userId]) { + if (this._usersPending[userId] || this._usersInFlight[userId]) { return this._usersPending[userId].prom; } @@ -91,7 +91,7 @@ class FlairStore extends EventEmitter { console.error('Could not get groups for user', this.props.userId, err); throw err; }).finally(() => { - delete this._usersPending[userId]; + delete this._usersInFlight[userId]; }); // This debounce will allow consecutive requests for the public groups of users that @@ -113,27 +113,25 @@ class FlairStore extends EventEmitter { } async _batchedGetPublicGroups(matrixClient) { - // Take the userIds from the keys of this._usersPending - const usersInFlight = Object.keys(this._usersPending); + // Move users pending to users in flight + this._usersInFlight = this._usersPending; + this._usersPending = {}; + let resp = { users: [], }; try { - resp = await matrixClient.getPublicisedGroups(usersInFlight); + resp = await matrixClient.getPublicisedGroups(Object.keys(this._usersInFlight)); } catch (err) { // Propagate the same error to all usersInFlight - usersInFlight.forEach((userId) => { - this._usersPending[userId].reject(err); + Object.keys(this._usersInFlight).forEach((userId) => { + this._usersInFlight[userId].reject(err); }); return; } const updatedUserGroups = resp.users; - usersInFlight.forEach((userId) => { - if (this._usersPending[userId]) { - this._usersPending[userId].resolve(updatedUserGroups[userId] || []); - } else { - console.error("Promise vanished for resolving groups for " + userId); - } + Object.keys(this._usersInFlight).forEach((userId) => { + this._usersInFlight[userId].resolve(updatedUserGroups[userId] || []); }); }