diff --git a/src/components/containers/MetricOverviewContainer.jsx b/src/components/containers/MetricOverviewContainer.jsx index 20e73a1..07306c3 100644 --- a/src/components/containers/MetricOverviewContainer.jsx +++ b/src/components/containers/MetricOverviewContainer.jsx @@ -39,11 +39,6 @@ class MetricOverviewContainer extends React.Component { // normal. return null; - - // Due to the way ensemble is currently hosted, this code is not run - // when metric data is not found. - // - // https://github.com/mozilla/ensemble/issues/313 } else if (dataFetch.rejected) { if (dataFetch.reason && dataFetch.reason.message) { // eslint-disable-next-line no-console diff --git a/src/lib/withFetch.jsx b/src/lib/withFetch.jsx index 22a458d..41b956f 100644 --- a/src/lib/withFetch.jsx +++ b/src/lib/withFetch.jsx @@ -10,9 +10,11 @@ async function fetchJSON(url) { })); if (!response.ok) { - // An error response may have no body, or one that isn't JSON. + // An error response may have no body, or one that isn't JSON (Cloud + // Storage answers a missing file with XML), and HTTP/2 responses have + // no statusText. const body = await response.json().catch(() => null); - throw new Error(body?.error || body?.message || response.statusText); + throw new Error(body?.error || body?.message || response.statusText || `HTTP ${response.status}`); } if (response.status === 204 || response.headers.get('content-length') === '0') { diff --git a/src/tests/jest/withFetch.test.jsx b/src/tests/jest/withFetch.test.jsx index 7ef279f..ffc6193 100644 --- a/src/tests/jest/withFetch.test.jsx +++ b/src/tests/jest/withFetch.test.jsx @@ -13,6 +13,10 @@ function settle(url, body, httpStatus = 200) { })); } +function settleWithText(url, text, httpStatus) { + pendingRequests[url](new Response(text, { status: httpStatus })); +} + const ShowFetchState = withFetch(props => `https://data.example/${props.slug}.json`)(({ dataFetch }) => { if (dataFetch.pending) return
pending
; if (dataFetch.rejected) returnrejected: {dataFetch.reason.message}
; @@ -71,6 +75,22 @@ it('Rejects an empty non-2xx response with its status text', async () => { expect(await screen.findByText('rejected: Not Found')).toBeInTheDocument(); }); +it('Rejects a non-2xx response whose body is not JSON with its HTTP status', async () => { + render(NoSuchKeyNoSuchKey