Add support for reading binary responses

.eslintrc.base.js

@@ -111,6 +111,10 @@ module.exports = (projectRoot) => ({
     eqeqeq: ["error"],
     "object-shorthand": ["error", "always"],
     "@typescript-eslint/unbound-method": ["error"],
+    "@typescript-eslint/no-implicit-any-catch": [
+      "error",
+      { allowExplicitAny: true },
+    ],
     "typescript-enum/no-const-enum": ["error"], // not supported in SWC
 
     "@typescript-eslint/naming-convention": [
@@ -145,7 +149,7 @@ module.exports = (projectRoot) => ({
         // ATTENTION: the rules here are not simple, mainly because of this:
         // https://github.com/typescript-eslint/typescript-eslint/issues/6133
         //
-        // Besides that, we also wand contradictory things, like:
+        // Besides that, we also want contradictory things, like:
         //
         // 1. Having constructor close to fields definition (because people
         //    often define fields in the constructor arguments), although it

docs/classes/RestStream.md

@@ -6,6 +6,11 @@ Once created, RestStream must be iterated in full, otherwise the connection
 will remain dangling. Also, this class is where we hide the details of the
 actual stream reading using AsyncGenerator bridge abstraction.
 
+RestStream can also read binary data depending on the Content-Type response
+header and/or the charset provided there. The binary data is still returned
+as a string, one string character per each byte. To convert it to a Buffer,
+use something like `Buffer.from(responseText, "binary")`.
+
 ## Constructors
 
 ### constructor
@@ -22,7 +27,7 @@ actual stream reading using AsyncGenerator bridge abstraction.
 
 #### Defined in
 
-[src/RestStream.ts:12](https://github.com/clickup/rest-client/blob/master/src/RestStream.ts#L12)
+[src/RestStream.ts:17](https://github.com/clickup/rest-client/blob/master/src/RestStream.ts#L17)
 
 ## Properties
 
@@ -32,7 +37,7 @@ actual stream reading using AsyncGenerator bridge abstraction.
 
 #### Defined in
 
-[src/RestStream.ts:13](https://github.com/clickup/rest-client/blob/master/src/RestStream.ts#L13)
+[src/RestStream.ts:18](https://github.com/clickup/rest-client/blob/master/src/RestStream.ts#L18)
 
 ## Methods
 
@@ -55,7 +60,7 @@ done in all cases, so safe to be used to e.g. receive a trimmed response.
 
 #### Defined in
 
-[src/RestStream.ts:25](https://github.com/clickup/rest-client/blob/master/src/RestStream.ts#L25)
+[src/RestStream.ts:30](https://github.com/clickup/rest-client/blob/master/src/RestStream.ts#L30)
 
 ___
 
@@ -71,7 +76,7 @@ Closes the connection.
 
 #### Defined in
 
-[src/RestStream.ts:44](https://github.com/clickup/rest-client/blob/master/src/RestStream.ts#L44)
+[src/RestStream.ts:49](https://github.com/clickup/rest-client/blob/master/src/RestStream.ts#L49)
 
 ___
 
@@ -89,4 +94,4 @@ remain open.
 
 #### Defined in
 
-[src/RestStream.ts:66](https://github.com/clickup/rest-client/blob/master/src/RestStream.ts#L66)
+[src/RestStream.ts:71](https://github.com/clickup/rest-client/blob/master/src/RestStream.ts#L71)

package.json

@@ -1,7 +1,7 @@
 {
   "name": "@clickup/rest-client",
   "description": "A syntax sugar tool around Node fetch() API, tailored to work with TypeScript and response validators",
-  "version": "2.10.292",
+  "version": "2.10.293",
   "license": "MIT",
   "main": "dist/index.js",
   "types": "dist/index.d.ts",

src/RestRequest.ts

@@ -178,7 +178,7 @@ export default class RestRequest<TAssertShape = any> {
               });
 
               return res;
-            } catch (e) {
+            } catch (e: unknown) {
               await reader?.close();
               throw e;
             }
@@ -188,7 +188,7 @@ export default class RestRequest<TAssertShape = any> {
         // The only place where we return the response. Otherwise we retry or
         // throw an exception.
         return new RestStream(res, reader!);
-      } catch (error) {
+      } catch (error: unknown) {
         this._logResponse({
           attempt,
           req: this,

src/RestStream.ts

@@ -5,6 +5,11 @@ import type RestResponse from "./RestResponse";
  * Once created, RestStream must be iterated in full, otherwise the connection
  * will remain dangling. Also, this class is where we hide the details of the
  * actual stream reading using AsyncGenerator bridge abstraction.
+ *
+ * RestStream can also read binary data depending on the Content-Type response
+ * header and/or the charset provided there. The binary data is still returned
+ * as a string, one string character per each byte. To convert it to a Buffer,
+ * use something like `Buffer.from(responseText, "binary")`.
  */
 export default class RestStream {
   private _generator?: AsyncGenerator<string, void>;
@@ -41,7 +46,7 @@ export default class RestStream {
   /**
    * Closes the connection.
    */
-  async close() {
+  async close(): Promise<void> {
     // First, try to interrupt the active iteration, if any.
     await this[Symbol.asyncIterator]().return();
     // It is possible that this.[Symbol.asyncIterator] has never been iterated
@@ -63,7 +68,7 @@ export default class RestStream {
    * remain open.
    */
   @Memoize()
-  async *[Symbol.asyncIterator]() {
+  async *[Symbol.asyncIterator](): AsyncGenerator<string, void> {
     const generator = this._generator;
     if (!generator) {
       return;

src/__tests__/RestFetchReader.test.ts

@@ -1,5 +1,7 @@
 import delay from "delay";
+import range from "lodash/range";
 import {
+  BINARY_BUF,
   consumeIterable,
   createFetchReader,
   server,
@@ -134,3 +136,15 @@ test("timeout_after_reader_waited_for_too_long", async () => {
   );
   await serverAssertConnectionsCount(0);
 });
+
+test("read_binary_data", async () => {
+  const reader = createFetchReader(`${ORIGIN}/binary`, { timeoutMs: 2000 });
+  await reader.preload(42);
+  const preload = reader.textFetched;
+  expect(reader.textIsPartial).toBeTruthy();
+  const data = await consumeIterable(reader, 10);
+  expect(Buffer.from(preload + data, "binary")).toEqual(
+    Buffer.concat(range(10).map(() => BINARY_BUF))
+  );
+  await serverAssertConnectionsCount(0);
+});

src/__tests__/helpers.ts

@@ -1,6 +1,7 @@
 import http from "http";
 import type { Readable } from "stream";
 import delay from "delay";
+import range from "lodash/range";
 import { Headers } from "node-fetch";
 import { DEFAULT_OPTIONS, RestRequest, RestResponse, RestStream } from "..";
 import type { RestFetchReaderOptions } from "../internal/RestFetchReader";
@@ -9,6 +10,9 @@ import RestFetchReader from "../internal/RestFetchReader";
 // Two European Euro symbols in UTF-8.
 export const UTF8_BUF = Buffer.from([0xe2, 0x82, 0xac, 0xe2, 0x82, 0xac]);
 
+// Binary data, all possible bytes.
+export const BINARY_BUF = Buffer.from(range(256));
+
 // eslint-disable-next-line @typescript-eslint/no-misused-promises
 export const server = http.createServer(async (req, res) => {
   res.setHeader("Content-Type", "text/plain; charset=utf-8");
@@ -66,6 +70,18 @@ export const server = http.createServer(async (req, res) => {
     return res.end();
   }
 
+  // Returns a binary response.
+  if (req.url === "/binary") {
+    res.setHeader("Content-Type", "application/octet-stream");
+    res.writeHead(200);
+    for (const _ in range(10)) {
+      await write(res, BINARY_BUF);
+      await delay(50);
+    }
+
+    return res.end();
+  }
+
   res.writeHead(404);
   return res.end("not found");
 });

src/internal/RestFetchReader.ts

@@ -2,6 +2,7 @@ import AbortControllerPolyfilled from "abort-controller";
 import { Memoize } from "fast-typescript-memoize";
 import type { RequestInit } from "node-fetch";
 import fetch, { Headers, Request } from "node-fetch";
+import inferResBodyEncoding from "./inferResBodyEncoding";
 
 export interface RestFetchReaderOptions {
   timeoutMs?: number;
@@ -90,7 +91,7 @@ export default class RestFetchReader {
 
         this._textFetched += value;
       }
-    } catch (e) {
+    } catch (e: unknown) {
       await generator.return();
       throw e;
     }
@@ -140,15 +141,11 @@ export default class RestFetchReader {
       this._status = res.status;
       this._headers = res.headers;
 
-      // An opinionated choice is made here to always decode the response stream
-      // as UTF-8. This is because JSON is by definition a UTF-8 stream. In the
-      // future, when we need binary streams, we can tweak it by introducing
-      // some intermediate layer and doing some refactoring, but for now it's an
-      // overkill. See also
-      // https://nodejs.org/api/stream.html#readablesetencodingencoding on how
-      // Node streams handle decoding when the returned chunks cross the
-      // boundaries of multi-byte characters.
-      res.body.setEncoding("utf-8");
+      // See https://nodejs.org/api/stream.html#readablesetencodingencoding on
+      // how Node streams and setEncoding() handle decoding when the returned
+      // chunks cross the boundaries of multi-byte characters (TL;DR: it works
+      // fine, that's why we work with string and not Buffer here).
+      res.body.setEncoding(inferResBodyEncoding(res));
 
       await this._options.heartbeat?.();
       for await (const chunk of res.body) {
@@ -157,7 +154,7 @@ export default class RestFetchReader {
         yield chunk as string;
         onAfterRead?.(this);
       }
-    } catch (e) {
+    } catch (e: unknown) {
       if (controller.signal.aborted && onTimeout) {
         onTimeout(this, e);
       } else {

src/internal/inferResBodyEncoding.ts

@@ -0,0 +1,33 @@
+import type { Response } from "node-fetch";
+
+const CHARSET_RE =
+  /(?:charset|encoding)\s{0,10}=\s{0,10}['"]? {0,10}([-\w]{1,100})/i;
+const BUFFER_ENCODINGS = ["ascii", "utf8", "utf-8", "utf16le", "ucs2", "ucs-2"];
+
+/**
+ * Tries its best to infer the encoding of the Response, falling back to UTF-8
+ * as an opinionated default value on failure.
+ */
+export default function inferResBodyEncoding(res: Response): BufferEncoding {
+  const contentType = res.headers.get("content-type")?.toLowerCase();
+  const charset = contentType?.match(CHARSET_RE)
+    ? RegExp.$1.toLowerCase()
+    : undefined;
+  return contentType?.startsWith("application/octet-stream")
+    ? // It's a binary Content-Type.
+      "binary"
+    : charset && !BUFFER_ENCODINGS.includes(charset)
+    ? // The charset is provided in Content-Type, but unknown by Buffer.
+      "binary"
+    : charset && BUFFER_ENCODINGS.includes(charset)
+    ? // Charset is provided in Content-Type header, and Buffer knows
+      // how to decode it.
+      (charset as BufferEncoding)
+    : // An opinionated choice is made here to always default-decode the
+      // response stream as UTF-8. This is because JSON is by definition a UTF-8
+      // stream, and people often time respond with JSONs forgetting to provide
+      // "; charset=utf-8" part of the Content-Type header (or they forget
+      // Content-Type header at all, or put some wrong value as "text/plain"
+      // there; there is an endless list of mistake variations here).
+      "utf-8";
+}