Merge pull request #13764 from Budibase/fix/rest-content-processing

Changing REST content response processing

packages/server/src/api/routes/tests/queries/rest.spec.ts

@@ -64,13 +64,11 @@ describe("rest", () => {
     cached = await getCachedVariable(basedOnQuery._id!, "foo")
     expect(cached).toBeNull()
 
-    nock("http://one.example.com")
+    const body1 = [{ name: "one" }]
-      .get("/")
+    const body2 = [{ name: "two" }]
-      .reply(200, [{ name: "one" }])
+    nock("http://one.example.com").get("/").reply(200, body1)
     nock("http://two.example.com").get("/?test=one").reply(500)
-    nock("http://two.example.com")
+    nock("http://two.example.com").get("/?test=one").reply(200, body2)
-      .get("/?test=one")
-      .reply(200, [{ name: "two" }])
 
     const res = await config.api.query.preview({
       datasourceId: datasource._id!,

packages/server/src/integrations/rest.ts

@@ -148,6 +148,10 @@ class RestIntegration implements IntegrationBase {
       response.headers,
       { downloadImages: this.config.downloadImages }
     )
+    let contentLength = response.headers.get("content-length")
+    if (!contentLength && raw) {
+      contentLength = Buffer.byteLength(raw, "utf8").toString()
+    }
     if (
       contentDisposition.includes("filename") ||
       contentDisposition.includes("attachment") ||
@@ -156,36 +160,46 @@ class RestIntegration implements IntegrationBase {
       filename =
         path.basename(parse(contentDisposition).parameters?.filename) || ""
     }
+
+    let triedParsing: boolean = false,
+      responseTxt: string | undefined
     try {
       if (filename) {
         return handleFileResponse(response, filename, this.startTimeMs)
       } else {
+        responseTxt = response.text ? await response.text() : ""
+        const hasContent =
+          (contentLength && parseInt(contentLength) > 0) ||
+          responseTxt.length > 0
         if (response.status === 204) {
           data = []
           raw = ""
-        } else if (contentType.includes("application/json")) {
+        } else if (hasContent && contentType.includes("application/json")) {
-          data = await response.json()
+          triedParsing = true
-          raw = JSON.stringify(data)
+          data = JSON.parse(responseTxt)
+          raw = responseTxt
         } else if (
-          contentType.includes("text/xml") ||
+          (hasContent && contentType.includes("text/xml")) ||
           contentType.includes("application/xml")
         ) {
-          let xmlResponse = await handleXml(response)
+          triedParsing = true
+          let xmlResponse = await handleXml(responseTxt)
           data = xmlResponse.data
           raw = xmlResponse.rawXml
         } else {
-          data = await response.text()
+          data = responseTxt
           raw = data as string
         }
       }
     } catch (err) {
-      throw `Failed to parse response body: ${err}`
+      if (triedParsing) {
+        data = responseTxt
+        raw = data as string
+      } else {
+        throw new Error(`Failed to parse response body: ${err}`)
+      }
     }
 
-    let contentLength = response.headers.get("content-length")
-    if (!contentLength && raw) {
-      contentLength = Buffer.byteLength(raw, "utf8").toString()
-    }
     const size = formatBytes(contentLength || "0")
     const time = `${Math.round(performance.now() - this.startTimeMs)}ms`
     headers = response.headers.raw()

packages/server/src/integrations/tests/rest.spec.ts

@@ -1,19 +1,27 @@
 jest.mock("node-fetch", () => {
+  const obj = {
+    my_next_cursor: 123,
+  }
+  const str = JSON.stringify(obj)
   return jest.fn(() => ({
     headers: {
       raw: () => {
-        return { "content-type": ["application/json"] }
+        return {
+          "content-type": ["application/json"],
+          "content-length": str.length,
+        }
       },
       get: (name: string) => {
-        if (name.toLowerCase() === "content-type") {
+        const lcName = name.toLowerCase()
+        if (lcName === "content-type") {
           return ["application/json"]
+        } else if (lcName === "content-length") {
+          return str.length
         }
       },
     },
-    json: jest.fn(() => ({
+    json: jest.fn(() => obj),
-      my_next_cursor: 123,
+    text: jest.fn(() => str),
-    })),
-    text: jest.fn(),
   }))
 })
 
@@ -231,7 +239,8 @@ describe("REST Integration", () => {
     }
 
     it("should be able to parse JSON response", async () => {
-      const input = buildInput({ a: 1 }, null, "application/json")
+      const obj = { a: 1 }
+      const input = buildInput(obj, JSON.stringify(obj), "application/json")
       const output = await config.integration.parseResponse(input)
       expect(output.data).toEqual({ a: 1 })
       expect(output.info.code).toEqual(200)
@@ -261,7 +270,7 @@ describe("REST Integration", () => {
     test.each([...contentTypes, undefined])(
       "should not throw an error on 204 no content",
       async contentType => {
-        const input = buildInput(undefined, null, contentType, 204)
+        const input = buildInput(undefined, "", contentType, 204)
         const output = await config.integration.parseResponse(input)
         expect(output.data).toEqual([])
         expect(output.extra.raw).toEqual("")

packages/server/src/integrations/utils/utils.ts

@@ -485,9 +485,8 @@ export function isValidFilter(value: any) {
   return value != null && value !== ""
 }
 
-export async function handleXml(response: any) {
+export async function handleXml(rawXml: string) {
-  let data,
+  let data
-    rawXml = await response.text()
   data =
     (await xmlParser(rawXml, {
       explicitArray: false,