diff --git a/spec/unit/matrix-client.spec.ts b/spec/unit/matrix-client.spec.ts index 58e6a579990..8d73939a54a 100644 --- a/spec/unit/matrix-client.spec.ts +++ b/spec/unit/matrix-client.spec.ts @@ -40,6 +40,8 @@ import { makeBeaconInfoContent } from "../../src/content-helpers"; import { M_BEACON_INFO } from "../../src/@types/beacon"; import { ContentHelpers, + ClientPrefix, + Direction, EventTimeline, ICreateRoomOpts, IRequestOpts, @@ -68,9 +70,20 @@ jest.mock("../../src/webrtc/call", () => ({ supportsMatrixCall: jest.fn(() => false), })); +// Utility function to ease the transition from our QueryDict type to a Map +// which we can use to build a URLSearchParams +function convertQueryDictToMap(queryDict?: QueryDict): Map { + if (!queryDict) { + return new Map(); + } + + return new Map(Object.entries(queryDict).map(([k, v]) => [k, String(v)])); +} + type HttpLookup = { method: string; path: string; + prefix?: string; data?: Record; error?: object; expectBody?: Record; @@ -87,6 +100,42 @@ type WrappedRoom = Room & { _state: Map; }; +describe("convertQueryDictToMap", () => { + it("returns an empty map when dict is undefined", () => { + expect(convertQueryDictToMap(undefined)).toEqual(new Map()); + }); + + it("converts an empty QueryDict to an empty map", () => { + expect(convertQueryDictToMap({})).toEqual(new Map()); + }); + + it("converts a QueryDict of strings to the equivalent map", () => { + expect(convertQueryDictToMap({ a: "b", c: "d" })).toEqual( + new Map([ + ["a", "b"], + ["c", "d"], + ]), + ); + }); + + it("converts the values of the supplied QueryDict to strings", () => { + expect(convertQueryDictToMap({ arr: ["b", "c"], num: 45, boo: true, und: undefined })).toEqual( + new Map([ + ["arr", "b,c"], + ["num", "45"], + ["boo", "true"], + ["und", "undefined"], + ]), + ); + }); + + it("produces sane URLSearchParams conversions", () => { + expect(new URLSearchParams(Array.from(convertQueryDictToMap({ a: "b", c: "d" }))).toString()).toEqual( + "a=b&c=d", + ); + }); +}); + describe("MatrixClient", function () { const userId = "@alice:bar"; const identityServerUrl = "https://identity.server"; @@ -135,7 +184,14 @@ describe("MatrixClient", function () { method: string; path: string; } | null = null; - function httpReq(method: Method, path: string, qp?: QueryDict, data?: BodyInit, opts?: IRequestOpts) { + function httpReq( + method: Method, + path: string, + queryParams?: QueryDict, + body?: BodyInit, + requestOpts: IRequestOpts = {}, + ) { + const { prefix } = requestOpts; if (path === KEEP_ALIVE_PATH && acceptKeepalives) { return Promise.resolve({ unstable_features: unstableFeatures, @@ -171,14 +227,17 @@ describe("MatrixClient", function () { }; return pendingLookup.promise; } - if (next.path === path && next.method === method) { + // Either we don't care about the prefix if it wasn't defined in the expected + // lookup or it should match. + const doesMatchPrefix = !next.prefix || next.prefix === prefix; + if (doesMatchPrefix && next.path === path && next.method === method) { logger.log("MatrixClient[UT] Matched. Returning " + (next.error ? "BAD" : "GOOD") + " response"); if (next.expectBody) { - expect(data).toEqual(next.expectBody); + expect(body).toEqual(next.expectBody); } if (next.expectQueryParams) { Object.keys(next.expectQueryParams).forEach(function (k) { - expect(qp?.[k]).toEqual(next.expectQueryParams![k]); + expect(queryParams?.[k]).toEqual(next.expectQueryParams![k]); }); } @@ -198,12 +257,22 @@ describe("MatrixClient", function () { } return Promise.resolve(next.data); } - // Jest doesn't let us have custom expectation errors, so if you're seeing this then - // you forgot to handle at least 1 pending request. Check your tests to ensure your - // number of expectations lines up with your number of requests made, and that those - // requests match your expectations. - expect(true).toBe(false); - return new Promise(() => {}); + + const receivedRequestQueryString = new URLSearchParams( + Array.from(convertQueryDictToMap(queryParams)), + ).toString(); + const receivedRequestDebugString = `${method} ${prefix}${path}${receivedRequestQueryString}`; + const expectedQueryString = new URLSearchParams( + Array.from(convertQueryDictToMap(next.expectQueryParams)), + ).toString(); + const expectedRequestDebugString = `${next.method} ${next.prefix ?? ""}${next.path}${expectedQueryString}`; + // If you're seeing this then you forgot to handle at least 1 pending request. + throw new Error( + `A pending request was not handled: ${receivedRequestDebugString} ` + + `(next request expected was ${expectedRequestDebugString})\n` + + `Check your tests to ensure your number of expectations lines up with your number of requests ` + + `made, and that those requests match your expectations.`, + ); } function makeClient() { @@ -286,6 +355,166 @@ describe("MatrixClient", function () { client.stopClient(); }); + describe("timestampToEvent", () => { + const roomId = "!room:server.org"; + const eventId = "$eventId:example.org"; + const unstableMSC3030Prefix = "/_matrix/client/unstable/org.matrix.msc3030"; + + async function assertRequestsMade( + responses: { + prefix?: string; + error?: { httpStatus: Number; errcode: string }; + data?: { event_id: string }; + }[], + expectRejects = false, + ) { + const queryParams = { + ts: "0", + dir: "f", + }; + const path = `/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`; + // Set up the responses we are going to send back + httpLookups = responses.map((res) => { + return { + method: "GET", + path, + expectQueryParams: queryParams, + ...res, + }; + }); + + // When we ask for the event timestamp (this is what we are testing) + const answer = client.timestampToEvent(roomId, 0, Direction.Forward); + + if (expectRejects) { + await expect(answer).rejects.toBeDefined(); + } else { + await answer; + } + + // Then the number of requests me made matches our expectation + const calls = mocked(client.http.authedRequest).mock.calls; + expect(calls.length).toStrictEqual(responses.length); + + // And each request was as we expected + let i = 0; + for (const call of calls) { + const response = responses[i]; + const [callMethod, callPath, callQueryParams, , callOpts] = call; + const callPrefix = callOpts?.prefix; + + expect(callMethod).toStrictEqual("GET"); + if (response.prefix) { + expect(callPrefix).toStrictEqual(response.prefix); + } + expect(callPath).toStrictEqual(path); + expect(callQueryParams).toStrictEqual(queryParams); + i++; + } + } + + it("should call stable endpoint", async () => { + await assertRequestsMade([ + { + data: { event_id: eventId }, + }, + ]); + }); + + it("should fallback to unstable endpoint when stable endpoint 400s", async () => { + await assertRequestsMade([ + { + prefix: ClientPrefix.V1, + error: { + httpStatus: 400, + errcode: "M_UNRECOGNIZED", + }, + }, + { + prefix: unstableMSC3030Prefix, + data: { event_id: eventId }, + }, + ]); + }); + + it("should fallback to unstable endpoint when stable endpoint 404s", async () => { + await assertRequestsMade([ + { + prefix: ClientPrefix.V1, + error: { + httpStatus: 404, + errcode: "M_UNRECOGNIZED", + }, + }, + { + prefix: unstableMSC3030Prefix, + data: { event_id: eventId }, + }, + ]); + }); + + it("should fallback to unstable endpoint when stable endpoint 405s", async () => { + await assertRequestsMade([ + { + prefix: ClientPrefix.V1, + error: { + httpStatus: 405, + errcode: "M_UNRECOGNIZED", + }, + }, + { + prefix: unstableMSC3030Prefix, + data: { event_id: eventId }, + }, + ]); + }); + + it("should not fallback to unstable endpoint when stable endpoint returns an error (500)", async () => { + await assertRequestsMade( + [ + { + prefix: ClientPrefix.V1, + error: { + httpStatus: 500, + errcode: "Fake response error", + }, + }, + ], + true, + ); + }); + + it("should not fallback to unstable endpoint when stable endpoint is rate-limiting (429)", async () => { + await assertRequestsMade( + [ + { + prefix: ClientPrefix.V1, + error: { + httpStatus: 429, + errcode: "M_UNRECOGNIZED", // Still refuses even if the errcode claims unrecognised + }, + }, + ], + true, + ); + }); + + it("should not fallback to unstable endpoint when stable endpoint says bad gateway (502)", async () => { + await assertRequestsMade( + [ + { + prefix: ClientPrefix.V1, + error: { + httpStatus: 502, + errcode: "Fake response error", + }, + }, + ], + true, + ); + }); + }); + describe("getSafeUserId()", () => { it("returns the logged in user id", () => { expect(client.getSafeUserId()).toEqual(userId); diff --git a/src/client.ts b/src/client.ts index ae42103f26e..6a7735b51bf 100644 --- a/src/client.ts +++ b/src/client.ts @@ -9392,27 +9392,50 @@ export class MatrixClient extends TypedEventEmitter { + public async timestampToEvent( + roomId: string, + timestamp: number, + dir: Direction, + ): Promise { const path = utils.encodeUri("/rooms/$roomId/timestamp_to_event", { $roomId: roomId, }); + const queryParams = { + ts: timestamp.toString(), + dir: dir, + }; - return this.http.authedRequest( - Method.Get, - path, - { - ts: timestamp.toString(), - dir: dir, - }, - undefined, - { - prefix: "/_matrix/client/unstable/org.matrix.msc3030", - }, - ); + try { + return await this.http.authedRequest(Method.Get, path, queryParams, undefined, { + prefix: ClientPrefix.V1, + }); + } catch (err) { + // Fallback to the prefixed unstable endpoint. Since the stable endpoint is + // new, we should also try the unstable endpoint before giving up. We can + // remove this fallback request in a year (remove after 2023-11-28). + if ( + (err).errcode === "M_UNRECOGNIZED" && + // XXX: The 400 status code check should be removed in the future + // when Synapse is compliant with MSC3743. + ((err).httpStatus === 400 || + // This the correct standard status code for an unsupported + // endpoint according to MSC3743. Not Found and Method Not Allowed + // both indicate that this endpoint+verb combination is + // not supported. + (err).httpStatus === 404 || + (err).httpStatus === 405) + ) { + return await this.http.authedRequest(Method.Get, path, queryParams, undefined, { + prefix: "/_matrix/client/unstable/org.matrix.msc3030", + }); + } + + throw err; + } } }