Add invite retries to file trees

package.json

@@ -55,6 +55,7 @@
     "bs58": "^4.0.1",
     "content-type": "^1.0.4",
     "loglevel": "^1.7.1",
+    "p-retry": "^4.5.0",
     "qs": "^6.9.6",
     "request": "^2.88.2",
     "unhomoglyph": "^1.0.6"

spec/unit/models/MSC3089TreeSpace.spec.ts

@@ -25,6 +25,7 @@ import {
 } from "../../../src/models/MSC3089TreeSpace";
 import { DEFAULT_ALPHABET } from "../../../src/utils";
 import { MockBlob } from "../../MockBlob";
+import { MatrixError } from "../../../src/http-api";
 
 describe("MSC3089TreeSpace", () => {
     let client: MatrixClient;
@@ -93,7 +94,7 @@ describe("MSC3089TreeSpace", () => {
             });
         client.sendStateEvent = fn;
         await tree.setName(newName);
-        expect(fn.mock.calls.length).toBe(1);
+        expect(fn).toHaveBeenCalledTimes(1);
     });
 
     it('should support inviting users to the space', async () => {
@@ -104,8 +105,62 @@ describe("MSC3089TreeSpace", () => {
             return Promise.resolve();
         });
         client.invite = fn;
-        await tree.invite(target);
-        expect(fn.mock.calls.length).toBe(1);
+        await tree.invite(target, false);
+        expect(fn).toHaveBeenCalledTimes(1);
+    });
+
+    it('should retry invites to the space', async () => {
+        const target = targetUser;
+        const fn = jest.fn().mockImplementation((inviteRoomId: string, userId: string) => {
+            expect(inviteRoomId).toEqual(roomId);
+            expect(userId).toEqual(target);
+            if (fn.mock.calls.length === 1) return Promise.reject(new Error("Sample Failure"));
+            return Promise.resolve();
+        });
+        client.invite = fn;
+        await tree.invite(target, false);
+        expect(fn).toHaveBeenCalledTimes(2);
+    });
+
+    it('should not retry invite permission errors', async () => {
+        const target = targetUser;
+        const fn = jest.fn().mockImplementation((inviteRoomId: string, userId: string) => {
+            expect(inviteRoomId).toEqual(roomId);
+            expect(userId).toEqual(target);
+            return Promise.reject(new MatrixError({ errcode: "M_FORBIDDEN", error: "Sample Failure" }));
+        });
+        client.invite = fn;
+        try {
+            await tree.invite(target, false);
+
+            // noinspection ExceptionCaughtLocallyJS
+            throw new Error("Failed to fail");
+        } catch (e) {
+            expect(e.errcode).toEqual("M_FORBIDDEN");
+        }
+
+        expect(fn).toHaveBeenCalledTimes(1);
+    });
+
+    it('should invite to subspaces', async () => {
+        const target = targetUser;
+        const fn = jest.fn().mockImplementation((inviteRoomId: string, userId: string) => {
+            expect(inviteRoomId).toEqual(roomId);
+            expect(userId).toEqual(target);
+            return Promise.resolve();
+        });
+        client.invite = fn;
+        tree.getDirectories = () => [
+            // Bare minimum overrides. We proxy to our mock function manually so we can
+            // count the calls, not to ensure accuracy. The invite function behaving correctly
+            // is covered by another test.
+            { invite: (userId) => fn(tree.roomId, userId) } as MSC3089TreeSpace,
+            { invite: (userId) => fn(tree.roomId, userId) } as MSC3089TreeSpace,
+            { invite: (userId) => fn(tree.roomId, userId) } as MSC3089TreeSpace,
+        ];
+
+        await tree.invite(target, true);
+        expect(fn).toHaveBeenCalledTimes(4);
     });
 
     async function evaluatePowerLevels(pls: any, role: TreePermissions, expectedPl: number) {

spec/unit/utils.spec.ts

@@ -7,6 +7,7 @@ import {
     lexicographicCompare,
     nextString,
     prevString,
+    simpleRetryOperation,
     stringToBase,
 } from "../../src/utils";
 import { logger } from "../../src/logger";
@@ -267,6 +268,34 @@ describe("utils", function() {
         });
     });
 
+    describe('simpleRetryOperation', () => {
+        it('should retry', async () => {
+            let count = 0;
+            const val = {};
+            const fn = (attempt) => {
+                count++;
+
+                // If this expectation fails then it can appear as a Jest Timeout due to
+                // the retry running beyond the test limit.
+                expect(attempt).toEqual(count);
+
+                if (count > 1) {
+                    return Promise.resolve(val);
+                } else {
+                    return Promise.reject(new Error("Iterative failure"));
+                }
+            };
+
+            const ret = await simpleRetryOperation(fn);
+            expect(ret).toBe(val);
+            expect(count).toEqual(2);
+        });
+
+        // We don't test much else of the function because then we're just testing that the
+        // underlying library behaves, which should be tested on its own. Our API surface is
+        // all that concerns us.
+    });
+
     describe('DEFAULT_ALPHABET', () => {
         it('should be usefully printable ASCII in order', () => {
             expect(DEFAULT_ALPHABET).toEqual(

src/models/MSC3089TreeSpace.ts

@@ -19,8 +19,16 @@ import { EventType, IEncryptedFile, MsgType, UNSTABLE_MSC3089_BRANCH, UNSTABLE_M
 import { Room } from "./room";
 import { logger } from "../logger";
 import { MatrixEvent } from "./event";
-import { averageBetweenStrings, DEFAULT_ALPHABET, lexicographicCompare, nextString, prevString } from "../utils";
+import {
+    averageBetweenStrings,
+    DEFAULT_ALPHABET,
+    lexicographicCompare,
+    nextString,
+    prevString,
+    simpleRetryOperation,
+} from "../utils";
 import { MSC3089Branch } from "./MSC3089Branch";
+import promiseRetry from "p-retry";
 
 /**
  * The recommended defaults for a tree space's power levels. Note that this
@@ -110,12 +118,29 @@ export class MSC3089TreeSpace {
      * Invites a user to the tree space. They will be given the default Viewer
      * permission level unless specified elsewhere.
      * @param {string} userId The user ID to invite.
+     * @param {boolean} andSubspaces True (default) to invite the user to all
+     * directories/subspaces too, recursively.
      * @returns {Promise<void>} Resolves when complete.
      */
-    public invite(userId: string): Promise<void> {
-        // TODO: [@@TR] Reliable invites
+    public invite(userId: string, andSubspaces = true): Promise<void> {
         // TODO: [@@TR] Share keys
-        return this.client.invite(this.roomId, userId);
+        const promises: Promise<void>[] = [this.retryInvite(userId)];
+        if (andSubspaces) {
+            promises.push(...this.getDirectories().map(d => d.invite(userId, andSubspaces)));
+        }
+        return Promise.all(promises).then(); // .then() to coerce types
+    }
+
+    private retryInvite(userId: string): Promise<void> {
+        return simpleRetryOperation(() => {
+            return this.client.invite(this.roomId, userId).catch(e => {
+                // We don't want to retry permission errors forever...
+                if (e?.errcode === "M_FORBIDDEN") {
+                    throw new promiseRetry.AbortError(e);
+                }
+                throw e;
+            });
+        });
     }
 
     /**

src/utils.ts

@@ -20,7 +20,8 @@ limitations under the License.
  * @module utils
  */
 
-import unhomoglyph from 'unhomoglyph';
+import unhomoglyph from "unhomoglyph";
+import promiseRetry from "p-retry";
 
 /**
  * Encode a dictionary of query parameters.
@@ -443,6 +444,26 @@ export async function chunkPromises<T>(fns: (() => Promise<T>)[], chunkSize: num
     return results;
 }
 
+/**
+ * Retries the function until it succeeds or is interrupted. The given function must return
+ * a promise which throws/rejects on error, otherwise the retry will assume the request
+ * succeeded. The promise chain returned will contain the successful promise. The given function
+ * should always return a new promise.
+ * @param {Function} promiseFn The function to call to get a fresh promise instance. Takes an
+ * attempt count as an argument, for logging/debugging purposes.
+ * @returns {Promise<T>} The promise for the retried operation.
+ */
+export function simpleRetryOperation<T>(promiseFn: (attempt: number) => Promise<T>): Promise<T> {
+    return promiseRetry((attempt: number) => {
+        return promiseFn(attempt);
+    }, {
+        forever: true,
+        factor: 2,
+        minTimeout: 3000, // ms
+        maxTimeout: 15000, // ms
+    });
+}
+
 // We need to be able to access the Node.js crypto library from within the
 // Matrix SDK without needing to `require("crypto")`, which will fail in
 // browsers.  So `index.ts` will call `setCrypto` to store it, and when we need

yarn.lock

@@ -1132,6 +1132,7 @@
 
 "@matrix-org/olm@https://gitlab.matrix.org/api/v4/projects/27/packages/npm/@matrix-org/olm/-/@matrix-org/olm-3.2.3.tgz":
   version "3.2.3"
+  uid cc332fdd25c08ef0e40f4d33fc3f822a0f98b6f4
   resolved "https://gitlab.matrix.org/api/v4/projects/27/packages/npm/@matrix-org/olm/-/@matrix-org/olm-3.2.3.tgz#cc332fdd25c08ef0e40f4d33fc3f822a0f98b6f4"
 
 "@nicolo-ribaudo/chokidar-2@2.1.8-no-fsevents":
@@ -1305,6 +1306,11 @@
     "@types/tough-cookie" "*"
     form-data "^2.5.0"
 
+"@types/retry@^0.12.0":
+  version "0.12.0"
+  resolved "https://registry.yarnpkg.com/@types/retry/-/retry-0.12.0.tgz#2b35eccfcee7d38cd72ad99232fbd58bffb3c84d"
+  integrity sha512-wWKOClTTiizcZhXnPY4wikVAwmdYHp8q6DmC+EJUzAMsycb7HB32Kh9RN4+0gExjmPmZSAQjgURXIGATPegAvA==
+
 "@types/stack-utils@^2.0.0":
   version "2.0.0"
   resolved "https://registry.yarnpkg.com/@types/stack-utils/-/stack-utils-2.0.0.tgz#7036640b4e21cc2f259ae826ce843d277dad8cff"
@@ -5220,6 +5226,14 @@ p-locate@^4.1.0:
   dependencies:
     p-limit "^2.2.0"
 
+p-retry@^4.5.0:
+  version "4.5.0"
+  resolved "https://registry.yarnpkg.com/p-retry/-/p-retry-4.5.0.tgz#6685336b3672f9ee8174d3769a660cb5e488521d"
+  integrity sha512-5Hwh4aVQSu6BEP+w2zKlVXtFAaYQe1qWuVADSgoeVlLjwe/Q/AMSoRR4MDeaAfu8llT+YNbEijWu/YF3m6avkg==
+  dependencies:
+    "@types/retry" "^0.12.0"
+    retry "^0.12.0"
+
 p-try@^2.0.0:
   version "2.2.0"
   resolved "https://registry.yarnpkg.com/p-try/-/p-try-2.2.0.tgz#cb2868540e313d61de58fafbe35ce9004d5540e6"
@@ -5943,6 +5957,11 @@ ret@~0.1.10:
   resolved "https://registry.yarnpkg.com/ret/-/ret-0.1.15.tgz#b8a4825d5bdb1fc3f6f53c2bc33f81388681c7bc"
   integrity sha512-TTlYpa+OL+vMMNG24xSlQGEJ3B/RzEfUlLct7b5G/ytav+wPrplCpVMFuwzXbkecJrb6IYo1iFb0S9v37754mg==
 
+retry@^0.12.0:
+  version "0.12.0"
+  resolved "https://registry.yarnpkg.com/retry/-/retry-0.12.0.tgz#1b42a6266a21f07421d1b0b54b7dc167b01c013b"
+  integrity sha1-G0KmJmoh8HQh0bC1S33BZ7AcATs=
+
 reusify@^1.0.4:
   version "1.0.4"
   resolved "https://registry.yarnpkg.com/reusify/-/reusify-1.0.4.tgz#90da382b1e126efc02146e90845a88db12925d76"