From ac3f6114d8604b51f91b1f58e71d772433ed52a6 Mon Sep 17 00:00:00 2001 From: June Kelly Date: Tue, 24 Aug 2021 09:54:30 +0100 Subject: [PATCH] Merge pull request #4836 from overleaf/jk-fetch-json-error-message FetchError: set error message when statusText it missing GitOrigin-RevId: 05461a6918af3ee339e66df2abc48635a082f6b7 --- .../frontend/js/infrastructure/fetch-json.js | 32 +++++++++++++++++++ .../infrastructure/fetch-json.test.js | 24 ++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/services/web/frontend/js/infrastructure/fetch-json.js b/services/web/frontend/js/infrastructure/fetch-json.js index 681d8f6d91..cf297d8850 100644 --- a/services/web/frontend/js/infrastructure/fetch-json.js +++ b/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 diff --git a/services/web/test/frontend/infrastructure/fetch-json.test.js b/services/web/test/frontend/infrastructure/fetch-json.test.js index fc25001f3a..6d29cffdaa 100644 --- a/services/web/test/frontend/infrastructure/fetch-json.test.js +++ b/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 }