Просмотр исходного кода

Merge pull request #4836 from overleaf/jk-fetch-json-error-message

FetchError: set error message when statusText it missing
GitOrigin-RevId: 05461a6918af3ee339e66df2abc48635a082f6b7
June Kelly 5 лет назад
Родитель
Сommit
ac3f6114d8

+ 32 - 0
services/web/frontend/js/infrastructure/fetch-json.js

@@ -43,6 +43,31 @@ export function deleteJSON(path, options) {
   return fetchJSON(path, { ...options, method: 'DELETE' })
 }
 
+/**
+ * @param {number} statusCode
+ * @returns {string}
+ */
+function getErrorMessageForStatusCode(statusCode) {
+  switch (statusCode) {
+    case 400:
+      return 'Bad Request'
+    case 401:
+      return 'Unauthorized'
+    case 403:
+      return 'Forbidden'
+    case 404:
+      return 'Not Found'
+    case 500:
+      return 'Internal Server Error'
+    case 502:
+      return 'Bad Gateway'
+    case 503:
+      return 'Service Unavailable'
+    default:
+      return `Unexpected Error: ${statusCode}`
+  }
+}
+
 export class FetchError extends OError {
   /**
    * @param {string} message
@@ -52,6 +77,13 @@ export class FetchError extends OError {
    * @param {Object} [data]
    */
   constructor(message, url, options, response, data) {
+    // On HTTP2, the `statusText` property is not set,
+    // so this `message` will be undefined. We need to
+    // set a message based on the response `status`, so
+    // our error UI rendering will work
+    if (!message) {
+      message = getErrorMessageForStatusCode(response.status)
+    }
     super(message, { statusCode: response ? response.status : undefined })
     this.url = url
     this.options = options

+ 24 - 0
services/web/test/frontend/infrastructure/fetch-json.test.js

@@ -1,5 +1,6 @@
 import { expect } from 'chai'
 import fetchMock from 'fetch-mock'
+import { Response } from 'node-fetch'
 import {
   deleteJSON,
   FetchError,
@@ -136,6 +137,29 @@ describe('fetchJSON', function () {
     }
   })
 
+  it('handles 5xx responses without a status message', async function () {
+    // It's hard to make a Response object with statusText=null,
+    // so we need to do some monkey-work to make it happen
+    const response = new Response('weird scary error', {
+      ok: false,
+      status: 599,
+    })
+    Object.defineProperty(response, 'statusText', {
+      get: () => null,
+      set: () => {},
+    })
+    fetchMock.get('/test', response)
+
+    return expect(getJSON('/test'))
+      .to.eventually.be.rejectedWith('Unexpected Error: 599')
+      .and.be.an.instanceOf(FetchError)
+      .to.nested.include({
+        'response.status': 599,
+        'info.statusCode': 599,
+        message: 'Unexpected Error: 599',
+      })
+  })
+
   it('handles POST requests', function () {
     const body = { example: true }