Merge pull request #3618 from overleaf/ae-fetch-json-oerror

Ensure that fetchJSON always throws OError

GitOrigin-RevId: 24bea67992e7b5957b2eaaf5ffe2202879534d39

services/web/frontend/js/infrastructure/fetch-json.js

@@ -3,24 +3,69 @@
 // - set the JSON content-type in the request headers
 // - throw errors on non-ok response
 // - parse JSON response body, unless response is empty
-const OError = require('@overleaf/o-error')
+import OError from '@overleaf/o-error'
 
+/**
+ * @typedef {Object} FetchOptions
+ * @extends RequestInit
+ * @property {Object} body
+ */
+
+/**
+ * @param {string} path
+ * @param {FetchOptions} [options]
+ */
 export function getJSON(path, options) {
   return fetchJSON(path, { ...options, method: 'GET' })
 }
 
+/**
+ * @param {string} path
+ * @param {FetchOptions} [options]
+ */
 export function postJSON(path, options) {
   return fetchJSON(path, { ...options, method: 'POST' })
 }
 
+/**
+ * @param {string} path
+ * @param {FetchOptions} [options]
+ */
 export function putJSON(path, options) {
   return fetchJSON(path, { ...options, method: 'PUT' })
 }
 
+/**
+ * @param {string} path
+ * @param {FetchOptions} [options]
+ */
 export function deleteJSON(path, options) {
   return fetchJSON(path, { ...options, method: 'DELETE' })
 }
 
+export class FetchError extends OError {
+  /**
+   * @param {string} message
+   * @param {string} url
+   * @param {FetchOptions} [options]
+   * @param {Response} [response]
+   * @param {Object} [data]
+   */
+  constructor(message, url, options, response, data) {
+    super(message, { statusCode: response ? response.status : undefined })
+    this.url = url
+    this.options = options
+    this.response = response
+    this.data = data
+  }
+}
+
+/**
+ * @param {string} path
+ * @param {FetchOptions} [options]
+ *
+ * @return Promise<Object>
+ */
 function fetchJSON(
   path,
   {
@@ -48,36 +93,58 @@ function fetchJSON(
   }
 
   return fetch(path, options)
-    .then(parseResponseBody)
+    .catch(error => {
-    .then(({ data, response }) => {
+      // the fetch failed
-      if (!response.ok) {
+      throw new FetchError(
-        throw new OError(response.statusText, {
+        'There was an error fetching the JSON',
-          statusCode: response.status,
+        path,
-          data,
+        options
-          response
+      ).withCause(error)
+    })
+    .then(response => {
+      return parseResponseBody(response)
+        .catch(error => {
+          // parsing the response body failed
+          throw new FetchError(
+            'There was an error parsing the response body',
+            path,
+            options,
+            response
+          ).withCause(error)
         })
-      }
+        .then(data => {
+          if (!response.ok) {
+            // the response from the server was not 2xx
+            throw new FetchError(
+              response.statusText,
+              path,
+              options,
+              response,
+              data
+            )
+          }
 
-      return data
+          return data
+        })
     })
 }
 
+/**
+ * @param {Response} response
+ * @returns {Promise<Object>}
+ */
 function parseResponseBody(response) {
   const contentType = response.headers.get('Content-Type')
   if (/application\/json/.test(contentType)) {
-    return response.json().then(data => {
+    return response.json()
-      return { data, response }
-    })
   } else if (
     /text\/plain/.test(contentType) ||
     /text\/html/.test(contentType)
   ) {
-    return response.text().then(text => {
+    return response.text().then(message => ({ message }))
-      return { data: { message: text }, response }
-    })
   } else {
     // response body ignored as content-type is either not set (e.g. 204
     // responses) or unsupported
-    return Promise.resolve({ data: {}, response })
+    return Promise.resolve({})
   }
 }

services/web/test/frontend/infrastructure/fetch-json.test.js

@@ -1,8 +1,8 @@
 import { expect } from 'chai'
 import fetchMock from 'fetch-mock'
-import OError from '@overleaf/o-error'
 import {
   deleteJSON,
+  FetchError,
   getJSON,
   postJSON,
   putJSON
@@ -41,10 +41,12 @@ describe('fetchJSON', function() {
 
     return expect(getJSON('/test'))
       .to.eventually.be.rejectedWith('Bad Request')
-      .and.be.an.instanceOf(OError)
+      .and.be.an.instanceOf(FetchError)
       .to.nested.include({
-        'info.response.status': 400,
+        message: 'Bad Request',
-        'info.data.message': 'The request was invalid'
+        'data.message': 'The request was invalid',
+        'response.status': 400,
+        'info.statusCode': 400
       })
   })
 
@@ -53,9 +55,10 @@ describe('fetchJSON', function() {
 
     return expect(getJSON('/test'))
       .to.eventually.be.rejectedWith('Internal Server Error')
-      .and.be.an.instanceOf(OError)
+      .and.be.an.instanceOf(FetchError)
       .to.nested.include({
-        'info.response.status': 500
+        'response.status': 500,
+        'info.statusCode': 500
       })
   })