From 8946b74e2d3e7cc769c864f7aed9536324d38d9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Benedikt=20R=C3=B6tsch?= Date: Mon, 15 Mar 2021 17:30:54 +0100 Subject: [PATCH] fix: pretty print Contentful errors after all retry attempts failed --- .../npm/__snapshots__/package.test.js.snap | 10 + .../src/__tests__/fetch-network-errors.js | 219 ++++++++++++++++++ .../gatsby-source-contentful/src/fetch.js | 90 ++++--- 3 files changed, 290 insertions(+), 29 deletions(-) create mode 100644 packages/gatsby-source-contentful/src/__tests__/fetch-network-errors.js diff --git a/packages/gatsby-recipes/src/providers/npm/__snapshots__/package.test.js.snap b/packages/gatsby-recipes/src/providers/npm/__snapshots__/package.test.js.snap index 1386303baa81e..b04bae9391dd4 100644 --- a/packages/gatsby-recipes/src/providers/npm/__snapshots__/package.test.js.snap +++ b/packages/gatsby-recipes/src/providers/npm/__snapshots__/package.test.js.snap @@ -66,6 +66,16 @@ Object { } `; +exports[`npm package resource installs 2 resources, one prod & one dev: NPMPackage destroy 1`] = ` +Object { + "_message": "Installed NPM package is-sorted@1.0.2", + "description": "A small module to check if an Array is sorted", + "id": "is-sorted", + "name": "is-sorted", + "version": "1.0.2", +} +`; + exports[`package manager client commands generates the correct commands for npm 1`] = ` Array [ "install", diff --git a/packages/gatsby-source-contentful/src/__tests__/fetch-network-errors.js b/packages/gatsby-source-contentful/src/__tests__/fetch-network-errors.js new file mode 100644 index 0000000000000..27c7045dadd87 --- /dev/null +++ b/packages/gatsby-source-contentful/src/__tests__/fetch-network-errors.js @@ -0,0 +1,219 @@ +/** + * @jest-environment node + */ + +import nock from "nock" +import fetchData from "../fetch" +import { createPluginConfig } from "../plugin-options" + +nock.disableNetConnect() + +const host = `localhost` +const options = { + spaceId: `12345`, + accessToken: `67890`, + host, + contentfulClientConfig: { + retryLimit: 2, + }, +} +const baseURI = `https://${host}` + +const start = jest.fn() +const end = jest.fn() +const mockActivity = { + start, + end, + tick: jest.fn(), + done: end, +} + +const reporter = { + info: jest.fn(), + verbose: jest.fn(), + panic: jest.fn(e => { + throw e + }), + activityTimer: jest.fn(() => mockActivity), + createProgress: jest.fn(() => mockActivity), +} + +const pluginConfig = createPluginConfig(options) + +describe(`fetch-retry`, () => { + afterEach(() => { + nock.cleanAll() + reporter.verbose.mockClear() + reporter.panic.mockClear() + }) + + test(`request retries when network timeout happens`, async () => { + const scope = nock(baseURI) + // Space + .get(`/spaces/${options.spaceId}/`) + .reply(200, { items: [] }) + // Locales + .get(`/spaces/${options.spaceId}/environments/master/locales`) + .reply(200, { items: [{ code: `en`, default: true }] }) + // Sync + .get( + `/spaces/${options.spaceId}/environments/master/sync?initial=true&limit=100` + ) + .times(1) + .replyWithError({ code: `ETIMEDOUT` }) + .get( + `/spaces/${options.spaceId}/environments/master/sync?initial=true&limit=100` + ) + .reply(200, { items: [] }) + // Content types + .get( + `/spaces/${options.spaceId}/environments/master/content_types?skip=0&limit=100&order=sys.createdAt` + ) + .reply(200, { items: [] }) + + await fetchData({ pluginConfig, reporter }) + + expect(reporter.panic).not.toBeCalled() + expect(scope.isDone()).toBeTruthy() + }) + + test(`request should fail after to many retries`, async () => { + // Due to the retries, this can take up to 10 seconds + jest.setTimeout(10000) + + const scope = nock(baseURI) + // Space + .get(`/spaces/${options.spaceId}/`) + .reply(200, { items: [] }) + // Locales + .get(`/spaces/${options.spaceId}/environments/master/locales`) + .reply(200, { items: [{ code: `en`, default: true }] }) + // Sync + .get( + `/spaces/${options.spaceId}/environments/master/sync?initial=true&limit=100` + ) + .times(3) + .reply( + 500, + { + sys: { + type: `Error`, + id: `MockedContentfulError`, + }, + message: `Mocked message of Contentful error`, + }, + { [`x-contentful-request-id`]: `123abc` } + ) + + try { + await fetchData({ pluginConfig, reporter }) + jest.fail() + } catch (e) { + const msg = expect(e.context.sourceMessage) + msg.toEqual( + expect.stringContaining( + `Fetching contentful data failed: 500 MockedContentfulError` + ) + ) + msg.toEqual(expect.stringContaining(`Request ID: 123abc`)) + msg.toEqual( + expect.stringContaining(`The request was sent with 3 attempts`) + ) + } + expect(reporter.panic).toBeCalled() + expect(scope.isDone()).toBeTruthy() + }) +}) + +describe(`fetch-network-errors`, () => { + test(`catches plain network error`, async () => { + const scope = nock(baseURI) + // Space + .get(`/spaces/${options.spaceId}/`) + .replyWithError({ code: `ECONNRESET` }) + try { + await fetchData({ + pluginConfig: createPluginConfig({ + ...options, + contentfulClientConfig: { retryOnError: false }, + }), + reporter, + }) + jest.fail() + } catch (e) { + expect(e.context.sourceMessage).toEqual( + expect.stringContaining( + `Accessing your Contentful space failed: ECONNRESET` + ) + ) + } + + expect(reporter.panic).toBeCalled() + expect(scope.isDone()).toBeTruthy() + }) + + test(`catches error with response string`, async () => { + const scope = nock(baseURI) + // Space + .get(`/spaces/${options.spaceId}/`) + .reply(502, `Bad Gateway`) + + try { + await fetchData({ + pluginConfig: createPluginConfig({ + ...options, + contentfulClientConfig: { retryOnError: false }, + }), + reporter, + }) + jest.fail() + } catch (e) { + expect(e.context.sourceMessage).toEqual( + expect.stringContaining( + `Accessing your Contentful space failed: Bad Gateway` + ) + ) + } + + expect(reporter.panic).toBeCalled() + expect(scope.isDone()).toBeTruthy() + }) + + test(`catches error with response object`, async () => { + const scope = nock(baseURI) + // Space + .get(`/spaces/${options.spaceId}/`) + .reply(429, { + sys: { + type: `Error`, + id: `MockedContentfulError`, + }, + message: `Mocked message of Contentful error`, + requestId: `123abc`, + }) + + try { + await fetchData({ + pluginConfig: createPluginConfig({ + ...options, + contentfulClientConfig: { retryOnError: false }, + }), + reporter, + }) + jest.fail() + } catch (e) { + const msg = expect(e.context.sourceMessage) + + msg.toEqual( + expect.stringContaining( + `Accessing your Contentful space failed: MockedContentfulError` + ) + ) + msg.toEqual(expect.stringContaining(`Mocked message of Contentful error`)) + msg.toEqual(expect.stringContaining(`Request ID: 123abc`)) + } + + expect(reporter.panic).toBeCalled() + expect(scope.isDone()).toBeTruthy() + }) +}) diff --git a/packages/gatsby-source-contentful/src/fetch.js b/packages/gatsby-source-contentful/src/fetch.js index 1b569482fe449..30d45d079bede 100644 --- a/packages/gatsby-source-contentful/src/fetch.js +++ b/packages/gatsby-source-contentful/src/fetch.js @@ -4,6 +4,50 @@ const chalk = require(`chalk`) const { formatPluginOptionsForCLI } = require(`./plugin-options`) const { CODES } = require(`./report`) +const createContentfulErrorMessage = e => { + if (typeof e === `string`) { + return e + } + + // If we get a response, use this as basis to create the error message + // Contentful JS SDK tends to give errors with different structures: + // https://github.com/contentful/contentful.js/blob/b67b77ac8c919c4ec39203f8cac2043854ab0014/lib/create-contentful-api.js#L89-L99 + if (e.response) { + e = { ...e, ...e.response, ...(e.response?.data || {}) } + } + + let errorMessage = [ + // Generic error responses + e.code && `${e.code}`, + e.status && `${e.status}`, + e.statusText, + // Contentful API error Responses + e?.sys?.id, + ] + .filter(Boolean) + .join(` `) + + if (e.message) { + errorMessage += `\n\n${e.message}` + } + + const requestId = + (e.headers && + typeof e.headers === `object` && + e.headers[`x-contentful-request-id`]) || + e.requestId + + if (requestId) { + errorMessage += `\n\nRequest ID: ${requestId}` + } + + if (e.attempts) { + errorMessage += `\n\nThe request was sent with ${e.attempts} attempts` + } + + return errorMessage +} + module.exports = async function contentfulFetch({ syncToken, pluginConfig, @@ -21,8 +65,8 @@ module.exports = async function contentfulFetch({ integration: `gatsby-source-contentful`, responseLogger: response => { function createMetadataLog(response) { - if (process.env.gatsby_log_level === `verbose`) { - return `` + if (!response.headers) { + return null } return [ response?.headers[`content-length`] && @@ -36,31 +80,12 @@ module.exports = async function contentfulFetch({ .join(` `) } - // Log error and throw it in an extended shape - if (response.isAxiosError) { - reporter.verbose( - `${response.config.method} /${response.config.url}: ${ - response.response.status - } ${response.response.statusText} (${createMetadataLog( - response.response - )})` - ) - let errorMessage = `${response.response.status} ${response.response.statusText}` - if (response.response?.data?.message) { - errorMessage += `\n\n${response.response.data.message}` - } - const contentfulApiError = new Error(errorMessage) - // Special response naming to ensure the error object is not touched by - // https://github.com/contentful/contentful.js/commit/41039afa0c1462762514c61458556e6868beba61 - contentfulApiError.responseData = response.response - contentfulApiError.request = response.request - contentfulApiError.config = response.config - - throw contentfulApiError - } - // Sync progress - if (response.config.url === `sync`) { + if ( + response.config.url === `sync` && + !response.isAxiosError && + response?.data.items + ) { syncProgress.tick(response.data.items.length) } @@ -137,7 +162,10 @@ module.exports = async function contentfulFetch({ reporter.panic({ context: { - sourceMessage: `Accessing your Contentful space failed: ${e.message} + sourceMessage: `Accessing your Contentful space failed: ${createContentfulErrorMessage( + e + )} + Try setting GATSBY_CONTENTFUL_OFFLINE=true to see if we can serve from cache. ${details ? `\n${details}\n` : ``} Used options: @@ -168,7 +196,9 @@ ${formatPluginOptionsForCLI(pluginConfig.getOriginalPluginOptions(), errors)}`, { id: CODES.SyncError, context: { - sourceMessage: `Fetching contentful data failed: ${e.message}`, + sourceMessage: `Fetching contentful data failed: ${createContentfulErrorMessage( + e + )}`, }, }, e @@ -187,7 +217,9 @@ ${formatPluginOptionsForCLI(pluginConfig.getOriginalPluginOptions(), errors)}`, { id: CODES.FetchContentTypes, context: { - sourceMessage: `Error fetching content types: ${e.message}`, + sourceMessage: `Error fetching content types: ${createContentfulErrorMessage( + e + )}`, }, }, e