From 3a1897629ae56b13f220aaa8d16cb47206a28f5e Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 28 Nov 2022 22:50:37 -0600 Subject: [PATCH 01/19] Prefer stable endpoint first --- src/client.ts | 52 +++++++++++++++++++++++++++++++++++---------------- 1 file changed, 36 insertions(+), 16 deletions(-) diff --git a/src/client.ts b/src/client.ts index b091a31ec45..2e0492eb12d 100644 --- a/src/client.ts +++ b/src/client.ts @@ -9297,11 +9297,11 @@ export class MatrixClient extends TypedEventEmittererr).httpStatus === 400 && (err).errcode === "M_UNRECOGNIZED") { + return await this.http.authedRequest( + Method.Get, + path, + queryParams, + undefined, + { + prefix: "/_matrix/client/unstable/org.matrix.msc3030", + }, + ); + } + + throw err; + } } } From d3f08fec03d6805bf776a676451618a453cd8f2c Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 30 Nov 2022 18:45:05 -0600 Subject: [PATCH 02/19] Add tests --- spec/unit/matrix-client.spec.ts | 175 ++++++++++++++++++++++++++++++-- src/client.ts | 11 +- 2 files changed, 174 insertions(+), 12 deletions(-) diff --git a/spec/unit/matrix-client.spec.ts b/spec/unit/matrix-client.spec.ts index ba4be9f9b20..55f99d9af20 100644 --- a/spec/unit/matrix-client.spec.ts +++ b/spec/unit/matrix-client.spec.ts @@ -19,6 +19,12 @@ import { mocked } from "jest-mock"; import { logger } from "../../src/logger"; import { MatrixClient, ClientEvent } from "../../src/client"; import { Filter } from "../../src/filter"; +import { + Method, + ClientPrefix, + IRequestOpts, +} from "../../src/http-api"; +import { QueryDict } from "../../src/utils"; import { DEFAULT_TREE_POWER_LEVELS_TEMPLATE } from "../../src/models/MSC3089TreeSpace"; import { EventType, @@ -52,6 +58,27 @@ jest.mock("../../src/webrtc/call", () => ({ supportsMatrixCall: jest.fn(() => false), })); +enum AnsiColorCode { + Red = 31, + Green = 32, + Yellow = 33, +} +// Color text in the terminal +function decorateStringWithAnsiColor(inputString: string, decorationColor: AnsiColorCode) { + return `\x1b[${decorationColor}m${inputString}\x1b[0m`; +} + +function convertQueryDictToStringRecord(queryDict?: QueryDict): Record { + if (!queryDict) { + return {}; + } + + return Object.entries(queryDict).reduce((resultant, [key, value]) => { + resultant[key] = String(value); + return resultant; + }, {}); +} + describe("MatrixClient", function() { const userId = "@alice:bar"; const identityServerUrl = "https://identity.server"; @@ -92,10 +119,11 @@ describe("MatrixClient", function() { let httpLookups: { method: string; path: string; + prefix?: string; data?: object; error?: object; expectBody?: object; - expectQueryParams?: object; + expectQueryParams?: QueryDict; thenCall?: Function; }[] = []; let acceptKeepalives: boolean; @@ -104,7 +132,14 @@ describe("MatrixClient", function() { method: string; path: string; } | null = null; - function httpReq(method, path, qp, data, prefix) { + function httpReq( + method: Method, + path: string, + queryParams?: QueryDict, + body?: Body, + requestOpts: IRequestOpts = {} + ) { + const { prefix } = requestOpts; if (path === KEEP_ALIVE_PATH && acceptKeepalives) { return Promise.resolve({ unstable_features: { @@ -135,17 +170,20 @@ 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]); }); } @@ -165,12 +203,15 @@ 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(() => {}); + // If you're seeing this then you forgot to handle at least 1 pending request. + const receivedRequest = decorateStringWithAnsiColor(`${method} ${prefix}${path}${new URLSearchParams(convertQueryDictToStringRecord(queryParams)).toString()}`, AnsiColorCode.Red); + const expectedRequest = decorateStringWithAnsiColor(`${next.method} ${next.prefix ?? ''}${next.path}${new URLSearchParams(convertQueryDictToStringRecord(next.expectQueryParams)).toString()}`, AnsiColorCode.Green); + throw new Error( + `A pending request was not handled: ${receivedRequest} ` + + `(next request expected was ${expectedRequest})\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() { @@ -231,6 +272,118 @@ 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"; + + it('should call stable endpoint', async () => { + httpLookups = [{ + method: "GET", + path: `/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`, + data: { event_id: eventId }, + expectQueryParams: { + ts: '0', + dir: 'f' + }, + }]; + + await client.timestampToEvent(roomId, 0, 'f'); + + expect(client.http.authedRequest.mock.calls.length).toStrictEqual(1); + const [method, path, queryParams,, { prefix }] = client.http.authedRequest.mock.calls[0]; + expect(method).toStrictEqual('GET'); + expect(prefix).toStrictEqual(ClientPrefix.V1); + expect(path).toStrictEqual( + `/rooms/${encodeURIComponent(roomId)}/timestamp_to_event` + ); + expect(queryParams).toStrictEqual({ + ts: '0', + dir: 'f' + }); + }); + + it('should fallback to unstable endpoint when no support for stable endpoint', async () => { + httpLookups = [{ + method: "GET", + path: `/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`, + prefix: ClientPrefix.V1, + error: { + httpStatus: 404, + errcode: "M_UNRECOGNIZED" + }, + expectQueryParams: { + ts: '0', + dir: 'f' + }, + }, { + method: "GET", + path: `/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`, + prefix: unstableMsc3030Prefix, + data: { event_id: eventId }, + expectQueryParams: { + ts: '0', + dir: 'f' + }, + }]; + + await client.timestampToEvent(roomId, 0, 'f'); + + expect(client.http.authedRequest.mock.calls.length).toStrictEqual(2); + const [stableMethod, stablePath, stableQueryParams,, { prefix: stablePrefix }] = client.http.authedRequest.mock.calls[0]; + expect(stableMethod).toStrictEqual('GET'); + expect(stablePrefix).toStrictEqual(ClientPrefix.V1); + expect(stablePath).toStrictEqual( + `/rooms/${encodeURIComponent(roomId)}/timestamp_to_event` + ); + expect(stableQueryParams).toStrictEqual({ + ts: '0', + dir: 'f' + }); + + const [unstableMethod, unstablePath, unstableQueryParams,, { prefix: unstablePrefix }] = client.http.authedRequest.mock.calls[1]; + expect(unstableMethod).toStrictEqual('GET'); + expect(unstablePrefix).toStrictEqual(unstableMsc3030Prefix); + expect(unstablePath).toStrictEqual( + `/rooms/${encodeURIComponent(roomId)}/timestamp_to_event` + ); + expect(unstableQueryParams).toStrictEqual({ + ts: '0', + dir: 'f' + }); + }); + + it('should not fallback to unstable endpoint when stable endpoint returns an error', async () => { + httpLookups = [{ + method: "GET", + path: `/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`, + prefix: ClientPrefix.V1, + error: { + httpStatus: 500, + errcode: "Fake response error" + }, + expectQueryParams: { + ts: '0', + dir: 'f' + }, + }]; + + await expect(client.timestampToEvent(roomId, 0, 'f')).rejects.toBeDefined(); + + expect(client.http.authedRequest.mock.calls.length).toStrictEqual(1); + const [method, path, queryParams,, { prefix }] = client.http.authedRequest.mock.calls[0]; + expect(method).toStrictEqual('GET'); + expect(prefix).toStrictEqual(ClientPrefix.V1); + expect(path).toStrictEqual( + `/rooms/${encodeURIComponent(roomId)}/timestamp_to_event` + ); + expect(queryParams).toStrictEqual({ + ts: '0', + dir: 'f' + }); + }); + }); + describe("sendEvent", () => { const roomId = "!room:example.org"; const body = "This is the body"; diff --git a/src/client.ts b/src/client.ts index 2e0492eb12d..5b19ef08d0e 100644 --- a/src/client.ts +++ b/src/client.ts @@ -9328,7 +9328,16 @@ export class MatrixClient extends TypedEventEmittererr).httpStatus === 400 && (err).errcode === "M_UNRECOGNIZED") { + 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. + (err).httpStatus === 404 + ) + ) { return await this.http.authedRequest( Method.Get, path, From d1ede036e21f696ab0a40c4f121f1e500ebd27a0 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 30 Nov 2022 18:51:49 -0600 Subject: [PATCH 03/19] Add return type --- spec/unit/matrix-client.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/unit/matrix-client.spec.ts b/spec/unit/matrix-client.spec.ts index 55f99d9af20..e208e87efb7 100644 --- a/spec/unit/matrix-client.spec.ts +++ b/spec/unit/matrix-client.spec.ts @@ -64,7 +64,7 @@ enum AnsiColorCode { Yellow = 33, } // Color text in the terminal -function decorateStringWithAnsiColor(inputString: string, decorationColor: AnsiColorCode) { +function decorateStringWithAnsiColor(inputString: string, decorationColor: AnsiColorCode): string { return `\x1b[${decorationColor}m${inputString}\x1b[0m`; } From 9a731cdf4f421dc81977cc2552d2b597f51f5a61 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 30 Nov 2022 18:53:23 -0600 Subject: [PATCH 04/19] Add some comments --- spec/unit/matrix-client.spec.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/spec/unit/matrix-client.spec.ts b/spec/unit/matrix-client.spec.ts index e208e87efb7..237ffe4ac2d 100644 --- a/spec/unit/matrix-client.spec.ts +++ b/spec/unit/matrix-client.spec.ts @@ -63,11 +63,13 @@ enum AnsiColorCode { Green = 32, Yellow = 33, } -// Color text in the terminal +// Add color to text in the terminal output function decorateStringWithAnsiColor(inputString: string, decorationColor: AnsiColorCode): string { return `\x1b[${decorationColor}m${inputString}\x1b[0m`; } +// Utility function to ease the transition from our QueryDict type to a string record +// which we can use to stringify with URLSearchParams function convertQueryDictToStringRecord(queryDict?: QueryDict): Record { if (!queryDict) { return {}; From ad8bb5d2cd34bea4320a32b482dacd365e13d5f0 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 30 Nov 2022 19:01:20 -0600 Subject: [PATCH 05/19] Fix lints --- spec/unit/matrix-client.spec.ts | 63 +++++++++++++++++++++++---------- 1 file changed, 44 insertions(+), 19 deletions(-) diff --git a/spec/unit/matrix-client.spec.ts b/spec/unit/matrix-client.spec.ts index 237ffe4ac2d..b8ee2eac938 100644 --- a/spec/unit/matrix-client.spec.ts +++ b/spec/unit/matrix-client.spec.ts @@ -139,7 +139,7 @@ describe("MatrixClient", function() { path: string, queryParams?: QueryDict, body?: Body, - requestOpts: IRequestOpts = {} + requestOpts: IRequestOpts = {}, ) { const { prefix } = requestOpts; if (path === KEEP_ALIVE_PATH && acceptKeepalives) { @@ -205,9 +205,22 @@ describe("MatrixClient", function() { } return Promise.resolve(next.data); } + + const receivedRequestQueryString = new URLSearchParams( + convertQueryDictToStringRecord(queryParams), + ).toString(); + const receivedRequest = decorateStringWithAnsiColor( + `${method} ${prefix}${path}${receivedRequestQueryString}`, + AnsiColorCode.Red, + ); + const expectedQueryString = new URLSearchParams( + convertQueryDictToStringRecord(next.expectQueryParams), + ).toString(); + const expectedRequest = decorateStringWithAnsiColor( + `${next.method} ${next.prefix ?? ''}${next.path}${expectedQueryString}`, + AnsiColorCode.Green, + ); // If you're seeing this then you forgot to handle at least 1 pending request. - const receivedRequest = decorateStringWithAnsiColor(`${method} ${prefix}${path}${new URLSearchParams(convertQueryDictToStringRecord(queryParams)).toString()}`, AnsiColorCode.Red); - const expectedRequest = decorateStringWithAnsiColor(`${next.method} ${next.prefix ?? ''}${next.path}${new URLSearchParams(convertQueryDictToStringRecord(next.expectQueryParams)).toString()}`, AnsiColorCode.Green); throw new Error( `A pending request was not handled: ${receivedRequest} ` + `(next request expected was ${expectedRequest})\n` + @@ -286,7 +299,7 @@ describe("MatrixClient", function() { data: { event_id: eventId }, expectQueryParams: { ts: '0', - dir: 'f' + dir: 'f', }, }]; @@ -297,11 +310,11 @@ describe("MatrixClient", function() { expect(method).toStrictEqual('GET'); expect(prefix).toStrictEqual(ClientPrefix.V1); expect(path).toStrictEqual( - `/rooms/${encodeURIComponent(roomId)}/timestamp_to_event` + `/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`, ); expect(queryParams).toStrictEqual({ ts: '0', - dir: 'f' + dir: 'f', }); }); @@ -312,11 +325,11 @@ describe("MatrixClient", function() { prefix: ClientPrefix.V1, error: { httpStatus: 404, - errcode: "M_UNRECOGNIZED" + errcode: "M_UNRECOGNIZED", }, expectQueryParams: { ts: '0', - dir: 'f' + dir: 'f', }, }, { method: "GET", @@ -325,33 +338,45 @@ describe("MatrixClient", function() { data: { event_id: eventId }, expectQueryParams: { ts: '0', - dir: 'f' + dir: 'f', }, }]; await client.timestampToEvent(roomId, 0, 'f'); expect(client.http.authedRequest.mock.calls.length).toStrictEqual(2); - const [stableMethod, stablePath, stableQueryParams,, { prefix: stablePrefix }] = client.http.authedRequest.mock.calls[0]; + const [ + stableMethod, + stablePath, + stableQueryParams, + , + { prefix: stablePrefix }, + ] = client.http.authedRequest.mock.calls[0]; expect(stableMethod).toStrictEqual('GET'); expect(stablePrefix).toStrictEqual(ClientPrefix.V1); expect(stablePath).toStrictEqual( - `/rooms/${encodeURIComponent(roomId)}/timestamp_to_event` + `/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`, ); expect(stableQueryParams).toStrictEqual({ ts: '0', - dir: 'f' + dir: 'f', }); - const [unstableMethod, unstablePath, unstableQueryParams,, { prefix: unstablePrefix }] = client.http.authedRequest.mock.calls[1]; + const [ + unstableMethod, + unstablePath, + unstableQueryParams, + , + { prefix: unstablePrefix }, + ] = client.http.authedRequest.mock.calls[1]; expect(unstableMethod).toStrictEqual('GET'); expect(unstablePrefix).toStrictEqual(unstableMsc3030Prefix); expect(unstablePath).toStrictEqual( - `/rooms/${encodeURIComponent(roomId)}/timestamp_to_event` + `/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`, ); expect(unstableQueryParams).toStrictEqual({ ts: '0', - dir: 'f' + dir: 'f', }); }); @@ -362,11 +387,11 @@ describe("MatrixClient", function() { prefix: ClientPrefix.V1, error: { httpStatus: 500, - errcode: "Fake response error" + errcode: "Fake response error", }, expectQueryParams: { ts: '0', - dir: 'f' + dir: 'f', }, }]; @@ -377,11 +402,11 @@ describe("MatrixClient", function() { expect(method).toStrictEqual('GET'); expect(prefix).toStrictEqual(ClientPrefix.V1); expect(path).toStrictEqual( - `/rooms/${encodeURIComponent(roomId)}/timestamp_to_event` + `/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`, ); expect(queryParams).toStrictEqual({ ts: '0', - dir: 'f' + dir: 'f', }); }); }); From 9a98e8008f3699164609819596daad49df86be12 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 30 Nov 2022 19:13:29 -0600 Subject: [PATCH 06/19] Fix relevant strict ts error --- spec/unit/matrix-client.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/unit/matrix-client.spec.ts b/spec/unit/matrix-client.spec.ts index b8ee2eac938..180f44bd229 100644 --- a/spec/unit/matrix-client.spec.ts +++ b/spec/unit/matrix-client.spec.ts @@ -78,7 +78,7 @@ function convertQueryDictToStringRecord(queryDict?: QueryDict): Record { resultant[key] = String(value); return resultant; - }, {}); + }, {} as Record); } describe("MatrixClient", function() { From bf78a64d821ab6581932624a27dbe00185ebac2b Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 8 Dec 2022 17:47:56 -0600 Subject: [PATCH 07/19] Remove console coloring in favor of future PR See https://github.com/matrix-org/matrix-js-sdk/pull/2915#discussion_r1041539703 --- spec/unit/matrix-client.spec.ts | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/spec/unit/matrix-client.spec.ts b/spec/unit/matrix-client.spec.ts index 180f44bd229..f1cb7a67b07 100644 --- a/spec/unit/matrix-client.spec.ts +++ b/spec/unit/matrix-client.spec.ts @@ -58,16 +58,6 @@ jest.mock("../../src/webrtc/call", () => ({ supportsMatrixCall: jest.fn(() => false), })); -enum AnsiColorCode { - Red = 31, - Green = 32, - Yellow = 33, -} -// Add color to text in the terminal output -function decorateStringWithAnsiColor(inputString: string, decorationColor: AnsiColorCode): string { - return `\x1b[${decorationColor}m${inputString}\x1b[0m`; -} - // Utility function to ease the transition from our QueryDict type to a string record // which we can use to stringify with URLSearchParams function convertQueryDictToStringRecord(queryDict?: QueryDict): Record { @@ -209,21 +199,15 @@ describe("MatrixClient", function() { const receivedRequestQueryString = new URLSearchParams( convertQueryDictToStringRecord(queryParams), ).toString(); - const receivedRequest = decorateStringWithAnsiColor( - `${method} ${prefix}${path}${receivedRequestQueryString}`, - AnsiColorCode.Red, - ); + const receivedRequestDebugString = `${method} ${prefix}${path}${receivedRequestQueryString}`; const expectedQueryString = new URLSearchParams( convertQueryDictToStringRecord(next.expectQueryParams), ).toString(); - const expectedRequest = decorateStringWithAnsiColor( - `${next.method} ${next.prefix ?? ''}${next.path}${expectedQueryString}`, - AnsiColorCode.Green, - ); + 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: ${receivedRequest} ` + - `(next request expected was ${expectedRequest})\n` + + `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.`, ); From c953fc9fb7f8b02848d552b3045d3985322ec2f8 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 8 Dec 2022 17:56:53 -0600 Subject: [PATCH 08/19] Update casing See https://github.com/matrix-org/matrix-js-sdk/pull/2915#discussion_r1041542066 --- spec/unit/matrix-client.spec.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/unit/matrix-client.spec.ts b/spec/unit/matrix-client.spec.ts index f1cb7a67b07..56124449c35 100644 --- a/spec/unit/matrix-client.spec.ts +++ b/spec/unit/matrix-client.spec.ts @@ -274,7 +274,7 @@ describe("MatrixClient", function() { describe('timestampToEvent', () => { const roomId = '!room:server.org'; const eventId = "$eventId:example.org"; - const unstableMsc3030Prefix = "/_matrix/client/unstable/org.matrix.msc3030"; + const unstableMSC3030Prefix = "/_matrix/client/unstable/org.matrix.msc3030"; it('should call stable endpoint', async () => { httpLookups = [{ @@ -318,7 +318,7 @@ describe("MatrixClient", function() { }, { method: "GET", path: `/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`, - prefix: unstableMsc3030Prefix, + prefix: unstableMSC3030Prefix, data: { event_id: eventId }, expectQueryParams: { ts: '0', @@ -354,7 +354,7 @@ describe("MatrixClient", function() { { prefix: unstablePrefix }, ] = client.http.authedRequest.mock.calls[1]; expect(unstableMethod).toStrictEqual('GET'); - expect(unstablePrefix).toStrictEqual(unstableMsc3030Prefix); + expect(unstablePrefix).toStrictEqual(unstableMSC3030Prefix); expect(unstablePath).toStrictEqual( `/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`, ); From 9841f92415b925da7d9bac5da43681c98377431c Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 8 Dec 2022 18:37:33 -0600 Subject: [PATCH 09/19] Fix some eslint --- src/client.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client.ts b/src/client.ts index b37362d9168..daac552fc62 100644 --- a/src/client.ts +++ b/src/client.ts @@ -9311,9 +9311,9 @@ export class MatrixClient extends TypedEventEmitter Date: Tue, 13 Dec 2022 16:26:14 -0600 Subject: [PATCH 10/19] Prettier fixes --- spec/unit/matrix-client.spec.ts | 173 +++++++++++++++----------------- src/client.ts | 65 +++++------- 2 files changed, 107 insertions(+), 131 deletions(-) diff --git a/spec/unit/matrix-client.spec.ts b/spec/unit/matrix-client.spec.ts index 03d9d0d2445..0be036b0fc2 100644 --- a/spec/unit/matrix-client.spec.ts +++ b/spec/unit/matrix-client.spec.ts @@ -220,20 +220,18 @@ describe("MatrixClient", function () { return Promise.resolve(next.data); } - const receivedRequestQueryString = new URLSearchParams( - convertQueryDictToStringRecord(queryParams), - ).toString(); + const receivedRequestQueryString = new URLSearchParams(convertQueryDictToStringRecord(queryParams)).toString(); const receivedRequestDebugString = `${method} ${prefix}${path}${receivedRequestQueryString}`; const expectedQueryString = new URLSearchParams( convertQueryDictToStringRecord(next.expectQueryParams), ).toString(); - const expectedRequestDebugString = `${next.method} ${next.prefix ?? ''}${next.path}${expectedQueryString}`; + 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.`, + `(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.`, ); } @@ -317,126 +315,117 @@ describe("MatrixClient", function () { client.stopClient(); }); - describe('timestampToEvent', () => { - const roomId = '!room:server.org'; + describe("timestampToEvent", () => { + const roomId = "!room:server.org"; const eventId = "$eventId:example.org"; const unstableMSC3030Prefix = "/_matrix/client/unstable/org.matrix.msc3030"; - it('should call stable endpoint', async () => { - httpLookups = [{ - method: "GET", - path: `/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`, - data: { event_id: eventId }, - expectQueryParams: { - ts: '0', - dir: 'f', + it("should call stable endpoint", async () => { + httpLookups = [ + { + method: "GET", + path: `/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`, + data: { event_id: eventId }, + expectQueryParams: { + ts: "0", + dir: "f", + }, }, - }]; + ]; await client.timestampToEvent(roomId, 0, Direction.Forward); expect(mocked(client.http.authedRequest).mock.calls.length).toStrictEqual(1); - const [method, path, queryParams,, { prefix }] = mocked(client.http.authedRequest).mock.calls[0]; - expect(method).toStrictEqual('GET'); + const [method, path, queryParams, , { prefix }] = mocked(client.http.authedRequest).mock.calls[0]; + expect(method).toStrictEqual("GET"); expect(prefix).toStrictEqual(ClientPrefix.V1); - expect(path).toStrictEqual( - `/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`, - ); + expect(path).toStrictEqual(`/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`); expect(queryParams).toStrictEqual({ - ts: '0', - dir: 'f', + ts: "0", + dir: "f", }); }); - it('should fallback to unstable endpoint when no support for stable endpoint', async () => { - httpLookups = [{ - method: "GET", - path: `/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`, - prefix: ClientPrefix.V1, - error: { - httpStatus: 404, - errcode: "M_UNRECOGNIZED", - }, - expectQueryParams: { - ts: '0', - dir: 'f', + it("should fallback to unstable endpoint when no support for stable endpoint", async () => { + httpLookups = [ + { + method: "GET", + path: `/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`, + prefix: ClientPrefix.V1, + error: { + httpStatus: 404, + errcode: "M_UNRECOGNIZED", + }, + expectQueryParams: { + ts: "0", + dir: "f", + }, }, - }, { - method: "GET", - path: `/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`, - prefix: unstableMSC3030Prefix, - data: { event_id: eventId }, - expectQueryParams: { - ts: '0', - dir: 'f', + { + method: "GET", + path: `/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`, + prefix: unstableMSC3030Prefix, + data: { event_id: eventId }, + expectQueryParams: { + ts: "0", + dir: "f", + }, }, - }]; + ]; await client.timestampToEvent(roomId, 0, Direction.Forward); expect(mocked(client.http.authedRequest).mock.calls.length).toStrictEqual(2); - const [ - stableMethod, - stablePath, - stableQueryParams, - , - { prefix: stablePrefix }, - ] = mocked(client.http.authedRequest).mock.calls[0]; - expect(stableMethod).toStrictEqual('GET'); + const [stableMethod, stablePath, stableQueryParams, , { prefix: stablePrefix }] = mocked( + client.http.authedRequest, + ).mock.calls[0]; + expect(stableMethod).toStrictEqual("GET"); expect(stablePrefix).toStrictEqual(ClientPrefix.V1); - expect(stablePath).toStrictEqual( - `/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`, - ); + expect(stablePath).toStrictEqual(`/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`); expect(stableQueryParams).toStrictEqual({ - ts: '0', - dir: 'f', + ts: "0", + dir: "f", }); - const [ - unstableMethod, - unstablePath, - unstableQueryParams, - , - { prefix: unstablePrefix }, - ] = mocked(client.http.authedRequest).mock.calls[1]; - expect(unstableMethod).toStrictEqual('GET'); + const [unstableMethod, unstablePath, unstableQueryParams, , { prefix: unstablePrefix }] = mocked( + client.http.authedRequest, + ).mock.calls[1]; + expect(unstableMethod).toStrictEqual("GET"); expect(unstablePrefix).toStrictEqual(unstableMSC3030Prefix); - expect(unstablePath).toStrictEqual( - `/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`, - ); + expect(unstablePath).toStrictEqual(`/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`); expect(unstableQueryParams).toStrictEqual({ - ts: '0', - dir: 'f', + ts: "0", + dir: "f", }); }); - it('should not fallback to unstable endpoint when stable endpoint returns an error', async () => { - httpLookups = [{ - method: "GET", - path: `/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`, - prefix: ClientPrefix.V1, - error: { - httpStatus: 500, - errcode: "Fake response error", - }, - expectQueryParams: { - ts: '0', - dir: 'f', + it("should not fallback to unstable endpoint when stable endpoint returns an error", async () => { + httpLookups = [ + { + method: "GET", + path: `/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`, + prefix: ClientPrefix.V1, + error: { + httpStatus: 500, + errcode: "Fake response error", + }, + expectQueryParams: { + ts: "0", + dir: "f", + }, }, - }]; + ]; await expect(client.timestampToEvent(roomId, 0, Direction.Forward)).rejects.toBeDefined(); expect(mocked(client.http.authedRequest).mock.calls.length).toStrictEqual(1); - const [method, path, queryParams,, { prefix }] = mocked(client.http.authedRequest).mock.calls[0]; - expect(method).toStrictEqual('GET'); + const [method, path, queryParams, , { prefix }] = mocked(client.http.authedRequest).mock.calls[0]; + expect(method).toStrictEqual("GET"); expect(prefix).toStrictEqual(ClientPrefix.V1); - expect(path).toStrictEqual( - `/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`, - ); + expect(path).toStrictEqual(`/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`); expect(queryParams).toStrictEqual({ - ts: '0', - dir: 'f', + ts: "0", + dir: "f", }); }); }); diff --git a/src/client.ts b/src/client.ts index be5c7c5fb1f..10f59eca50e 100644 --- a/src/client.ts +++ b/src/client.ts @@ -480,9 +480,9 @@ export interface ICapability { enabled: boolean; } -export interface IChangePasswordCapability extends ICapability { } +export interface IChangePasswordCapability extends ICapability {} -export interface IThreadsCapability extends ICapability { } +export interface IThreadsCapability extends ICapability {} interface ICapabilities { [key: string]: any; @@ -1244,12 +1244,12 @@ export class MatrixClient extends TypedEventEmittererr).errcode === "M_UNRECOGNIZED" && ( - // XXX: The 400 status code check should be removed in the future - // when Synapse is compliant with MSC3743. - (err).httpStatus === 400 || + (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. - (err).httpStatus === 404 - ) + (err).httpStatus === 404) ) { - return await this.http.authedRequest( - Method.Get, - path, - queryParams, - undefined, - { - prefix: "/_matrix/client/unstable/org.matrix.msc3030", - }, - ); + return await this.http.authedRequest(Method.Get, path, queryParams, undefined, { + prefix: "/_matrix/client/unstable/org.matrix.msc3030", + }); } throw err; @@ -9338,12 +9325,12 @@ export function fixNotificationCountOnDecryption(cli: MatrixClient, event: Matri hasReadEvent = thread ? thread.hasUserReadEvent(cli.getUserId()!, event.getId()!) : // If the thread object does not exist in the room yet, we don't - // want to calculate notification for this event yet. We have not - // restored the read receipts yet and can't accurately calculate - // highlight notifications at this stage. - // - // This issue can likely go away when MSC3874 is implemented - true; + // want to calculate notification for this event yet. We have not + // restored the read receipts yet and can't accurately calculate + // highlight notifications at this stage. + // + // This issue can likely go away when MSC3874 is implemented + true; } else { hasReadEvent = room.hasUserReadEvent(cli.getUserId()!, event.getId()!); } From a0aa5074ed95514737f7a3e58c912a1be849ea3f Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 13 Dec 2022 16:37:47 -0600 Subject: [PATCH 11/19] Fix prefix lint See https://github.com/matrix-org/matrix-js-sdk/pull/2915#discussion_r1043951639 --- spec/unit/matrix-client.spec.ts | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/spec/unit/matrix-client.spec.ts b/spec/unit/matrix-client.spec.ts index 0be036b0fc2..f5c44006726 100644 --- a/spec/unit/matrix-client.spec.ts +++ b/spec/unit/matrix-client.spec.ts @@ -336,7 +336,8 @@ describe("MatrixClient", function () { await client.timestampToEvent(roomId, 0, Direction.Forward); expect(mocked(client.http.authedRequest).mock.calls.length).toStrictEqual(1); - const [method, path, queryParams, , { prefix }] = mocked(client.http.authedRequest).mock.calls[0]; + const [method, path, queryParams, , { prefix } = { prefix: undefined }] = mocked(client.http.authedRequest) + .mock.calls[0]; expect(method).toStrictEqual("GET"); expect(prefix).toStrictEqual(ClientPrefix.V1); expect(path).toStrictEqual(`/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`); @@ -376,9 +377,8 @@ describe("MatrixClient", function () { await client.timestampToEvent(roomId, 0, Direction.Forward); expect(mocked(client.http.authedRequest).mock.calls.length).toStrictEqual(2); - const [stableMethod, stablePath, stableQueryParams, , { prefix: stablePrefix }] = mocked( - client.http.authedRequest, - ).mock.calls[0]; + const [stableMethod, stablePath, stableQueryParams, , { prefix: stablePrefix } = { prefix: undefined }] = + mocked(client.http.authedRequest).mock.calls[0]; expect(stableMethod).toStrictEqual("GET"); expect(stablePrefix).toStrictEqual(ClientPrefix.V1); expect(stablePath).toStrictEqual(`/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`); @@ -387,9 +387,13 @@ describe("MatrixClient", function () { dir: "f", }); - const [unstableMethod, unstablePath, unstableQueryParams, , { prefix: unstablePrefix }] = mocked( - client.http.authedRequest, - ).mock.calls[1]; + const [ + unstableMethod, + unstablePath, + unstableQueryParams, + , + { prefix: unstablePrefix } = { prefix: undefined }, + ] = mocked(client.http.authedRequest).mock.calls[1]; expect(unstableMethod).toStrictEqual("GET"); expect(unstablePrefix).toStrictEqual(unstableMSC3030Prefix); expect(unstablePath).toStrictEqual(`/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`); @@ -419,7 +423,8 @@ describe("MatrixClient", function () { await expect(client.timestampToEvent(roomId, 0, Direction.Forward)).rejects.toBeDefined(); expect(mocked(client.http.authedRequest).mock.calls.length).toStrictEqual(1); - const [method, path, queryParams, , { prefix }] = mocked(client.http.authedRequest).mock.calls[0]; + const [method, path, queryParams, , { prefix } = { prefix: undefined }] = mocked(client.http.authedRequest) + .mock.calls[0]; expect(method).toStrictEqual("GET"); expect(prefix).toStrictEqual(ClientPrefix.V1); expect(path).toStrictEqual(`/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`); From ca98d9ff111dc5357d166dcb87f3db564e6ee802 Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Fri, 6 Jan 2023 13:59:34 +0000 Subject: [PATCH 12/19] Tests for convertQueryDictToStringRecord --- spec/unit/matrix-client.spec.ts | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/spec/unit/matrix-client.spec.ts b/spec/unit/matrix-client.spec.ts index 2b48b69c392..b03c9b7f376 100644 --- a/spec/unit/matrix-client.spec.ts +++ b/spec/unit/matrix-client.spec.ts @@ -101,6 +101,33 @@ type WrappedRoom = Room & { _state: Map; }; +describe("convertQueryDictToStringRecord", () => { + it("returns an empty map when dict is undefined", () => { + expect(convertQueryDictToStringRecord(undefined)).toEqual({}); + }); + + it("converts an empty QueryDict to an empty map", () => { + expect(convertQueryDictToStringRecord({})).toEqual({}); + }); + + it("converts a QueryDict of strings to the equivalent map", () => { + expect(convertQueryDictToStringRecord({ a: "b", c: "d" })).toEqual({ a: "b", c: "d" }); + }); + + it("converts the values of the supplied QueryDict to strings", () => { + expect(convertQueryDictToStringRecord({ arr: ["b", "c"], num: 45, boo: true, und: undefined })).toEqual({ + arr: "b,c", + num: "45", + boo: "true", + und: "undefined", + }); + }); + + it("produces sane URLSearchParams conversions", () => { + expect(new URLSearchParams(convertQueryDictToStringRecord({ a: "b", c: "d" })).toString()).toEqual("a=b&c=d"); + }); +}); + describe("MatrixClient", function () { const userId = "@alice:bar"; const identityServerUrl = "https://identity.server"; From b1566ee54056d373e76109ff4bc28df0debecd4c Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Fri, 6 Jan 2023 14:16:21 +0000 Subject: [PATCH 13/19] Switch to a Map for convertQueryDictToStringRecord --- spec/unit/matrix-client.spec.ts | 44 +++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/spec/unit/matrix-client.spec.ts b/spec/unit/matrix-client.spec.ts index b03c9b7f376..ce073aa7a69 100644 --- a/spec/unit/matrix-client.spec.ts +++ b/spec/unit/matrix-client.spec.ts @@ -70,15 +70,12 @@ jest.mock("../../src/webrtc/call", () => ({ // Utility function to ease the transition from our QueryDict type to a string record // which we can use to stringify with URLSearchParams -function convertQueryDictToStringRecord(queryDict?: QueryDict): Record { +function convertQueryDictToStringRecord(queryDict?: QueryDict): Map { if (!queryDict) { - return {}; + return new Map(); } - return Object.entries(queryDict).reduce((resultant, [key, value]) => { - resultant[key] = String(value); - return resultant; - }, {} as Record); + return new Map(Object.entries(queryDict).map(([k, v]) => [k, String(v)])); } type HttpLookup = { @@ -103,28 +100,37 @@ type WrappedRoom = Room & { describe("convertQueryDictToStringRecord", () => { it("returns an empty map when dict is undefined", () => { - expect(convertQueryDictToStringRecord(undefined)).toEqual({}); + expect(convertQueryDictToStringRecord(undefined)).toEqual(new Map()); }); it("converts an empty QueryDict to an empty map", () => { - expect(convertQueryDictToStringRecord({})).toEqual({}); + expect(convertQueryDictToStringRecord({})).toEqual(new Map()); }); it("converts a QueryDict of strings to the equivalent map", () => { - expect(convertQueryDictToStringRecord({ a: "b", c: "d" })).toEqual({ a: "b", c: "d" }); + expect(convertQueryDictToStringRecord({ a: "b", c: "d" })).toEqual( + new Map([ + ["a", "b"], + ["c", "d"], + ]), + ); }); it("converts the values of the supplied QueryDict to strings", () => { - expect(convertQueryDictToStringRecord({ arr: ["b", "c"], num: 45, boo: true, und: undefined })).toEqual({ - arr: "b,c", - num: "45", - boo: "true", - und: "undefined", - }); + expect(convertQueryDictToStringRecord({ 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(convertQueryDictToStringRecord({ a: "b", c: "d" })).toString()).toEqual("a=b&c=d"); + expect(new URLSearchParams(Array.from(convertQueryDictToStringRecord({ a: "b", c: "d" }))).toString()).toEqual( + "a=b&c=d", + ); }); }); @@ -248,10 +254,12 @@ describe("MatrixClient", function () { return Promise.resolve(next.data); } - const receivedRequestQueryString = new URLSearchParams(convertQueryDictToStringRecord(queryParams)).toString(); + const receivedRequestQueryString = new URLSearchParams( + Array.from(convertQueryDictToStringRecord(queryParams)), + ).toString(); const receivedRequestDebugString = `${method} ${prefix}${path}${receivedRequestQueryString}`; const expectedQueryString = new URLSearchParams( - convertQueryDictToStringRecord(next.expectQueryParams), + Array.from(convertQueryDictToStringRecord(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. From d7442147b9a00c951e93bef44eee33a6f590b803 Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Fri, 6 Jan 2023 14:21:35 +0000 Subject: [PATCH 14/19] Rename convertQueryDictToMap --- spec/unit/matrix-client.spec.ts | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/spec/unit/matrix-client.spec.ts b/spec/unit/matrix-client.spec.ts index ce073aa7a69..19c8f176c79 100644 --- a/spec/unit/matrix-client.spec.ts +++ b/spec/unit/matrix-client.spec.ts @@ -68,9 +68,9 @@ jest.mock("../../src/webrtc/call", () => ({ supportsMatrixCall: jest.fn(() => false), })); -// Utility function to ease the transition from our QueryDict type to a string record -// which we can use to stringify with URLSearchParams -function convertQueryDictToStringRecord(queryDict?: QueryDict): Map { +// 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(); } @@ -100,15 +100,15 @@ type WrappedRoom = Room & { describe("convertQueryDictToStringRecord", () => { it("returns an empty map when dict is undefined", () => { - expect(convertQueryDictToStringRecord(undefined)).toEqual(new Map()); + expect(convertQueryDictToMap(undefined)).toEqual(new Map()); }); it("converts an empty QueryDict to an empty map", () => { - expect(convertQueryDictToStringRecord({})).toEqual(new Map()); + expect(convertQueryDictToMap({})).toEqual(new Map()); }); it("converts a QueryDict of strings to the equivalent map", () => { - expect(convertQueryDictToStringRecord({ a: "b", c: "d" })).toEqual( + expect(convertQueryDictToMap({ a: "b", c: "d" })).toEqual( new Map([ ["a", "b"], ["c", "d"], @@ -117,7 +117,7 @@ describe("convertQueryDictToStringRecord", () => { }); it("converts the values of the supplied QueryDict to strings", () => { - expect(convertQueryDictToStringRecord({ arr: ["b", "c"], num: 45, boo: true, und: undefined })).toEqual( + expect(convertQueryDictToMap({ arr: ["b", "c"], num: 45, boo: true, und: undefined })).toEqual( new Map([ ["arr", "b,c"], ["num", "45"], @@ -128,7 +128,7 @@ describe("convertQueryDictToStringRecord", () => { }); it("produces sane URLSearchParams conversions", () => { - expect(new URLSearchParams(Array.from(convertQueryDictToStringRecord({ a: "b", c: "d" }))).toString()).toEqual( + expect(new URLSearchParams(Array.from(convertQueryDictToMap({ a: "b", c: "d" }))).toString()).toEqual( "a=b&c=d", ); }); @@ -255,11 +255,11 @@ describe("MatrixClient", function () { } const receivedRequestQueryString = new URLSearchParams( - Array.from(convertQueryDictToStringRecord(queryParams)), + Array.from(convertQueryDictToMap(queryParams)), ).toString(); const receivedRequestDebugString = `${method} ${prefix}${path}${receivedRequestQueryString}`; const expectedQueryString = new URLSearchParams( - Array.from(convertQueryDictToStringRecord(next.expectQueryParams)), + 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. From c4ca0b2e07ed991705b430ccf34e2fba3a5b7d31 Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Fri, 6 Jan 2023 15:13:44 +0000 Subject: [PATCH 15/19] Refactor timestampToEvent tests --- spec/unit/matrix-client.spec.ts | 158 ++++++++++++++------------------ 1 file changed, 68 insertions(+), 90 deletions(-) diff --git a/spec/unit/matrix-client.spec.ts b/spec/unit/matrix-client.spec.ts index 19c8f176c79..9a2ae6c978a 100644 --- a/spec/unit/matrix-client.spec.ts +++ b/spec/unit/matrix-client.spec.ts @@ -356,118 +356,96 @@ describe("MatrixClient", function () { 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 () => { - httpLookups = [ + await assertRequestsMade([ { - method: "GET", - path: `/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`, data: { event_id: eventId }, - expectQueryParams: { - ts: "0", - dir: "f", - }, }, - ]; - - await client.timestampToEvent(roomId, 0, Direction.Forward); - - expect(mocked(client.http.authedRequest).mock.calls.length).toStrictEqual(1); - const [method, path, queryParams, , { prefix } = { prefix: undefined }] = mocked(client.http.authedRequest) - .mock.calls[0]; - expect(method).toStrictEqual("GET"); - expect(prefix).toStrictEqual(ClientPrefix.V1); - expect(path).toStrictEqual(`/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`); - expect(queryParams).toStrictEqual({ - ts: "0", - dir: "f", - }); + ]); }); it("should fallback to unstable endpoint when no support for stable endpoint", async () => { - httpLookups = [ + await assertRequestsMade([ { - method: "GET", - path: `/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`, prefix: ClientPrefix.V1, error: { httpStatus: 404, errcode: "M_UNRECOGNIZED", }, - expectQueryParams: { - ts: "0", - dir: "f", - }, }, { - method: "GET", - path: `/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`, prefix: unstableMSC3030Prefix, data: { event_id: eventId }, - expectQueryParams: { - ts: "0", - dir: "f", - }, }, - ]; - - await client.timestampToEvent(roomId, 0, Direction.Forward); - - expect(mocked(client.http.authedRequest).mock.calls.length).toStrictEqual(2); - const [stableMethod, stablePath, stableQueryParams, , { prefix: stablePrefix } = { prefix: undefined }] = - mocked(client.http.authedRequest).mock.calls[0]; - expect(stableMethod).toStrictEqual("GET"); - expect(stablePrefix).toStrictEqual(ClientPrefix.V1); - expect(stablePath).toStrictEqual(`/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`); - expect(stableQueryParams).toStrictEqual({ - ts: "0", - dir: "f", - }); - - const [ - unstableMethod, - unstablePath, - unstableQueryParams, - , - { prefix: unstablePrefix } = { prefix: undefined }, - ] = mocked(client.http.authedRequest).mock.calls[1]; - expect(unstableMethod).toStrictEqual("GET"); - expect(unstablePrefix).toStrictEqual(unstableMSC3030Prefix); - expect(unstablePath).toStrictEqual(`/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`); - expect(unstableQueryParams).toStrictEqual({ - ts: "0", - dir: "f", - }); + ]); }); it("should not fallback to unstable endpoint when stable endpoint returns an error", async () => { - httpLookups = [ - { - method: "GET", - path: `/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`, - prefix: ClientPrefix.V1, - error: { - httpStatus: 500, - errcode: "Fake response error", - }, - expectQueryParams: { - ts: "0", - dir: "f", + await assertRequestsMade( + [ + { + prefix: ClientPrefix.V1, + error: { + httpStatus: 500, + errcode: "Fake response error", + }, }, - }, - ]; - - await expect(client.timestampToEvent(roomId, 0, Direction.Forward)).rejects.toBeDefined(); - - expect(mocked(client.http.authedRequest).mock.calls.length).toStrictEqual(1); - const [method, path, queryParams, , { prefix } = { prefix: undefined }] = mocked(client.http.authedRequest) - .mock.calls[0]; - expect(method).toStrictEqual("GET"); - expect(prefix).toStrictEqual(ClientPrefix.V1); - expect(path).toStrictEqual(`/rooms/${encodeURIComponent(roomId)}/timestamp_to_event`); - expect(queryParams).toStrictEqual({ - ts: "0", - dir: "f", - }); + ], + true, + ); }); }); From 628bcbf33a3dfa3d685ee5bf4bc933beca30590f Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Fri, 6 Jan 2023 15:22:58 +0000 Subject: [PATCH 16/19] Fall back to the unstable endpoint if we receive a 405 status --- spec/unit/matrix-client.spec.ts | 18 +++++++++++++++++- src/client.ts | 5 ++++- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/spec/unit/matrix-client.spec.ts b/spec/unit/matrix-client.spec.ts index 9a2ae6c978a..4756544f301 100644 --- a/spec/unit/matrix-client.spec.ts +++ b/spec/unit/matrix-client.spec.ts @@ -417,7 +417,7 @@ describe("MatrixClient", function () { ]); }); - it("should fallback to unstable endpoint when no support for stable endpoint", async () => { + it("should fallback to unstable endpoint when stable endpoint 404s", async () => { await assertRequestsMade([ { prefix: ClientPrefix.V1, @@ -433,6 +433,22 @@ describe("MatrixClient", function () { ]); }); + 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", async () => { await assertRequestsMade( [ diff --git a/src/client.ts b/src/client.ts index e36ef7c410b..7ec95929927 100644 --- a/src/client.ts +++ b/src/client.ts @@ -9391,7 +9391,10 @@ export class MatrixClient extends TypedEventEmittererr).httpStatus === 400 || // This the correct standard status code for an unsupported // endpoint according to MSC3743. - (err).httpStatus === 404) + (err).httpStatus === 404 || + // This the correct standard status code for an invalid + // method according to MSC3743. + (err).httpStatus === 405) ) { return await this.http.authedRequest(Method.Get, path, queryParams, undefined, { prefix: "/_matrix/client/unstable/org.matrix.msc3030", From 12cc7be31cd7f7891e1f89bdd1636d536e596e75 Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Fri, 6 Jan 2023 15:33:48 +0000 Subject: [PATCH 17/19] Test 400, 429 and 502 responses --- spec/unit/matrix-client.spec.ts | 48 ++++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/spec/unit/matrix-client.spec.ts b/spec/unit/matrix-client.spec.ts index 4756544f301..ba1c94a2986 100644 --- a/spec/unit/matrix-client.spec.ts +++ b/spec/unit/matrix-client.spec.ts @@ -417,6 +417,22 @@ describe("MatrixClient", function () { ]); }); + 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([ { @@ -449,7 +465,7 @@ describe("MatrixClient", function () { ]); }); - it("should not fallback to unstable endpoint when stable endpoint returns an error", async () => { + it("should not fallback to unstable endpoint when stable endpoint returns an error (500)", async () => { await assertRequestsMade( [ { @@ -463,6 +479,36 @@ describe("MatrixClient", function () { 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()", () => { From 981acf0044f89a325c637eb24ed2e6fa53e8faab Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Fri, 6 Jan 2023 16:38:02 +0000 Subject: [PATCH 18/19] Rename test to fit renamed function. Co-authored-by: Eric Eastwood --- spec/unit/matrix-client.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/unit/matrix-client.spec.ts b/spec/unit/matrix-client.spec.ts index 8e31e3bfa14..8d73939a54a 100644 --- a/spec/unit/matrix-client.spec.ts +++ b/spec/unit/matrix-client.spec.ts @@ -100,7 +100,7 @@ type WrappedRoom = Room & { _state: Map; }; -describe("convertQueryDictToStringRecord", () => { +describe("convertQueryDictToMap", () => { it("returns an empty map when dict is undefined", () => { expect(convertQueryDictToMap(undefined)).toEqual(new Map()); }); From c7c16256dfd0ead12f30f21a64544b8dbf8d66ec Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Mon, 9 Jan 2023 16:53:52 +0000 Subject: [PATCH 19/19] Update comment to reflect commonality between 404 and 405 status --- src/client.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/client.ts b/src/client.ts index f39bbf718ef..6a7735b51bf 100644 --- a/src/client.ts +++ b/src/client.ts @@ -9423,10 +9423,10 @@ export class MatrixClient extends TypedEventEmittererr).httpStatus === 400 || // This the correct standard status code for an unsupported - // endpoint according to MSC3743. + // endpoint according to MSC3743. Not Found and Method Not Allowed + // both indicate that this endpoint+verb combination is + // not supported. (err).httpStatus === 404 || - // This the correct standard status code for an invalid - // method according to MSC3743. (err).httpStatus === 405) ) { return await this.http.authedRequest(Method.Get, path, queryParams, undefined, {