From c7dbd6e33bf4b90d547934ee49cba86f223b7212 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Thu, 24 Jul 2025 12:06:52 +0100 Subject: [PATCH] fetch api: add support for downloading raw response (#4917) * Factor out `BaseRequestOpts` ... to make it easier to find the docs from methods that use it. * fetch api: add support for downloading raw response I need to make an authenticated request to the media repo, and expect to get a binary file back. AFAICT there is no easy way to do that right now. * Clarify doc strings * Various fixes --- spec/unit/http-api/fetch.spec.ts | 35 +++++++++++++++++- src/http-api/fetch.ts | 37 ++++++++++++------- src/http-api/interface.ts | 63 ++++++++++++++++++++++++++------ 3 files changed, 109 insertions(+), 26 deletions(-) diff --git a/spec/unit/http-api/fetch.spec.ts b/spec/unit/http-api/fetch.spec.ts index d7e0e19a9..0e3bed566 100644 --- a/spec/unit/http-api/fetch.spec.ts +++ b/spec/unit/http-api/fetch.spec.ts @@ -120,7 +120,15 @@ describe("FetchHttpApi", () => { await expect(api.requestOtherUrl(Method.Get, "http://url")).resolves.toBe(res); }); - it("should return text if json=false", async () => { + it("should set an Accept header, and parse the response as JSON, by default", async () => { + const result = { a: 1 }; + const fetchFn = jest.fn().mockResolvedValue({ ok: true, json: jest.fn().mockResolvedValue(result) }); + const api = new FetchHttpApi(new TypedEventEmitter(), { baseUrl, prefix, fetchFn, onlyData: true }); + await expect(api.requestOtherUrl(Method.Get, "http://url")).resolves.toBe(result); + expect(fetchFn.mock.calls[0][1].headers.Accept).toBe("application/json"); + }); + + it("should not set an Accept header, and should return text if json=false", async () => { const text = "418 I'm a teapot"; const fetchFn = jest.fn().mockResolvedValue({ ok: true, text: jest.fn().mockResolvedValue(text) }); const api = new FetchHttpApi(new TypedEventEmitter(), { baseUrl, prefix, fetchFn, onlyData: true }); @@ -129,6 +137,31 @@ describe("FetchHttpApi", () => { json: false, }), ).resolves.toBe(text); + expect(fetchFn.mock.calls[0][1].headers.Accept).not.toBeDefined(); + }); + + it("should not set an Accept header, and should return a blob, if rawResponseBody is true", async () => { + const blob = new Blob(["blobby"]); + const fetchFn = jest.fn().mockResolvedValue({ ok: true, blob: jest.fn().mockResolvedValue(blob) }); + const api = new FetchHttpApi(new TypedEventEmitter(), { baseUrl, prefix, fetchFn, onlyData: true }); + await expect( + api.requestOtherUrl(Method.Get, "http://url", undefined, { + rawResponseBody: true, + }), + ).resolves.toBe(blob); + expect(fetchFn.mock.calls[0][1].headers.Accept).not.toBeDefined(); + }); + + it("should throw an error if both `json` and `rawResponseBody` are defined", async () => { + const api = new FetchHttpApi(new TypedEventEmitter(), { + baseUrl, + prefix, + fetchFn: jest.fn(), + onlyData: true, + }); + await expect( + api.requestOtherUrl(Method.Get, "http://url", undefined, { rawResponseBody: false, json: true }), + ).rejects.toThrow("Invalid call to `FetchHttpApi`"); }); it("should send token via query params if useAuthorizationHeader=false", async () => { diff --git a/src/http-api/fetch.ts b/src/http-api/fetch.ts index b909c3b03..026f4b042 100644 --- a/src/http-api/fetch.ts +++ b/src/http-api/fetch.ts @@ -23,6 +23,7 @@ import { type TypedEventEmitter } from "../models/typed-event-emitter.ts"; import { Method } from "./method.ts"; import { ConnectionError, MatrixError, TokenRefreshError } from "./errors.ts"; import { + type BaseRequestOpts, HttpApiEvent, type HttpApiEventHandlerMap, type IHttpOpts, @@ -269,21 +270,20 @@ export class FetchHttpApi { method: Method, url: URL | string, body?: Body, - opts: Pick = {}, + opts: BaseRequestOpts = {}, ): Promise> { + if (opts.json !== undefined && opts.rawResponseBody !== undefined) { + throw new Error("Invalid call to `FetchHttpApi` sets both `opts.json` and `opts.rawResponseBody`"); + } + const urlForLogs = this.sanitizeUrlForLogs(url); + this.opts.logger?.debug(`FetchHttpApi: --> ${method} ${urlForLogs}`); const headers = Object.assign({}, opts.headers || {}); - const json = opts.json ?? true; - // We can't use getPrototypeOf here as objects made in other contexts e.g. over postMessage won't have same ref - const jsonBody = json && body?.constructor?.name === Object.name; - - if (json) { - if (jsonBody && !headers["Content-Type"]) { - headers["Content-Type"] = "application/json"; - } + const jsonResponse = !opts.rawResponseBody && opts.json !== false; + if (jsonResponse) { if (!headers["Accept"]) { headers["Accept"] = "application/json"; } @@ -299,9 +299,15 @@ export class FetchHttpApi { signals.push(opts.abortSignal); } + // If the body is an object, encode it as JSON and set the `Content-Type` header, + // unless that has been explicitly inhibited by setting `opts.json: false`. + // We can't use getPrototypeOf here as objects made in other contexts e.g. over postMessage won't have same ref let data: BodyInit; - if (jsonBody) { + if (opts.json !== false && body?.constructor?.name === Object.name) { data = JSON.stringify(body); + if (!headers["Content-Type"]) { + headers["Content-Type"] = "application/json"; + } } else { data = body as BodyInit; } @@ -343,10 +349,15 @@ export class FetchHttpApi { throw parseErrorResponse(res, await res.text()); } - if (this.opts.onlyData) { - return (json ? res.json() : res.text()) as ResponseType; + if (!this.opts.onlyData) { + return res as ResponseType; + } else if (opts.rawResponseBody) { + return (await res.blob()) as ResponseType; + } else if (jsonResponse) { + return await res.json(); + } else { + return (await res.text()) as ResponseType; } - return res as ResponseType; } private sanitizeUrlForLogs(url: URL | string): string { diff --git a/src/http-api/interface.ts b/src/http-api/interface.ts index 1b0df88b5..5d56dd7a7 100644 --- a/src/http-api/interface.ts +++ b/src/http-api/interface.ts @@ -47,6 +47,8 @@ export type AccessTokens = { * Can be passed to HttpApi instance as {@link IHttpOpts.tokenRefreshFunction} during client creation {@link ICreateClientOpts} */ export type TokenRefreshFunction = (refreshToken: string) => Promise; + +/** Options object for `FetchHttpApi` and {@link MatrixHttpApi}. */ export interface IHttpOpts { fetchFn?: typeof globalThis.fetch; @@ -67,24 +69,20 @@ 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. + */ onlyData?: boolean; + localTimeoutMs?: number; /** Optional logger instance. If provided, requests and responses will be logged. */ logger?: Logger; } -export interface IRequestOpts extends Pick { - /** - * The alternative base url to use. - * If not specified, uses this.opts.baseUrl - */ - baseUrl?: string; - /** - * The full prefix to use e.g. - * "/_matrix/client/v2_alpha". If not specified, uses this.opts.prefix. - */ - prefix?: string; +/** Options object for `FetchHttpApi.requestOtherUrl`. */ +export interface BaseRequestOpts extends Pick { /** * map of additional request headers */ @@ -96,7 +94,48 @@ export interface IRequestOpts extends Pick { */ localTimeoutMs?: number; keepAlive?: boolean; // defaults to false - json?: boolean; // defaults to true + + /** + * By default, we will: + * + * * If the `body` is an object, JSON-encode it and set `Content-Type: application/json` in the + * request headers (unless overridden by {@link headers}). + * + * * 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. + * + * 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. + * + * @deprecated Instead of setting this to `false`, set {@link rawResponseBody} to `true`. + */ + json?: boolean; + + /** + * Setting this to `true` does two things: + * + * * 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`. + */ + rawResponseBody?: boolean; +} + +export interface IRequestOpts extends BaseRequestOpts { + /** + * The alternative base url to use. + * If not specified, uses this.opts.baseUrl + */ + baseUrl?: string; + /** + * The full prefix to use e.g. + * "/_matrix/client/v2_alpha". If not specified, uses this.opts.prefix. + */ + prefix?: string; // Set to true to prevent the request function from emitting a Session.logged_out event. // This is intended for use on endpoints where M_UNKNOWN_TOKEN is a valid/notable error response,