From 2d0facd47b072d302dd5ff8a6840cd424b44b1cb Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Thu, 7 Aug 2025 11:27:27 +0100 Subject: [PATCH] Refactor MultiInviter (#30500) * MultiInviter: remove cancellation support This is unused and untested, so we can basically assume it doesn't work. * MultiInviter: factor out `handleUnknownProfileUsers` method * MultiInviter: remove unused `ignoreProfile` arg from `inviteMore` * MultiInviter: simplify `deferred` usage No point in doing `deferred.resolve(this.completionStates)` everywhere * MultiInviter.doInvite: do not `reject` for known fatal errors Using `reject` for known, handled, fatal errors is somewhat confusing here, since it looks like we swallow the error. (It's actually up to the caller to check the recoreded `errors` and report them.) Rather than rejecting, rely on the `_fatal` flag. * MultiInviter: move finish logic to `.invite` ... for less `deferred` complication * MultiInviter: rewrite loop as a `for` loop Async functions are a thing in modern javascript, and way easier to grok than a stack of promises. --- src/RoomInvite.tsx | 6 +- src/utils/MultiInviter.ts | 175 ++++++++++++++++---------------------- 2 files changed, 78 insertions(+), 103 deletions(-) diff --git a/src/RoomInvite.tsx b/src/RoomInvite.tsx index a02530a1cf..70aabed3de 100644 --- a/src/RoomInvite.tsx +++ b/src/RoomInvite.tsx @@ -26,9 +26,9 @@ export interface IInviteResult { } /** - * Invites multiple addresses to a room - * Simpler interface to utils/MultiInviter but with - * no option to cancel. + * Invites multiple addresses to a room. + * + * Simpler interface to {@link MultiInviter}. * * @param {string} roomId The ID of the room to invite to * @param {string[]} addresses Array of strings of addresses to invite. May be matrix IDs or 3pids. diff --git a/src/utils/MultiInviter.ts b/src/utils/MultiInviter.ts index 9ad16c0549..67bebe732d 100644 --- a/src/utils/MultiInviter.ts +++ b/src/utils/MultiInviter.ts @@ -44,13 +44,10 @@ const USER_BANNED = "IO.ELEMENT.BANNED"; * Invites multiple addresses to a room, handling rate limiting from the server */ export default class MultiInviter { - private canceled = false; private addresses: string[] = []; - private busy = false; private _fatal = false; private completionStates: CompletionStates = {}; // State of each address (invited or error) private errors: Record = {}; // { address: {errorText, errcode} } - private deferred: PromiseWithResolvers | null = null; private reason: string | undefined; /** @@ -76,7 +73,7 @@ export default class MultiInviter { * @param {string} reason Reason for inviting (optional) * @returns {Promise} Resolved when all invitations in the queue are complete */ - public invite(addresses: string[], reason?: string): Promise { + public async invite(addresses: string[], reason?: string): Promise { if (this.addresses.length > 0) { throw new Error("Already inviting/invited"); } @@ -92,20 +89,43 @@ export default class MultiInviter { }; } } - this.deferred = Promise.withResolvers(); - this.inviteMore(0); - return this.deferred.promise; - } + for (const addr of this.addresses) { + // don't try to invite it if it's an invalid address + // (it will already be marked as an error though, + // so no need to do so again) + if (getAddressType(addr) === null) { + continue; + } - /** - * Stops inviting. Causes promises returned by invite() to be rejected. - */ - public cancel(): void { - if (!this.busy) return; + // don't re-invite (there's no way in the UI to do this, but + // for sanity's sake) + if (this.completionStates[addr] === InviteState.Invited) { + continue; + } - this.canceled = true; - this.deferred?.reject(new Error("canceled")); + await this.doInvite(addr, false); + + if (this._fatal) { + // `doInvite` suffered a fatal error. The error should have been recorded in `errors`; it's up + // to the caller to report back to the user. + return this.completionStates; + } + } + + if (Object.keys(this.errors).length > 0) { + // There were problems inviting some people - see if we can invite them + // without caring if they exist or not. + const unknownProfileUsers = Object.keys(this.errors).filter((a) => + UNKNOWN_PROFILE_ERRORS.includes(this.errors[a].errcode), + ); + + if (unknownProfileUsers.length > 0) { + await this.handleUnknownProfileUsers(unknownProfileUsers); + } + } + + return this.completionStates; } public getCompletionState(addr: string): InviteState { @@ -193,17 +213,19 @@ export default class MultiInviter { } } - private doInvite(address: string, ignoreProfile = false): Promise { + /** + * Attempt to invite a user. + * + * Does not normally throw exceptions. If there was an error, this is reflected in {@link errors}. + * If the error was fatal and should prevent further invites from being done, {@link _fatal} is set. + */ + private doInvite(address: string, ignoreProfile: boolean): Promise { return new Promise((resolve, reject) => { logger.log(`Inviting ${address}`); const doInvite = this.inviteToRoom(this.roomId, address, ignoreProfile); doInvite .then(() => { - if (this.canceled) { - return; - } - this.completionStates[address] = InviteState.Invited; delete this.errors[address]; @@ -211,10 +233,6 @@ export default class MultiInviter { this.progressCallback?.(); }) .catch((err) => { - if (this.canceled) { - return; - } - logger.error(err); const room = this.roomId ? this.matrixClient.getRoom(this.roomId) : null; @@ -224,7 +242,6 @@ export default class MultiInviter { ]; let errorText: string | undefined; - let fatal = false; switch (err.errcode) { case "M_FORBIDDEN": if (isSpace) { @@ -238,7 +255,8 @@ export default class MultiInviter { ? _t("invite|error_unfederated_room") : _t("invite|error_permissions_room"); } - fatal = true; + // No point doing further invites. + this._fatal = true; break; case USER_ALREADY_INVITED: if (isSpace) { @@ -299,86 +317,43 @@ export default class MultiInviter { this.completionStates[address] = InviteState.Error; this.errors[address] = { errorText, errcode: err.errcode }; - this.busy = !fatal; - this._fatal = fatal; - - if (fatal) { - reject(err); - } else { - resolve(); - } + resolve(); }); }); } - private inviteMore(nextIndex: number, ignoreProfile = false): void { - if (this.canceled) { - return; - } + /** Handle users which failed with an error code which indicated that their profile was unknown. + * + * Depending on the `promptBeforeInviteUnknownUsers` setting, we either prompt the user for how to proceed, or + * send the invites anyway. + */ + private handleUnknownProfileUsers(unknownProfileUsers: string[]): Promise { + return new Promise((resolve) => { + const inviteUnknowns = (): void => { + const promises = unknownProfileUsers.map((u) => this.doInvite(u, true)); + Promise.all(promises).then(() => resolve()); + }; - if (nextIndex === this.addresses.length) { - this.busy = false; - if (Object.keys(this.errors).length > 0) { - // There were problems inviting some people - see if we can invite them - // without caring if they exist or not. - const unknownProfileUsers = Object.keys(this.errors).filter((a) => - UNKNOWN_PROFILE_ERRORS.includes(this.errors[a].errcode), - ); - - if (unknownProfileUsers.length > 0) { - const inviteUnknowns = (): void => { - const promises = unknownProfileUsers.map((u) => this.doInvite(u, true)); - Promise.all(promises).then(() => this.deferred?.resolve(this.completionStates)); - }; - - if (!SettingsStore.getValue("promptBeforeInviteUnknownUsers", this.roomId)) { - inviteUnknowns(); - return; - } - - logger.log("Showing failed to invite dialog..."); - Modal.createDialog(AskInviteAnywayDialog, { - unknownProfileUsers: unknownProfileUsers.map((u) => ({ - userId: u, - errorText: this.errors[u].errorText, - })), - onInviteAnyways: () => inviteUnknowns(), - onGiveUp: () => { - // Fake all the completion states because we already warned the user - for (const addr of unknownProfileUsers) { - this.completionStates[addr] = InviteState.Invited; - } - this.deferred?.resolve(this.completionStates); - }, - }); - return; - } + if (!SettingsStore.getValue("promptBeforeInviteUnknownUsers", this.roomId)) { + inviteUnknowns(); + return; } - this.deferred?.resolve(this.completionStates); - return; - } - const addr = this.addresses[nextIndex]; - - // don't try to invite it if it's an invalid address - // (it will already be marked as an error though, - // so no need to do so again) - if (getAddressType(addr) === null) { - this.inviteMore(nextIndex + 1); - return; - } - - // don't re-invite (there's no way in the UI to do this, but - // for sanity's sake) - if (this.completionStates[addr] === InviteState.Invited) { - this.inviteMore(nextIndex + 1); - return; - } - - this.doInvite(addr, ignoreProfile) - .then(() => { - this.inviteMore(nextIndex + 1, ignoreProfile); - }) - .catch(() => this.deferred?.resolve(this.completionStates)); + logger.log("Showing failed to invite dialog..."); + Modal.createDialog(AskInviteAnywayDialog, { + unknownProfileUsers: unknownProfileUsers.map((u) => ({ + userId: u, + errorText: this.errors[u].errorText, + })), + onInviteAnyways: () => inviteUnknowns(), + onGiveUp: () => { + // Fake all the completion states because we already warned the user + for (const addr of unknownProfileUsers) { + this.completionStates[addr] = InviteState.Invited; + } + resolve(); + }, + }); + }); } }