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

@@ -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<any, any>(), { 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<any, any>(), { 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<any, any>(), { 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<any, any>(), {
+            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 () => {

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<O extends IHttpOpts> {
         method: Method,
         url: URL | string,
         body?: Body,
-        opts: Pick<IRequestOpts, "headers" | "json" | "localTimeoutMs" | "keepAlive" | "abortSignal" | "priority"> = {},
+        opts: BaseRequestOpts = {},
     ): Promise<ResponseType<T, O>> {
+        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<O extends IHttpOpts> {
             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<O extends IHttpOpts> {
             throw parseErrorResponse(res, await res.text());
         }
 
-        if (this.opts.onlyData) {
+        if (!this.opts.onlyData) {
-            return (json ? res.json() : res.text()) as ResponseType<T, O>;
-        }
             return res as ResponseType<T, O>;
+        } else if (opts.rawResponseBody) {
+            return (await res.blob()) as ResponseType<T, O>;
+        } else if (jsonResponse) {
+            return await res.json();
+        } else {
+            return (await res.text()) as ResponseType<T, O>;
+        }
     }
 
     private sanitizeUrlForLogs(url: URL | string): string {

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<AccessTokens>;
+
+/** 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<RequestInit, "priority"> {
+/** Options object for `FetchHttpApi.requestOtherUrl`. */
-    /**
+export interface BaseRequestOpts extends Pick<RequestInit, "priority"> {
-     * 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;
     /**
      * map of additional request headers
      */
@@ -96,7 +94,48 @@ export interface IRequestOpts extends Pick<RequestInit, "priority"> {
      */
     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,