Check for valid fields on state future send

Also commonize error checking between message/state events/futures

Author: AndrewFerr

spec/unit/embedded.spec.ts

@@ -60,15 +60,21 @@ class MockWidgetApi extends EventEmitter {
     public requestCapabilityToSendToDevice = jest.fn();
     public requestCapabilityToReceiveToDevice = jest.fn();
     public sendRoomEvent = jest.fn(
-        (eventType: string, content: unknown, roomId?: string, futureTimeout?: number, futureGroupId?: string) =>
+        (
+            eventType: string,
+            content: unknown,
+            roomId?: string,
+            futureTimeout?: number,
+            futureGroupId?: string
+        ) =>
             futureTimeout === undefined && futureGroupId === undefined
                 ? { event_id: `$${Math.random()}` }
                 : {
                       future_group_id: futureGroupId ?? `fg-${Math.random()}`,
                       send_token: `st-${Math.random()}`,
                       cancel_token: `ct-${Math.random()}`,
                       refresh_token: `rt-${Math.random()}`,
-                  },
+                  }
     );
     public sendStateEvent = jest.fn(
         (
@@ -86,7 +92,7 @@ class MockWidgetApi extends EventEmitter {
                       send_token: `st-${Math.random()}`,
                       cancel_token: `ct-${Math.random()}`,
                       refresh_token: `rt-${Math.random()}`,
-                  },
+                  }
     );
     public sendToDevice = jest.fn();
     public requestOpenIDConnectToken = jest.fn(() => {
@@ -188,102 +194,173 @@ describe("RoomWidgetClient", () => {
     });
 
     describe("futures", () => {
-        const doesServerSupportUnstableFeatureMock = jest.fn((feature) =>
-            Promise.resolve(feature === "org.matrix.msc4140"),
-        );
+        describe("when supported", () => {
+            const doesServerSupportUnstableFeatureMock = jest.fn((feature) =>
+                Promise.resolve(feature === "org.matrix.msc4140")
+            );
 
-        beforeAll(() => {
-            MatrixClient.prototype.doesServerSupportUnstableFeature = doesServerSupportUnstableFeatureMock;
-        });
+            beforeAll(() => {
+                MatrixClient.prototype.doesServerSupportUnstableFeature = doesServerSupportUnstableFeatureMock;
+            });
 
-        afterAll(() => {
-            doesServerSupportUnstableFeatureMock.mockClear();
-        });
+            afterAll(() => {
+                doesServerSupportUnstableFeatureMock.mockReset();
+            });
 
-        it("sends futures in a new group for message events", async () => {
-            await makeClient({ sendEvent: ["org.matrix.rageshake_request"] });
-            expect(widgetApi.requestCapabilityForRoomTimeline).toHaveBeenCalledWith("!1:example.org");
-            expect(widgetApi.requestCapabilityToSendEvent).toHaveBeenCalledWith("org.matrix.rageshake_request");
-            const futureOpts = {
-                future_timeout: 2000,
-            };
-            await client._unstable_sendFuture("!1:example.org", futureOpts, null, "org.matrix.rageshake_request", {
-                request_id: 123,
+            it("sends futures in a new group for message events", async () => {
+                await makeClient({ sendEvent: ["org.matrix.rageshake_request"] });
+                expect(widgetApi.requestCapabilityForRoomTimeline).toHaveBeenCalledWith("!1:example.org");
+                expect(widgetApi.requestCapabilityToSendEvent).toHaveBeenCalledWith("org.matrix.rageshake_request");
+                await client._unstable_sendFuture(
+                    "!1:example.org",
+                    { future_timeout: 2000 },
+                    null,
+                    "org.matrix.rageshake_request",
+                    { request_id: 123 },
+                );
+                expect(widgetApi.sendRoomEvent).toHaveBeenCalledWith(
+                    "org.matrix.rageshake_request",
+                    { request_id: 123 },
+                    "!1:example.org",
+                    2000,
+                    undefined,
+                );
             });
-            expect(widgetApi.sendRoomEvent).toHaveBeenCalledWith(
-                "org.matrix.rageshake_request",
-                { request_id: 123 },
-                "!1:example.org",
-                futureOpts.future_timeout,
-                undefined,
-            );
-        });
 
-        it("sends action futures in an existing group for message events", async () => {
-            await makeClient({ sendEvent: ["org.matrix.rageshake_request"] });
-            expect(widgetApi.requestCapabilityForRoomTimeline).toHaveBeenCalledWith("!1:example.org");
-            expect(widgetApi.requestCapabilityToSendEvent).toHaveBeenCalledWith("org.matrix.rageshake_request");
-            const futureOpts = {
-                future_group_id: "fg",
-            };
-            await client._unstable_sendFuture("!1:example.org", futureOpts, null, "org.matrix.rageshake_request", {
-                request_id: 123,
+            it("sends action futures in an existing group for message events", async () => {
+                await makeClient({ sendEvent: ["org.matrix.rageshake_request"] });
+                expect(widgetApi.requestCapabilityForRoomTimeline).toHaveBeenCalledWith("!1:example.org");
+                expect(widgetApi.requestCapabilityToSendEvent).toHaveBeenCalledWith("org.matrix.rageshake_request");
+                const futureGroupId = `fg-${Math.random()}`;
+                await client._unstable_sendFuture(
+                    "!1:example.org",
+                    { future_group_id: futureGroupId },
+                    null,
+                    "org.matrix.rageshake_request",
+                    { request_id: 123 },
+                );
+                expect(widgetApi.sendRoomEvent).toHaveBeenCalledWith(
+                    "org.matrix.rageshake_request",
+                    { request_id: 123 },
+                    "!1:example.org",
+                    undefined,
+                    futureGroupId,
+                );
             });
-            expect(widgetApi.sendRoomEvent).toHaveBeenCalledWith(
-                "org.matrix.rageshake_request",
-                { request_id: 123 },
-                "!1:example.org",
-                undefined,
-                futureOpts.future_group_id,
-            );
-        });
 
-        it("sends futures in a new group for state events", async () => {
-            await makeClient({ sendState: [{ eventType: "org.example.foo", stateKey: "bar" }] });
-            expect(widgetApi.requestCapabilityForRoomTimeline).toHaveBeenCalledWith("!1:example.org");
-            expect(widgetApi.requestCapabilityToSendState).toHaveBeenCalledWith("org.example.foo", "bar");
-            const futureOpts = {
-                future_timeout: 2000,
-            };
-            await client._unstable_sendStateFuture(
-                "!1:example.org",
-                futureOpts,
-                "org.example.foo",
-                { hello: "world" },
-                "bar",
-            );
-            expect(widgetApi.sendStateEvent).toHaveBeenCalledWith(
-                "org.example.foo",
-                "bar",
-                { hello: "world" },
-                "!1:example.org",
-                futureOpts.future_timeout,
-                undefined,
-            );
+            it("sends futures in a new group for state events", async () => {
+                await makeClient({ sendState: [{ eventType: "org.example.foo", stateKey: "bar" }] });
+                expect(widgetApi.requestCapabilityForRoomTimeline).toHaveBeenCalledWith("!1:example.org");
+                expect(widgetApi.requestCapabilityToSendState).toHaveBeenCalledWith("org.example.foo", "bar");
+                await client._unstable_sendStateFuture(
+                    "!1:example.org",
+                    { future_timeout: 2000 },
+                    "org.example.foo",
+                    { hello: "world" },
+                    "bar",
+                );
+                expect(widgetApi.sendStateEvent).toHaveBeenCalledWith(
+                    "org.example.foo",
+                    "bar",
+                    { hello: "world" },
+                    "!1:example.org",
+                    2000,
+                    undefined,
+                );
+            });
+
+            it("sends action futures in an existing group for state events", async () => {
+                await makeClient({ sendState: [{ eventType: "org.example.foo", stateKey: "bar" }] });
+                expect(widgetApi.requestCapabilityForRoomTimeline).toHaveBeenCalledWith("!1:example.org");
+                expect(widgetApi.requestCapabilityToSendState).toHaveBeenCalledWith("org.example.foo", "bar");
+                const futureGroupId = `fg-${Math.random()}`;
+                await client._unstable_sendStateFuture(
+                    "!1:example.org",
+                    { future_group_id: futureGroupId },
+                    "org.example.foo",
+                    { hello: "world" },
+                    "bar",
+                );
+                expect(widgetApi.sendStateEvent).toHaveBeenCalledWith(
+                    "org.example.foo",
+                    "bar",
+                    { hello: "world" },
+                    "!1:example.org",
+                    undefined,
+                    futureGroupId,
+                );
+            });
+
+            describe("when improperly implemented", () => {
+                const response = {
+                    future_group_id: `fg-${Math.random()}`,
+                    send_token: `st-${Math.random()}`,
+                    cancel_token: `ct-${Math.random()}`,
+                    refresh_token: `rt-${Math.random()}`,
+                };
+
+                for (const missingField of Object.keys(response)) {
+                    const badResponse: Record<string, string> = {...response};
+                    delete badResponse[missingField];
+                    const fullBadResponse = { ...badResponse, room_id: "!1:example.org" };
+
+                    it(`fails to send message futures without a ${missingField}`, async () => {
+                        jest.spyOn(widgetApi, "sendRoomEvent").mockResolvedValue(fullBadResponse);
+                        await makeClient({ sendEvent: ["org.matrix.rageshake_request"] });
+                        await expect(
+                            client._unstable_sendFuture(
+                                "!1:example.org",
+                                { future_timeout: 2000 },
+                                null,
+                                "org.matrix.rageshake_request",
+                                { request_id: 123 },
+                            )
+                        ).rejects.toThrow(`'${missingField}' absent from response`);
+                    });
+
+                    it(`fails to send state futures without a ${missingField}`, async () => {
+                        jest.spyOn(widgetApi, "sendStateEvent").mockResolvedValue(fullBadResponse);
+                        await makeClient({ sendState: [{ eventType: "org.example.foo", stateKey: "bar" }] });
+                        await expect(
+                            client._unstable_sendStateFuture(
+                            "!1:example.org",
+                            { future_timeout: 2000, },
+                            "org.example.foo",
+                            { hello: "world" },
+                            "bar",
+                        )
+                        ).rejects.toThrow(`'${missingField}' absent from response`);
+                    });
+                }
+            });
         });
 
-        it("sends action futures in an existing group for state events", async () => {
-            await makeClient({ sendState: [{ eventType: "org.example.foo", stateKey: "bar" }] });
-            expect(widgetApi.requestCapabilityForRoomTimeline).toHaveBeenCalledWith("!1:example.org");
-            expect(widgetApi.requestCapabilityToSendState).toHaveBeenCalledWith("org.example.foo", "bar");
-            const futureOpts = {
-                future_group_id: "fg",
-            };
-            await client._unstable_sendStateFuture(
-                "!1:example.org",
-                futureOpts,
-                "org.example.foo",
-                { hello: "world" },
-                "bar",
-            );
-            expect(widgetApi.sendStateEvent).toHaveBeenCalledWith(
-                "org.example.foo",
-                "bar",
-                { hello: "world" },
-                "!1:example.org",
-                undefined,
-                futureOpts.future_group_id,
-            );
+        describe("when unsupported", () => {
+            it("fails to send message futures", async () => {
+                await makeClient({ sendEvent: ["org.matrix.rageshake_request"] });
+                await expect(
+                    client._unstable_sendFuture(
+                        "!1:example.org",
+                        { future_timeout: 2000 },
+                        null,
+                        "org.matrix.rageshake_request",
+                        { request_id: 123 },
+                    )
+                ).rejects.toThrow("Server does not support the Futures API");
+            });
+
+            it("fails to send state futures", async () => {
+                await makeClient({ sendState: [{ eventType: "org.example.foo", stateKey: "bar" }] });
+                await expect(
+                    client._unstable_sendStateFuture(
+                    "!1:example.org",
+                    { future_timeout: 2000, },
+                    "org.example.foo",
+                    { hello: "world" },
+                    "bar",
+                )
+                ).rejects.toThrow("Server does not support the Futures API");
+            });
         });
     });
 

src/embedded.ts

@@ -269,10 +269,7 @@ export class RoomWidgetClient extends MatrixClient {
                 "future_timeout" in futureOpts ? futureOpts.future_timeout : undefined,
                 "parent_future_id" in futureOpts ? futureOpts.parent_future_id : undefined,
             );
-            if (!response.future_id) {
-                throw new Error("'future_id' absent from response to a futures request");
-            }
-            return { future_id: response.future_id };
+            return this.validateSendFutureResponse(response);
         }
 
         let response: ISendEventFromWidgetResponseData;
@@ -284,10 +281,7 @@ export class RoomWidgetClient extends MatrixClient {
         }
 
         room.updatePendingEvent(event, EventStatus.SENT, response.event_id);
-        if (!response.event_id) {
-            throw new Error("'event_id' absent from response to an event request");
-        }
-        return { event_id: response.event_id };
+        return this.validateSendEventResponse(response);
     }
 
     public async sendStateEvent(
@@ -297,10 +291,7 @@ export class RoomWidgetClient extends MatrixClient {
         stateKey = "",
     ): Promise<ISendEventResponse> {
         const response = await this.widgetApi.sendStateEvent(eventType, stateKey, content, roomId);
-        if (response.event_id === undefined) {
-            throw new Error("'event_id' absent from response to an event request");
-        }
-        return { event_id: response.event_id };
+        return this.validateSendEventResponse(response);
     }
 
     /**
@@ -318,15 +309,29 @@ export class RoomWidgetClient extends MatrixClient {
             throw Error("Server does not support the Futures API");
         }
 
-        // TODO: better type checking
-        return (await this.widgetApi.sendStateEvent(
+        const response = await this.widgetApi.sendStateEvent(
             eventType,
             stateKey,
             content,
             roomId,
             "future_timeout" in futureOpts ? futureOpts.future_timeout : undefined,
             "parent_future_id" in futureOpts ? futureOpts.parent_future_id : undefined,
-        )) as SendFutureResponse;
+        );
+        return this.validateSendFutureResponse(response);
+    }
+
+    private validateSendEventResponse(response: ISendEventFromWidgetResponseData): ISendEventResponse {
+        if (response.event_id === undefined) {
+            throw new Error("'event_id' absent from response to an event request");
+        }
+        return { event_id: response.event_id };
+    }
+
+    private validateSendFutureResponse(response: ISendEventFromWidgetResponseData): SendFutureResponse {
+        if (!response.future_id) {
+            throw new Error("'future_id' absent from response to a future request");
+        }
+        return { future_id: response.future_id };
     }
 
     public async sendToDevice(eventType: string, contentMap: SendToDeviceContentMap): Promise<{}> {