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.
pull/21833/head
Luke Barnard 2017-11-02 15:59:26 +00:00
parent 6baf6af332
commit 21e09840dc
1 changed files with 11 additions and 13 deletions

View File

@ -66,7 +66,7 @@ class FlairStore extends EventEmitter {
} }
// Bulk lookup ongoing, return promise to resolve/reject // Bulk lookup ongoing, return promise to resolve/reject
if (this._usersPending[userId]) { if (this._usersPending[userId] || this._usersInFlight[userId]) {
return this._usersPending[userId].prom; 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); console.error('Could not get groups for user', this.props.userId, err);
throw err; throw err;
}).finally(() => { }).finally(() => {
delete this._usersPending[userId]; delete this._usersInFlight[userId];
}); });
// This debounce will allow consecutive requests for the public groups of users that // This debounce will allow consecutive requests for the public groups of users that
@ -113,27 +113,25 @@ class FlairStore extends EventEmitter {
} }
async _batchedGetPublicGroups(matrixClient) { async _batchedGetPublicGroups(matrixClient) {
// Take the userIds from the keys of this._usersPending // Move users pending to users in flight
const usersInFlight = Object.keys(this._usersPending); this._usersInFlight = this._usersPending;
this._usersPending = {};
let resp = { let resp = {
users: [], users: [],
}; };
try { try {
resp = await matrixClient.getPublicisedGroups(usersInFlight); resp = await matrixClient.getPublicisedGroups(Object.keys(this._usersInFlight));
} catch (err) { } catch (err) {
// Propagate the same error to all usersInFlight // Propagate the same error to all usersInFlight
usersInFlight.forEach((userId) => { Object.keys(this._usersInFlight).forEach((userId) => {
this._usersPending[userId].reject(err); this._usersInFlight[userId].reject(err);
}); });
return; return;
} }
const updatedUserGroups = resp.users; const updatedUserGroups = resp.users;
usersInFlight.forEach((userId) => { Object.keys(this._usersInFlight).forEach((userId) => {
if (this._usersPending[userId]) { this._usersInFlight[userId].resolve(updatedUserGroups[userId] || []);
this._usersPending[userId].resolve(updatedUserGroups[userId] || []);
} else {
console.error("Promise vanished for resolving groups for " + userId);
}
}); });
} }