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) return

rejected: {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(); + + settleWithText('https://data.example/one.json', 'NoSuchKey', 404); + + expect(await screen.findByText('rejected: HTTP 404')).toBeInTheDocument(); +}); + +it('Rejects a 2xx response whose body is not JSON', async () => { + render(); + + settleWithText('https://data.example/one.json', '', 200); + + expect(await screen.findByText(/^rejected: /)).toBeInTheDocument(); +}); + it('Ignores a response that arrives after a newer request has started', async () => { const { rerender } = render(); rerender(); diff --git a/tests/playwright/specs/fixtures/errors.spec.js b/tests/playwright/specs/fixtures/errors.spec.js index 416acd4..f6ce893 100644 --- a/tests/playwright/specs/fixtures/errors.spec.js +++ b/tests/playwright/specs/fixtures/errors.spec.js @@ -2,7 +2,7 @@ const { test, expect, readFixture } = require('../../fixtureData'); // The browser logs the failed request, and the container logs the reason. -test.use({ allowedErrors: /status of 500|Internal Server Error/ }); +test.use({ allowedErrors: /status of (500|404)|Internal Server Error|Not Found/ }); const internalServerError = { status: 500, json: { error: 'Internal Server Error' } }; @@ -28,3 +28,21 @@ test('A metric that fails to load shows #metric-overview-fetch-error while the o await expect(page.locator('#metric-overview-fetch-error')).toBeVisible(); await expect(page.locator('#metric-overview-2 h4')).toHaveText(secondMetric.title); }); + +test('A metric that does not exist shows #metric-overview-fetch-error', async ({ page }) => { + const dashboard = readFixture('user-activity/index.json'); + const [firstSlug] = dashboard.metrics; + // This is how Cloud Storage, which hosts the data, answers a missing file. + await page.route( + url => decodeURIComponent(url.pathname).endsWith(`/user-activity/${dashboard.defaultCategory}/${firstSlug}/index.json`), + route => route.fulfill({ + status: 404, + contentType: 'application/xml; charset=UTF-8', + body: "NoSuchKey", + }) + ); + + await page.goto('/dashboard/user-activity'); + + await expect(page.locator('#metric-overview-fetch-error')).toBeVisible(); +});