diff --git a/spec/unit/http-api/fetch.spec.ts b/spec/unit/http-api/fetch.spec.ts index 0e3bed566..b76944031 100644 --- a/spec/unit/http-api/fetch.spec.ts +++ b/spec/unit/http-api/fetch.spec.ts @@ -14,8 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { type Mocked } from "jest-mock"; - +import type { Mocked, MockedFunction } from "jest-mock"; import { FetchHttpApi } from "../../../src/http-api/fetch"; import { TypedEventEmitter } from "../../../src/models/typed-event-emitter"; import { @@ -43,8 +42,8 @@ describe("FetchHttpApi", () => { }); it("should support aborting multiple times", () => { - const fetchFn = jest.fn().mockResolvedValue({ ok: true }); - const api = new FetchHttpApi(new TypedEventEmitter(), { baseUrl, prefix, fetchFn }); + const fetchFn = makeMockFetchFn(); + const api = new FetchHttpApi(new TypedEventEmitter(), { baseUrl, prefix, fetchFn, onlyData: true }); api.request(Method.Get, "/foo"); api.request(Method.Get, "/baz"); @@ -68,13 +67,13 @@ describe("FetchHttpApi", () => { it("should fall back to global fetch if fetchFn not provided", () => { globalThis.fetch = jest.fn(); expect(globalThis.fetch).not.toHaveBeenCalled(); - const api = new FetchHttpApi(new TypedEventEmitter(), { baseUrl, prefix }); + const api = new FetchHttpApi(new TypedEventEmitter(), { baseUrl, prefix, onlyData: true }); api.fetch("test"); expect(globalThis.fetch).toHaveBeenCalled(); }); it("should update identity server base url", () => { - const api = new FetchHttpApi(new TypedEventEmitter(), { baseUrl, prefix }); + const api = new FetchHttpApi(new TypedEventEmitter(), { baseUrl, prefix, onlyData: true }); expect(api.opts.idBaseUrl).toBeUndefined(); api.setIdBaseUrl("https://id.foo.bar"); expect(api.opts.idBaseUrl).toBe("https://id.foo.bar"); @@ -82,23 +81,35 @@ describe("FetchHttpApi", () => { describe("idServerRequest", () => { it("should throw if no idBaseUrl", () => { - const api = new FetchHttpApi(new TypedEventEmitter(), { baseUrl, prefix }); + const api = new FetchHttpApi(new TypedEventEmitter(), { baseUrl, prefix, onlyData: true }); expect(() => api.idServerRequest(Method.Get, "/test", {}, IdentityPrefix.V2)).toThrow( "No identity server base URL set", ); }); it("should send params as query string for GET requests", () => { - const fetchFn = jest.fn().mockResolvedValue({ ok: true }); - const api = new FetchHttpApi(new TypedEventEmitter(), { baseUrl, idBaseUrl, prefix, fetchFn }); + const fetchFn = makeMockFetchFn(); + const api = new FetchHttpApi(new TypedEventEmitter(), { + baseUrl, + idBaseUrl, + prefix, + fetchFn, + onlyData: true, + }); api.idServerRequest(Method.Get, "/test", { foo: "bar", via: ["a", "b"] }, IdentityPrefix.V2); expect(fetchFn.mock.calls[0][0].searchParams.get("foo")).toBe("bar"); expect(fetchFn.mock.calls[0][0].searchParams.getAll("via")).toEqual(["a", "b"]); }); it("should send params as body for non-GET requests", () => { - const fetchFn = jest.fn().mockResolvedValue({ ok: true }); - const api = new FetchHttpApi(new TypedEventEmitter(), { baseUrl, idBaseUrl, prefix, fetchFn }); + const fetchFn = makeMockFetchFn(); + const api = new FetchHttpApi(new TypedEventEmitter(), { + baseUrl, + idBaseUrl, + prefix, + fetchFn, + onlyData: true, + }); const params = { foo: "bar", via: ["a", "b"] }; api.idServerRequest(Method.Post, "/test", params, IdentityPrefix.V2); expect(fetchFn.mock.calls[0][0].searchParams.get("foo")).not.toBe("bar"); @@ -106,18 +117,27 @@ describe("FetchHttpApi", () => { }); it("should add Authorization header if token provided", () => { - const fetchFn = jest.fn().mockResolvedValue({ ok: true }); - const api = new FetchHttpApi(new TypedEventEmitter(), { baseUrl, idBaseUrl, prefix, fetchFn }); + const fetchFn = makeMockFetchFn(); + const api = new FetchHttpApi(new TypedEventEmitter(), { + baseUrl, + idBaseUrl, + prefix, + fetchFn, + onlyData: true, + }); api.idServerRequest(Method.Post, "/test", {}, IdentityPrefix.V2, "token"); expect(fetchFn.mock.calls[0][1].headers.Authorization).toBe("Bearer token"); }); }); - it("should return the Response object if onlyData=false", async () => { - const res = { ok: true }; - const fetchFn = jest.fn().mockResolvedValue(res); - const api = new FetchHttpApi(new TypedEventEmitter(), { baseUrl, prefix, fetchFn, onlyData: false }); - await expect(api.requestOtherUrl(Method.Get, "http://url")).resolves.toBe(res); + it("should complain if constructed without `onlyData: true`", async () => { + expect( + () => + new FetchHttpApi(new TypedEventEmitter(), { + baseUrl, + prefix, + }), + ).toThrow("Constructing FetchHttpApi without `onlyData=true` is no longer supported."); }); it("should set an Accept header, and parse the response as JSON, by default", async () => { @@ -165,37 +185,40 @@ describe("FetchHttpApi", () => { }); it("should send token via query params if useAuthorizationHeader=false", async () => { - const fetchFn = jest.fn().mockResolvedValue({ ok: true }); + const fetchFn = makeMockFetchFn(); const api = new FetchHttpApi(new TypedEventEmitter(), { baseUrl, prefix, fetchFn, accessToken: "token", useAuthorizationHeader: false, + onlyData: true, }); await api.authedRequest(Method.Get, "/path"); expect(fetchFn.mock.calls[0][0].searchParams.get("access_token")).toBe("token"); }); it("should send token via headers by default", async () => { - const fetchFn = jest.fn().mockResolvedValue({ ok: true }); + const fetchFn = makeMockFetchFn(); const api = new FetchHttpApi(new TypedEventEmitter(), { baseUrl, prefix, fetchFn, accessToken: "token", + onlyData: true, }); await api.authedRequest(Method.Get, "/path"); expect(fetchFn.mock.calls[0][1].headers["Authorization"]).toBe("Bearer token"); }); it("should not send a token if not calling `authedRequest`", () => { - const fetchFn = jest.fn().mockResolvedValue({ ok: true }); + const fetchFn = makeMockFetchFn(); const api = new FetchHttpApi(new TypedEventEmitter(), { baseUrl, prefix, fetchFn, accessToken: "token", + onlyData: true, }); api.request(Method.Get, "/path"); expect(fetchFn.mock.calls[0][0].searchParams.get("access_token")).toBeFalsy(); @@ -203,13 +226,14 @@ describe("FetchHttpApi", () => { }); it("should ensure no token is leaked out via query params if sending via headers", async () => { - const fetchFn = jest.fn().mockResolvedValue({ ok: true }); + const fetchFn = makeMockFetchFn(); const api = new FetchHttpApi(new TypedEventEmitter(), { baseUrl, prefix, fetchFn, accessToken: "token", useAuthorizationHeader: true, + onlyData: true, }); await api.authedRequest(Method.Get, "/path", { access_token: "123" }); expect(fetchFn.mock.calls[0][0].searchParams.get("access_token")).toBeFalsy(); @@ -217,26 +241,28 @@ describe("FetchHttpApi", () => { }); it("should not override manually specified access token via query params", async () => { - const fetchFn = jest.fn().mockResolvedValue({ ok: true }); + const fetchFn = makeMockFetchFn(); const api = new FetchHttpApi(new TypedEventEmitter(), { baseUrl, prefix, fetchFn, accessToken: "token", useAuthorizationHeader: false, + onlyData: true, }); await api.authedRequest(Method.Get, "/path", { access_token: "RealToken" }); expect(fetchFn.mock.calls[0][0].searchParams.get("access_token")).toBe("RealToken"); }); it("should not override manually specified access token via header", async () => { - const fetchFn = jest.fn().mockResolvedValue({ ok: true }); + const fetchFn = makeMockFetchFn(); const api = new FetchHttpApi(new TypedEventEmitter(), { baseUrl, prefix, fetchFn, accessToken: "token", useAuthorizationHeader: true, + onlyData: true, }); await api.authedRequest(Method.Get, "/path", undefined, undefined, { headers: { Authorization: "Bearer RealToken" }, @@ -245,8 +271,8 @@ describe("FetchHttpApi", () => { }); it("should not override Accept header", async () => { - const fetchFn = jest.fn().mockResolvedValue({ ok: true }); - const api = new FetchHttpApi(new TypedEventEmitter(), { baseUrl, prefix, fetchFn }); + const fetchFn = makeMockFetchFn(); + const api = new FetchHttpApi(new TypedEventEmitter(), { baseUrl, prefix, fetchFn, onlyData: true }); await api.authedRequest(Method.Get, "/path", undefined, undefined, { headers: { Accept: "text/html" }, }); @@ -269,7 +295,7 @@ describe("FetchHttpApi", () => { ), }); const emitter = new TypedEventEmitter(); - const api = new FetchHttpApi(emitter, { baseUrl, prefix, fetchFn }); + const api = new FetchHttpApi(emitter, { baseUrl, prefix, fetchFn, onlyData: true }); await Promise.all([ emitPromise(emitter, HttpApiEvent.NoConsent), @@ -279,9 +305,9 @@ describe("FetchHttpApi", () => { describe("authedRequest", () => { it("should not include token if unset", async () => { - const fetchFn = jest.fn().mockResolvedValue({ ok: true }); + const fetchFn = makeMockFetchFn(); const emitter = new TypedEventEmitter(); - const api = new FetchHttpApi(emitter, { baseUrl, prefix, fetchFn }); + const api = new FetchHttpApi(emitter, { baseUrl, prefix, fetchFn, onlyData: true }); await api.authedRequest(Method.Post, "/account/password"); expect(fetchFn.mock.calls[0][1].headers.Authorization).toBeUndefined(); }); @@ -310,6 +336,7 @@ describe("FetchHttpApi", () => { const okayResponse = { ok: true, status: 200, + json: jest.fn().mockResolvedValue({ x: 1 }), }; describe("without a tokenRefreshFunction", () => { @@ -317,7 +344,14 @@ describe("FetchHttpApi", () => { const fetchFn = jest.fn().mockResolvedValue(unknownTokenResponse); const emitter = new TypedEventEmitter(); jest.spyOn(emitter, "emit"); - const api = new FetchHttpApi(emitter, { baseUrl, prefix, fetchFn, accessToken, refreshToken }); + const api = new FetchHttpApi(emitter, { + baseUrl, + prefix, + fetchFn, + accessToken, + refreshToken, + onlyData: true, + }); await expect(api.authedRequest(Method.Post, "/account/password")).rejects.toThrow( unknownTokenErr, ); @@ -339,6 +373,7 @@ describe("FetchHttpApi", () => { tokenRefreshFunction, accessToken, refreshToken, + onlyData: true, }); await expect(api.authedRequest(Method.Post, "/account/password")).rejects.toThrow( unknownTokenErr, @@ -360,6 +395,7 @@ describe("FetchHttpApi", () => { tokenRefreshFunction, accessToken, refreshToken, + onlyData: true, }); await expect(api.authedRequest(Method.Post, "/account/password")).rejects.toThrow( unknownTokenErr, @@ -388,11 +424,12 @@ describe("FetchHttpApi", () => { tokenRefreshFunction, accessToken, refreshToken, + onlyData: true, }); const result = await api.authedRequest(Method.Post, "/account/password", undefined, undefined, { headers: {}, }); - expect(result).toEqual(okayResponse); + expect(result).toEqual({ x: 1 }); expect(tokenRefreshFunction).toHaveBeenCalledWith(refreshToken); expect(fetchFn).toHaveBeenCalledTimes(2); @@ -431,6 +468,7 @@ describe("FetchHttpApi", () => { tokenRefreshFunction, accessToken, refreshToken, + onlyData: true, }); await expect(api.authedRequest(Method.Post, "/account/password")).rejects.toThrow( unknownTokenErr, @@ -486,6 +524,7 @@ describe("FetchHttpApi", () => { tokenRefreshFunction, accessToken, refreshToken, + onlyData: true, }); await expect(api.authedRequest(Method.Post, "/account/password")).rejects.toThrow( unknownTokenErr, @@ -506,7 +545,7 @@ describe("FetchHttpApi", () => { const makeApi = (thisBaseUrl = baseUrl): FetchHttpApi => { const fetchFn = jest.fn(); const emitter = new TypedEventEmitter(); - return new FetchHttpApi(emitter, { baseUrl: thisBaseUrl, prefix, fetchFn }); + return new FetchHttpApi(emitter, { baseUrl: thisBaseUrl, prefix, fetchFn, onlyData: true }); }; type TestParams = { @@ -559,7 +598,13 @@ describe("FetchHttpApi", () => { const makeApiWithExtraParams = (extraParams: QueryDict): FetchHttpApi => { const fetchFn = jest.fn(); const emitter = new TypedEventEmitter(); - return new FetchHttpApi(emitter, { baseUrl: localBaseUrl, prefix, fetchFn, extraParams }); + return new FetchHttpApi(emitter, { + baseUrl: localBaseUrl, + prefix, + fetchFn, + onlyData: true, + extraParams, + }); }; const userId = "@rsb-tbg:localhost"; @@ -612,7 +657,7 @@ describe("FetchHttpApi", () => { it("should work when extraParams is undefined", () => { const fetchFn = jest.fn(); const emitter = new TypedEventEmitter(); - const api = new FetchHttpApi(emitter, { baseUrl: localBaseUrl, prefix, fetchFn }); + const api = new FetchHttpApi(emitter, { baseUrl: localBaseUrl, prefix, fetchFn, onlyData: true }); const queryParams = { userId: "123" }; const result = api.getUrl("/test", queryParams); @@ -645,10 +690,11 @@ describe("FetchHttpApi", () => { prefix, fetchFn, logger: mockLogger, + onlyData: true, }); const prom = api.requestOtherUrl(Method.Get, "https://server:8448/some/path?query=param#fragment"); jest.advanceTimersByTime(1234); - responseResolvers.resolve({ ok: true, status: 200, text: () => Promise.resolve("RESPONSE") } as Response); + responseResolvers.resolve({ ok: true, status: 200, json: () => Promise.resolve("RESPONSE") } as Response); await prom; expect(mockLogger.debug).not.toHaveBeenCalledWith("fragment"); expect(mockLogger.debug).not.toHaveBeenCalledWith("query"); @@ -691,6 +737,7 @@ describe("FetchHttpApi", () => { tokenRefreshFunction, accessToken: "ACCESS_TOKEN", refreshToken: "REFRESH_TOKEN", + onlyData: true, }); const prom1 = api.authedRequest(Method.Get, "/path1"); @@ -746,6 +793,7 @@ describe("FetchHttpApi", () => { tokenRefreshFunction, accessToken: "ACCESS_TOKEN", refreshToken: "REFRESH_TOKEN", + onlyData: true, }); const prom1 = api.authedRequest(Method.Get, "/path1"); @@ -783,3 +831,7 @@ describe("FetchHttpApi", () => { expect(api.opts.refreshToken).toBe("NEW_REFRESH_TOKEN"); }); }); + +function makeMockFetchFn(): MockedFunction { + return jest.fn().mockResolvedValue({ ok: true, json: jest.fn().mockResolvedValue({}) }); +} diff --git a/spec/unit/http-api/index.spec.ts b/spec/unit/http-api/index.spec.ts index 9eb321891..2019a78eb 100644 --- a/spec/unit/http-api/index.spec.ts +++ b/spec/unit/http-api/index.spec.ts @@ -62,7 +62,7 @@ describe("MatrixHttpApi", () => { it("should fall back to `fetch` where xhr is unavailable", async () => { globalThis.XMLHttpRequest = undefined!; const fetchFn = jest.fn().mockResolvedValue({ ok: true, json: jest.fn().mockResolvedValue({}) }); - const api = new MatrixHttpApi(new TypedEventEmitter(), { baseUrl, prefix, fetchFn }); + const api = new MatrixHttpApi(new TypedEventEmitter(), { baseUrl, prefix, fetchFn, onlyData: true }); upload = api.uploadContent({} as File); await upload; expect(fetchFn).toHaveBeenCalled(); @@ -70,7 +70,7 @@ describe("MatrixHttpApi", () => { it("should prefer xhr where available", () => { const fetchFn = jest.fn().mockResolvedValue({ ok: true }); - const api = new MatrixHttpApi(new TypedEventEmitter(), { baseUrl, prefix, fetchFn }); + const api = new MatrixHttpApi(new TypedEventEmitter(), { baseUrl, prefix, fetchFn, onlyData: true }); upload = api.uploadContent({} as File); expect(fetchFn).not.toHaveBeenCalled(); expect(xhr.open).toHaveBeenCalled(); @@ -82,6 +82,7 @@ describe("MatrixHttpApi", () => { prefix, accessToken: "token", useAuthorizationHeader: false, + onlyData: true, }); upload = api.uploadContent({} as File); expect(xhr.open).toHaveBeenCalledWith( @@ -96,6 +97,7 @@ describe("MatrixHttpApi", () => { baseUrl, prefix, accessToken: "token", + onlyData: true, }); upload = api.uploadContent({} as File); expect(xhr.open).toHaveBeenCalledWith(Method.Post, baseUrl.toLowerCase() + "/_matrix/media/v3/upload"); @@ -103,7 +105,7 @@ describe("MatrixHttpApi", () => { }); it("should include filename by default", () => { - const api = new MatrixHttpApi(new TypedEventEmitter(), { baseUrl, prefix }); + const api = new MatrixHttpApi(new TypedEventEmitter(), { baseUrl, prefix, onlyData: true }); upload = api.uploadContent({} as File, { name: "name" }); expect(xhr.open).toHaveBeenCalledWith( Method.Post, @@ -112,13 +114,13 @@ describe("MatrixHttpApi", () => { }); it("should allow not sending the filename", () => { - const api = new MatrixHttpApi(new TypedEventEmitter(), { baseUrl, prefix }); + const api = new MatrixHttpApi(new TypedEventEmitter(), { baseUrl, prefix, onlyData: true }); upload = api.uploadContent({} as File, { name: "name", includeFilename: false }); expect(xhr.open).toHaveBeenCalledWith(Method.Post, baseUrl.toLowerCase() + "/_matrix/media/v3/upload"); }); it("should abort xhr when the upload is aborted", () => { - const api = new MatrixHttpApi(new TypedEventEmitter(), { baseUrl, prefix }); + const api = new MatrixHttpApi(new TypedEventEmitter(), { baseUrl, prefix, onlyData: true }); upload = api.uploadContent({} as File); api.cancelUpload(upload); expect(xhr.abort).toHaveBeenCalled(); @@ -126,7 +128,7 @@ describe("MatrixHttpApi", () => { }); it("should timeout if no progress in 30s", () => { - const api = new MatrixHttpApi(new TypedEventEmitter(), { baseUrl, prefix }); + const api = new MatrixHttpApi(new TypedEventEmitter(), { baseUrl, prefix, onlyData: true }); upload = api.uploadContent({} as File); jest.advanceTimersByTime(25000); // @ts-ignore @@ -138,7 +140,7 @@ describe("MatrixHttpApi", () => { }); it("should call progressHandler", () => { - const api = new MatrixHttpApi(new TypedEventEmitter(), { baseUrl, prefix }); + const api = new MatrixHttpApi(new TypedEventEmitter(), { baseUrl, prefix, onlyData: true }); const progressHandler = jest.fn(); upload = api.uploadContent({} as File, { progressHandler }); const progressEvent = new Event("progress") as ProgressEvent; @@ -154,7 +156,7 @@ describe("MatrixHttpApi", () => { }); it("should error when no response body", () => { - const api = new MatrixHttpApi(new TypedEventEmitter(), { baseUrl, prefix }); + const api = new MatrixHttpApi(new TypedEventEmitter(), { baseUrl, prefix, onlyData: true }); upload = api.uploadContent({} as File); xhr.readyState = DONE; @@ -167,7 +169,7 @@ describe("MatrixHttpApi", () => { }); it("should error on a 400-code", () => { - const api = new MatrixHttpApi(new TypedEventEmitter(), { baseUrl, prefix }); + const api = new MatrixHttpApi(new TypedEventEmitter(), { baseUrl, prefix, onlyData: true }); upload = api.uploadContent({} as File); xhr.readyState = DONE; @@ -184,7 +186,7 @@ describe("MatrixHttpApi", () => { }); it("should return response on successful upload", () => { - const api = new MatrixHttpApi(new TypedEventEmitter(), { baseUrl, prefix }); + const api = new MatrixHttpApi(new TypedEventEmitter(), { baseUrl, prefix, onlyData: true }); upload = api.uploadContent({} as File); xhr.readyState = DONE; @@ -198,14 +200,14 @@ describe("MatrixHttpApi", () => { }); it("should abort xhr when calling `cancelUpload`", () => { - const api = new MatrixHttpApi(new TypedEventEmitter(), { baseUrl, prefix }); + const api = new MatrixHttpApi(new TypedEventEmitter(), { baseUrl, prefix, onlyData: true }); upload = api.uploadContent({} as File); expect(api.cancelUpload(upload)).toBeTruthy(); expect(xhr.abort).toHaveBeenCalled(); }); it("should return false when `cancelUpload` is called but unsuccessful", async () => { - const api = new MatrixHttpApi(new TypedEventEmitter(), { baseUrl, prefix }); + const api = new MatrixHttpApi(new TypedEventEmitter(), { baseUrl, prefix, onlyData: true }); upload = api.uploadContent({} as File); xhr.readyState = DONE; @@ -220,7 +222,7 @@ describe("MatrixHttpApi", () => { }); it("should return active uploads in `getCurrentUploads`", () => { - const api = new MatrixHttpApi(new TypedEventEmitter(), { baseUrl, prefix }); + const api = new MatrixHttpApi(new TypedEventEmitter(), { baseUrl, prefix, onlyData: true }); upload = api.uploadContent({} as File); expect(api.getCurrentUploads().find((u) => u.promise === upload)).toBeTruthy(); api.cancelUpload(upload); @@ -228,7 +230,12 @@ describe("MatrixHttpApi", () => { }); it("should return expected object from `getContentUri`", () => { - const api = new MatrixHttpApi(new TypedEventEmitter(), { baseUrl, prefix, accessToken: "token" }); + const api = new MatrixHttpApi(new TypedEventEmitter(), { + baseUrl, + prefix, + accessToken: "token", + onlyData: true, + }); expect(api.getContentUri()).toMatchSnapshot(); }); }); diff --git a/src/client.ts b/src/client.ts index d2de367e2..8ffb37f61 100644 --- a/src/client.ts +++ b/src/client.ts @@ -6843,9 +6843,7 @@ export class MatrixClient extends TypedEventEmitter { return this.http.uploadContent(file, opts); diff --git a/src/http-api/fetch.ts b/src/http-api/fetch.ts index 026f4b042..d6d1b5a52 100644 --- a/src/http-api/fetch.ts +++ b/src/http-api/fetch.ts @@ -34,22 +34,6 @@ import { anySignal, parseErrorResponse, timeoutSignal } from "./utils.ts"; import { type QueryDict } from "../utils.ts"; import { TokenRefresher, TokenRefreshOutcome } from "./refresh.ts"; -interface TypedResponse extends Response { - json(): Promise; -} - -/** - * The type returned by {@link FetchHttpApi.request}, etc. - * - * If {@link IHttpOpts.onlyData} is unset or false, then the request methods return a - * {@link https://developer.mozilla.org/en-US/docs/Web/API/Response Response} object, - * which we abstract via `TypedResponse`. Otherwise, we just cast it to `T`. - * - * @typeParam T - The type (specified by the application on the request method) that we will cast the response to. - * @typeParam O - The type of the options object on the {@link FetchHttpApi} instance. - */ -export type ResponseType = O extends { onlyData: true } | undefined ? T : TypedResponse; - export class FetchHttpApi { private abortController = new AbortController(); private readonly tokenRefresher: TokenRefresher; @@ -59,7 +43,9 @@ export class FetchHttpApi { public readonly opts: O, ) { checkObjectHasKeys(opts, ["baseUrl", "prefix"]); - opts.onlyData = !!opts.onlyData; + if (!opts.onlyData) { + throw new Error("Constructing FetchHttpApi without `onlyData=true` is no longer supported."); + } opts.useAuthorizationHeader = opts.useAuthorizationHeader ?? true; this.tokenRefresher = new TokenRefresher(opts); @@ -91,7 +77,7 @@ export class FetchHttpApi { params: Record | undefined, prefix: string, accessToken?: string, - ): Promise> { + ): Promise { if (!this.opts.idBaseUrl) { throw new Error("No identity server base URL set"); } @@ -132,17 +118,8 @@ export class FetchHttpApi { * When `paramOpts.doNotAttemptTokenRefresh` is true, token refresh will not be attempted * when an expired token is encountered. Used to only attempt token refresh once. * - * @returns Promise which resolves to - * ``` - * { - * data: {Object}, - * headers: {Object}, - * code: {Number}, - * } - * ``` - * If `onlyData` is set, this will resolve to the `data` object only. - * @returns Rejects with an error if a problem occurred. - * This includes network problems and Matrix-specific error JSON. + * @returns The parsed response. + * @throws Error if a problem occurred. This includes network problems and Matrix-specific error JSON. */ public authedRequest( method: Method, @@ -150,7 +127,7 @@ export class FetchHttpApi { queryParams: QueryDict = {}, body?: Body, paramOpts: IRequestOpts = {}, - ): Promise> { + ): Promise { return this.doAuthedRequest(1, method, path, queryParams, body, paramOpts); } @@ -162,7 +139,7 @@ export class FetchHttpApi { queryParams: QueryDict, body?: Body, paramOpts: IRequestOpts = {}, - ): Promise> { + ): Promise { // avoid mutating paramOpts so they can be used on retry const opts = deepCopy(paramOpts); // we have to manually copy the abortSignal over as it is not a plain object @@ -228,18 +205,8 @@ export class FetchHttpApi { * * @param opts - additional options * - * @returns Promise which resolves to - * ``` - * { - * data: {Object}, - * headers: {Object}, - * code: {Number}, - * } - * ``` - * If `onlyData is set, this will resolve to the data` - * object only. - * @returns Rejects with an error if a problem - * occurred. This includes network problems and Matrix-specific error JSON. + * @returns The parsed response. + * @throws Error if a problem occurred. This includes network problems and Matrix-specific error JSON. */ public request( method: Method, @@ -247,7 +214,7 @@ export class FetchHttpApi { queryParams?: QueryDict, body?: Body, opts?: IRequestOpts, - ): Promise> { + ): Promise { const fullUri = this.getUrl(path, queryParams, opts?.prefix, opts?.baseUrl); return this.requestOtherUrl(method, fullUri, body, opts); } @@ -261,17 +228,15 @@ export class FetchHttpApi { * * @param opts - additional options * - * @returns Promise which resolves to data unless `onlyData` is specified as false, - * where the resolved value will be a fetch Response object. - * @returns Rejects with an error if a problem - * occurred. This includes network problems and Matrix-specific error JSON. + * @returns The parsed response. + * @throws Error if a problem occurred. This includes network problems and Matrix-specific error JSON. */ public async requestOtherUrl( method: Method, url: URL | string, body?: Body, opts: BaseRequestOpts = {}, - ): Promise> { + ): Promise { if (opts.json !== undefined && opts.rawResponseBody !== undefined) { throw new Error("Invalid call to `FetchHttpApi` sets both `opts.json` and `opts.rawResponseBody`"); } @@ -349,14 +314,12 @@ export class FetchHttpApi { throw parseErrorResponse(res, await res.text()); } - if (!this.opts.onlyData) { - return res as ResponseType; - } else if (opts.rawResponseBody) { - return (await res.blob()) as ResponseType; + if (opts.rawResponseBody) { + return (await res.blob()) as T; } else if (jsonResponse) { return await res.json(); } else { - return (await res.text()) as ResponseType; + return (await res.text()) as T; } } diff --git a/src/http-api/index.ts b/src/http-api/index.ts index 1474cdde9..346a9e549 100644 --- a/src/http-api/index.ts +++ b/src/http-api/index.ts @@ -48,9 +48,7 @@ export class MatrixHttpApi extends FetchHttpApi { * * @param opts - options object * - * @returns Promise which resolves to response object, as - * determined by this.opts.onlyData, opts.rawResponse, and - * opts.onlyContentUri. Rejects with an error (usually a MatrixError). + * @returns Promise which resolves to response object, or rejects with an error (usually a MatrixError). */ public uploadContent(file: FileType, opts: UploadOpts = {}): Promise { const includeFilename = opts.includeFilename ?? true; @@ -149,11 +147,7 @@ export class MatrixHttpApi extends FetchHttpApi { prefix: MediaPrefix.V3, headers, abortSignal: abortController.signal, - }) - .then((response) => { - return this.opts.onlyData ? response : response.json(); - }) - .then(uploadResolvers.resolve, uploadResolvers.reject); + }).then(uploadResolvers.resolve, uploadResolvers.reject); } // remove the upload from the list on completion diff --git a/src/http-api/interface.ts b/src/http-api/interface.ts index 5d56dd7a7..40100a5b5 100644 --- a/src/http-api/interface.ts +++ b/src/http-api/interface.ts @@ -69,10 +69,7 @@ export interface IHttpOpts { tokenRefreshFunction?: TokenRefreshFunction; useAuthorizationHeader?: boolean; // defaults to true - /** - * Normally, methods in `FetchHttpApi` will return a {@link https://developer.mozilla.org/en-US/docs/Web/API/Response Response} object. - * If this is set to `true`, they instead return the response body. - */ + /** For historical reasons, must be set to `true`. Will eventually be removed. */ onlyData?: boolean; localTimeoutMs?: number; @@ -103,11 +100,10 @@ export interface BaseRequestOpts extends Pick { * * * Set `Accept: application/json` in the request headers (again, unless overridden by {@link headers}). * - * * If `IHTTPOpts.onlyData` is set to `true` on the `FetchHttpApi` instance, parse the response as - * JSON and return the parsed response. + * * Parse the response as JSON and return the parsed response. * - * Setting this to `false` inhibits all three behaviors, and (if `IHTTPOpts.onlyData` is set to `true`) the response - * is instead parsed as a UTF-8 string. It defaults to `true`, unless {@link rawResponseBody} is set. + * Setting this to `false` inhibits all three behaviors, and the response is instead parsed as a UTF-8 string. It + * defaults to `true`, unless {@link rawResponseBody} is set. * * @deprecated Instead of setting this to `false`, set {@link rawResponseBody} to `true`. */ @@ -118,9 +114,8 @@ export interface BaseRequestOpts extends Pick { * * * Inhibits the automatic addition of `Accept: application/json` in the request headers. * - * * Assuming `IHTTPOpts.onlyData` is set to `true` on the `FetchHttpApi` instance, causes the - * raw response to be returned as a {@link https://developer.mozilla.org/en-US/docs/Web/API/Blob|Blob} - * instead of parsing it as `json`. + * * Causes the raw response to be returned as a {@link https://developer.mozilla.org/en-US/docs/Web/API/Blob|Blob} + * instead of parsing it as JSON. */ rawResponseBody?: boolean; }