chore: improve getResponseData (#715)

* chore: improve getResponseData

* use fast-content-type-parse

* make more resilient

---------

Co-authored-by: wolfy1339 <[email protected]>

Author: Uzlopak

package-lock.json

@@ -28,6 +28,7 @@
     "@octokit/endpoint": "^10.0.0",
     "@octokit/request-error": "^6.0.1",
     "@octokit/types": "^13.1.0",
+    "fast-content-type-parse": "^2.0.0",
     "universal-user-agent": "^7.0.2"
   },
   "devDependencies": {

package.json

@@ -1,3 +1,4 @@
+import { safeParse } from "fast-content-type-parse";
 import { isPlainObject } from "./is-plain-object.js";
 import { RequestError } from "@octokit/request-error";
 import type { EndpointInterface, OctokitResponse } from "@octokit/types";
@@ -147,25 +148,29 @@ export default async function fetchWrapper(
 
 async function getResponseData(response: Response): Promise<any> {
   const contentType = response.headers.get("content-type");
-  if (/application\/json/.test(contentType!)) {
-    return (
-      response
-        .json()
-        // In the event that we get an empty response body we fallback to
-        // using .text(), but this should be investigated since if this were
-        // to occur in the GitHub API it really should not return an empty body.
-        .catch(() => response.text())
-        // `node-fetch` is throwing a "body used already for" error if `.text()` is run
-        // after a failed .json(). To account for that we fallback to an empty string
-        .catch(() => "")
-    );
-  }
 
-  if (!contentType || /^text\/|charset=utf-8$/.test(contentType)) {
-    return response.text();
+  if (!contentType) {
+    return response.text().catch(() => "");
   }
 
-  return response.arrayBuffer();
+  const mimetype = safeParse(contentType);
+
+  if (mimetype.type === "application/json") {
+    let text = "";
+    try {
+      text = await response.text();
+      return JSON.parse(text);
+    } catch (err) {
+      return text;
+    }
+  } else if (
+    mimetype.type.startsWith("text/") ||
+    mimetype.parameters.charset?.toLowerCase() === "utf-8"
+  ) {
+    return response.text().catch(() => "");
+  } else {
+    return response.arrayBuffer().catch(() => new ArrayBuffer(0));
+  }
 }
 
 function toErrorMessage(data: string | ArrayBuffer | Record<string, unknown>) {

src/fetch-wrapper.ts

@@ -65,6 +65,9 @@ describe("request()", () => {
             json() {
               return Promise.resolve("funk");
             },
+            text() {
+              return Promise.resolve("funk");
+            },
           }),
       },
     });

test/request-common.test.ts

@@ -1397,4 +1397,23 @@ x//0u+zd/R/QRUzLOw4N72/Hu+UG6MNt5iDZFCtapRaKt6OvSBwy8w==
 
     request.closeMockServer();
   });
+
+  it("invalid json as response data", async () => {
+    expect.assertions(4);
+
+    const request = await mockRequestHttpServer(async (req, res) => {
+      expect(req.method).toBe("GET");
+      expect(req.url).toBe("/");
+
+      res.writeHead(200, {
+        "content-type": "application/json",
+      });
+      res.end('"invalid');
+    });
+
+    const response = await request("GET /");
+
+    expect(response.status).toEqual(200);
+    expect(response.data).toEqual('"invalid');
+  });
 });

test/request-native-fetch.test.ts

@@ -702,6 +702,9 @@ x//0u+zd/R/QRUzLOw4N72/Hu+UG6MNt5iDZFCtapRaKt6OvSBwy8w==
             json() {
               return Promise.resolve("funk");
             },
+            text() {
+              return Promise.resolve("funk");
+            },
           }),
       },
     });