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

@@ -26,9 +26,9 @@ export interface IInviteResult {
 }
 
 /**
- * Invites multiple addresses to a room
+ * Invites multiple addresses to a room.
- * Simpler interface to utils/MultiInviter but with
+ *
- * no option to cancel.
+ * 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.

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<string, IError> = {}; // { address: {errorText, errcode} }
-    private deferred: PromiseWithResolvers<CompletionStates> | 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<CompletionStates> {
+    public async invite(addresses: string[], reason?: string): Promise<CompletionStates> {
         if (this.addresses.length > 0) {
             throw new Error("Already inviting/invited");
         }
@@ -92,20 +89,43 @@ export default class MultiInviter {
                 };
             }
         }
-        this.deferred = Promise.withResolvers<CompletionStates>();
-        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;
+            }
 
-    /**
+            // don't re-invite (there's no way in the UI to do this, but
-     * Stops inviting. Causes promises returned by invite() to be rejected.
+            // for sanity's sake)
-     */
+            if (this.completionStates[addr] === InviteState.Invited) {
-    public cancel(): void {
+                continue;
-        if (!this.busy) return;
+            }
 
-        this.canceled = true;
+            await this.doInvite(addr, false);
-        this.deferred?.reject(new Error("canceled"));
+
+            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<void> {
+    /**
+     * 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<void> {
         return new Promise<void>((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;
+                    resolve();
-                    this._fatal = fatal;
-
-                    if (fatal) {
-                        reject(err);
-                    } else {
-                        resolve();
-                    }
                 });
         });
     }
 
-    private inviteMore(nextIndex: number, ignoreProfile = false): void {
+    /** Handle users which failed with an error code which indicated that their profile was unknown.
-        if (this.canceled) {
+     *
-            return;
+     * Depending on the `promptBeforeInviteUnknownUsers` setting, we either prompt the user for how to proceed, or
-        }
+     * send the invites anyway.
+     */
+    private handleUnknownProfileUsers(unknownProfileUsers: string[]): Promise<void> {
+        return new Promise<void>((resolve) => {
+            const inviteUnknowns = (): void => {
+                const promises = unknownProfileUsers.map((u) => this.doInvite(u, true));
+                Promise.all(promises).then(() => resolve());
+            };
 
-        if (nextIndex === this.addresses.length) {
+            if (!SettingsStore.getValue("promptBeforeInviteUnknownUsers", this.roomId)) {
-            this.busy = false;
+                inviteUnknowns();
-            if (Object.keys(this.errors).length > 0) {
+                return;
-                // 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;
-                }
             }
-            this.deferred?.resolve(this.completionStates);
-            return;
-        }
 
-        const addr = this.addresses[nextIndex];
+            logger.log("Showing failed to invite dialog...");
-
+            Modal.createDialog(AskInviteAnywayDialog, {
-        // don't try to invite it if it's an invalid address
+                unknownProfileUsers: unknownProfileUsers.map((u) => ({
-        // (it will already be marked as an error though,
+                    userId: u,
-        // so no need to do so again)
+                    errorText: this.errors[u].errorText,
-        if (getAddressType(addr) === null) {
+                })),
-            this.inviteMore(nextIndex + 1);
+                onInviteAnyways: () => inviteUnknowns(),
-            return;
+                onGiveUp: () => {
-        }
+                    // Fake all the completion states because we already warned the user
-
+                    for (const addr of unknownProfileUsers) {
-        // don't re-invite (there's no way in the UI to do this, but
+                        this.completionStates[addr] = InviteState.Invited;
-        // for sanity's sake)
+                    }
-        if (this.completionStates[addr] === InviteState.Invited) {
+                    resolve();
-            this.inviteMore(nextIndex + 1);
+                },
-            return;
+            });
-        }
+        });
-
-        this.doInvite(addr, ignoreProfile)
-            .then(() => {
-                this.inviteMore(nextIndex + 1, ignoreProfile);
-            })
-            .catch(() => this.deferred?.resolve(this.completionStates));
     }
 }