From 8b454e911688d9ca442ca36b4c59d34b21d12ac6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Benedikt=20R=C3=B6tsch?= Date: Mon, 15 Mar 2021 14:36:10 +0100 Subject: [PATCH 01/16] fix: do not log empty values when logging requests --- packages/gatsby-source-contentful/src/fetch.js | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/packages/gatsby-source-contentful/src/fetch.js b/packages/gatsby-source-contentful/src/fetch.js index 67c70ee965988..54afac590d622 100644 --- a/packages/gatsby-source-contentful/src/fetch.js +++ b/packages/gatsby-source-contentful/src/fetch.js @@ -67,10 +67,17 @@ module.exports = async function contentfulFetch({ syncProgress.tick(response.data.items.length) } + const metadataLog = createMetadataLog(response) + reporter.verbose( - `${response.config.method} /${response.config.url}: ${ - response.status - } ${response.statusText} (${createMetadataLog(response)})` + [ + `${response.config.method} /${response.config.url}:`, + response.status, + response.statusText, + metadataLog && `(${metadataLog})`, + ] + .filter(Boolean) + .join(` `) ) }, } From 356884567070476b722d63b2654700c30ad603b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Benedikt=20R=C3=B6tsch?= Date: Mon, 15 Mar 2021 14:37:17 +0100 Subject: [PATCH 02/16] fix: remove progress total as the real total is not available --- packages/gatsby-source-contentful/src/fetch.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/gatsby-source-contentful/src/fetch.js b/packages/gatsby-source-contentful/src/fetch.js index 54afac590d622..42d5a5955008d 100644 --- a/packages/gatsby-source-contentful/src/fetch.js +++ b/packages/gatsby-source-contentful/src/fetch.js @@ -11,7 +11,6 @@ module.exports = async function contentfulFetch({ }) { // Fetch articles. let syncProgress - let syncItemCount = 0 const pageLimit = pluginConfig.get(`pageLimit`) const contentfulClientOptions = { space: pluginConfig.get(`spaceId`), @@ -62,8 +61,6 @@ module.exports = async function contentfulFetch({ // Sync progress if (response.config.url === `sync`) { - syncItemCount += response.data.items.length - syncProgress.total = syncItemCount syncProgress.tick(response.data.items.length) } From 227754cb2a7d8e375e7e32836995f0569b7c7cea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Benedikt=20R=C3=B6tsch?= Date: Mon, 15 Mar 2021 17:29:59 +0100 Subject: [PATCH 03/16] fix: allow passing of any option to Contentful SDK again --- packages/gatsby-source-contentful/README.md | 8 ++++++-- packages/gatsby-source-contentful/src/fetch.js | 2 ++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/gatsby-source-contentful/README.md b/packages/gatsby-source-contentful/README.md index 9bcc639294b73..10b2642af2b1d 100644 --- a/packages/gatsby-source-contentful/README.md +++ b/packages/gatsby-source-contentful/README.md @@ -145,8 +145,6 @@ The environment to pull the content from, for more info on environments check ou Downloads and caches `ContentfulAsset`'s to the local filesystem. Allows you to query a `ContentfulAsset`'s `localFile` field, which is not linked to Contentful's CDN. Useful for reducing data usage. -You can pass in any other options available in the [contentful.js SDK](https://github.com/contentful/contentful.js#configuration). - **`localeFilter`** [function][optional] [default: `() => true`] Possibility to limit how many locales/nodes are created in GraphQL. This can limit the memory usage by reducing the amount of nodes created. Useful if you have a large space in Contentful and only want to get the data from one selected locale. @@ -181,6 +179,12 @@ Number of entries to retrieve from Contentful at a time. Due to some technical l Number of workers to use when downloading Contentful assets. Due to technical limitations, opening too many concurrent requests can cause stalled downloads. If you encounter this issue you can set this param to a lower number than 50, e.g 25. +**`contentfulClientConfig`** [object][optional] [default: `{}`] + +Additional config which will get passed to [Contentfuls JS SDK](https://github.com/contentful/contentful.js#configuration). + +Use this with caution, you might override values this plugin sets for you to connect to Contentful. + ## Notes on Contentful Content Models There are currently some things to keep in mind when building your content models at Contentful. diff --git a/packages/gatsby-source-contentful/src/fetch.js b/packages/gatsby-source-contentful/src/fetch.js index 42d5a5955008d..1b569482fe449 100644 --- a/packages/gatsby-source-contentful/src/fetch.js +++ b/packages/gatsby-source-contentful/src/fetch.js @@ -77,6 +77,8 @@ module.exports = async function contentfulFetch({ .join(` `) ) }, + // Allow passing of custom configuration to the Contentful SDK like headers + ...(pluginConfig.get(`contentfulClientConfig`) || {}), } const client = contentful.createClient(contentfulClientOptions) 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 04/16] 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 From 6da14b6a43bc9971fe108b8240e935c629c0c002 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Benedikt=20R=C3=B6tsch?= Date: Tue, 16 Mar 2021 13:12:38 +0100 Subject: [PATCH 05/16] docs: add reintroduced contentfulClientConfig to the plugin validation --- packages/gatsby-source-contentful/README.md | 2 +- packages/gatsby-source-contentful/src/gatsby-node.js | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/gatsby-source-contentful/README.md b/packages/gatsby-source-contentful/README.md index 10b2642af2b1d..3b475d21e026b 100644 --- a/packages/gatsby-source-contentful/README.md +++ b/packages/gatsby-source-contentful/README.md @@ -183,7 +183,7 @@ Number of workers to use when downloading Contentful assets. Due to technical li Additional config which will get passed to [Contentfuls JS SDK](https://github.com/contentful/contentful.js#configuration). -Use this with caution, you might override values this plugin sets for you to connect to Contentful. +Use this with caution, you might override values this plugin does set for you to connect to Contentful. ## Notes on Contentful Content Models diff --git a/packages/gatsby-source-contentful/src/gatsby-node.js b/packages/gatsby-source-contentful/src/gatsby-node.js index bfc4d6e269bdf..d91e022e45cf6 100644 --- a/packages/gatsby-source-contentful/src/gatsby-node.js +++ b/packages/gatsby-source-contentful/src/gatsby-node.js @@ -134,6 +134,13 @@ List of locales and their codes can be found in Contentful app -> Settings -> Lo If you are confident your Content Types will have natural-language IDs (e.g. \`blogPost\`), then you should set this option to \`false\`. If you are unable to ensure this, then you should leave this option set to \`true\` (the default).` ) .default(true), + contentfulClientConfig: Joi.object() + .description( + `Additional config which will get passed to [Contentfuls JS SDK](https://github.com/contentful/contentful.js#configuration). + + Use this with caution, you might override values this plugin does set for you to connect to Contentful.` + ) + .default(true), // default plugins passed by gatsby plugins: Joi.array(), }) From 935f1d62c383fbe6b1917e7c247210bdde2bc593 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Benedikt=20R=C3=B6tsch?= Date: Tue, 16 Mar 2021 13:24:46 +0100 Subject: [PATCH 06/16] docs: add missing quote --- packages/gatsby-source-contentful/src/gatsby-node.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/gatsby-source-contentful/src/gatsby-node.js b/packages/gatsby-source-contentful/src/gatsby-node.js index d91e022e45cf6..2e647737698f3 100644 --- a/packages/gatsby-source-contentful/src/gatsby-node.js +++ b/packages/gatsby-source-contentful/src/gatsby-node.js @@ -45,7 +45,7 @@ const validateContentfulAccess = async pluginOptions => { pluginOptions.spaceId )}" on environment "${ pluginOptions.environment - } with access token "${maskText( + }" with access token "${maskText( pluginOptions.accessToken )}". Make sure to double check them!` From 9b032272586713fb70f385ec169e7fe850766eea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Benedikt=20R=C3=B6tsch?= Date: Tue, 16 Mar 2021 13:43:33 +0100 Subject: [PATCH 07/16] fix: allow error responses with JSON strings --- packages/gatsby-source-contentful/src/fetch.js | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/packages/gatsby-source-contentful/src/fetch.js b/packages/gatsby-source-contentful/src/fetch.js index 30d45d079bede..0764e971f7e97 100644 --- a/packages/gatsby-source-contentful/src/fetch.js +++ b/packages/gatsby-source-contentful/src/fetch.js @@ -13,7 +13,20 @@ const createContentfulErrorMessage = e => { // 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 || {}) } + e = { ...e, ...e.response } + + if (typeof e.response.data === `string`) { + try { + const data = JSON.parse(e.response.data) + if (data && typeof data === `object`) { + e = { ...e, ...data } + } + } catch (err) { + e.message = `Unable to extract API error data from:\n${e.response.data}` + } + } else if (typeof e.response.data === `object`) { + e = { ...e, ...e.response.data } + } } let errorMessage = [ From f2a6e7b1affba1e04176199f9699fd7a57d6e862 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Benedikt=20R=C3=B6tsch?= Date: Tue, 16 Mar 2021 14:33:14 +0100 Subject: [PATCH 08/16] fix: do not pass error object to reporter.panic to display custom message to the end user --- .../gatsby-source-contentful/src/fetch.js | 47 ++++++++----------- 1 file changed, 19 insertions(+), 28 deletions(-) diff --git a/packages/gatsby-source-contentful/src/fetch.js b/packages/gatsby-source-contentful/src/fetch.js index 0764e971f7e97..da5078b59f76d 100644 --- a/packages/gatsby-source-contentful/src/fetch.js +++ b/packages/gatsby-source-contentful/src/fetch.js @@ -142,15 +142,12 @@ module.exports = async function contentfulFetch({ if (e.code === `ENOTFOUND`) { details = `You seem to be offline` } else if (e.code === `SELF_SIGNED_CERT_IN_CHAIN`) { - reporter.panic( - { - id: CODES.SelfSignedCertificate, - context: { - sourceMessage: `We couldn't make a secure connection to your contentful space. Please check if you have any self-signed SSL certificates installed.`, - }, + reporter.panic({ + id: CODES.SelfSignedCertificate, + context: { + sourceMessage: `We couldn't make a secure connection to your contentful space. Please check if you have any self-signed SSL certificates installed.`, }, - e - ) + }) } else if (e.responseData) { if (e.responseData.status === 404) { // host and space used to generate url @@ -205,17 +202,14 @@ ${formatPluginOptionsForCLI(pluginConfig.getOriginalPluginOptions(), errors)}`, : { initial: true, ...basicSyncConfig } currentSyncData = await client.sync(query) } catch (e) { - reporter.panic( - { - id: CODES.SyncError, - context: { - sourceMessage: `Fetching contentful data failed: ${createContentfulErrorMessage( - e - )}`, - }, + reporter.panic({ + id: CODES.SyncError, + context: { + sourceMessage: `Fetching contentful data failed: ${createContentfulErrorMessage( + e + )}`, }, - e - ) + }) } finally { syncProgress.done() } @@ -226,17 +220,14 @@ ${formatPluginOptionsForCLI(pluginConfig.getOriginalPluginOptions(), errors)}`, try { contentTypes = await pagedGet(client, `getContentTypes`, pageLimit) } catch (e) { - reporter.panic( - { - id: CODES.FetchContentTypes, - context: { - sourceMessage: `Error fetching content types: ${createContentfulErrorMessage( - e - )}`, - }, + reporter.panic({ + id: CODES.FetchContentTypes, + context: { + sourceMessage: `Error fetching content types: ${createContentfulErrorMessage( + e + )}`, }, - e - ) + }) } reporter.verbose(`Content types fetched ${contentTypes.items.length}`) From 7f0b22a9ea8800512fa0043c941b63feaacefbe6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Benedikt=20R=C3=B6tsch?= Date: Tue, 16 Mar 2021 15:49:46 +0100 Subject: [PATCH 09/16] style: clean up error message creation function --- .../gatsby-source-contentful/src/fetch.js | 39 ++++++++++++------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/packages/gatsby-source-contentful/src/fetch.js b/packages/gatsby-source-contentful/src/fetch.js index da5078b59f76d..9ce5b03d10a9e 100644 --- a/packages/gatsby-source-contentful/src/fetch.js +++ b/packages/gatsby-source-contentful/src/fetch.js @@ -4,46 +4,56 @@ const chalk = require(`chalk`) const { formatPluginOptionsForCLI } = require(`./plugin-options`) const { CODES } = require(`./report`) +/** + * Generate a user friendly error message. + * + * Contentful's API has its own error message structure, which might change depending of internal server or authentification errors. + * + * Additionally the SDK strips the error object, sometimes: + * https://github.com/contentful/contentful.js/blob/b67b77ac8c919c4ec39203f8cac2043854ab0014/lib/create-contentful-api.js#L89-L99 + * + * This code tries to work around this. + */ 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 we got a response, it is very likely that it is a Contentful API error. if (e.response) { - e = { ...e, ...e.response } + let parsedContentfulErrorData = {} + // Parse JSON response data, and add it to the object. if (typeof e.response.data === `string`) { try { - const data = JSON.parse(e.response.data) - if (data && typeof data === `object`) { - e = { ...e, ...data } + parsedContentfulErrorData = JSON.parse(e.response.data) + if (typeof parsedContentfulErrorData !== `object`) { + throw new Error() } } catch (err) { e.message = `Unable to extract API error data from:\n${e.response.data}` } + // If response data was parsed alredy, just add it. } else if (typeof e.response.data === `object`) { - e = { ...e, ...e.response.data } + parsedContentfulErrorData = e.response.data } + + e = { ...e, ...e.response, ...parsedContentfulErrorData } } let errorMessage = [ - // Generic error responses + // Generic error values e.code && `${e.code}`, e.status && `${e.status}`, e.statusText, - // Contentful API error Responses + // Contentful API error response values e?.sys?.id, ] .filter(Boolean) .join(` `) + // Add message if it exists. Usually error default or Contentful's error message if (e.message) { errorMessage += `\n\n${e.message}` } + // Get request ID from headers or Contentful's error data const requestId = (e.headers && typeof e.headers === `object` && @@ -54,6 +64,7 @@ const createContentfulErrorMessage = e => { errorMessage += `\n\nRequest ID: ${requestId}` } + // Tell the user about how many request attempts Contentful SDK made if (e.attempts) { errorMessage += `\n\nThe request was sent with ${e.attempts} attempts` } From 2d2dc07edd7ae61537afb0451a4ef6ab81ba519b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Benedikt=20R=C3=B6tsch?= Date: Tue, 16 Mar 2021 15:50:38 +0100 Subject: [PATCH 10/16] Update packages/gatsby-source-contentful/src/gatsby-node.js Co-authored-by: Ward Peeters --- packages/gatsby-source-contentful/src/gatsby-node.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/gatsby-source-contentful/src/gatsby-node.js b/packages/gatsby-source-contentful/src/gatsby-node.js index 2e647737698f3..caef04f76a9ef 100644 --- a/packages/gatsby-source-contentful/src/gatsby-node.js +++ b/packages/gatsby-source-contentful/src/gatsby-node.js @@ -140,7 +140,8 @@ List of locales and their codes can be found in Contentful app -> Settings -> Lo Use this with caution, you might override values this plugin does set for you to connect to Contentful.` ) - .default(true), + .unknown(true) + .default({}), // default plugins passed by gatsby plugins: Joi.array(), }) From e013695494597c0af327c1c6e28b940c0b9ed379 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Benedikt=20R=C3=B6tsch?= Date: Mon, 22 Mar 2021 09:39:56 +0100 Subject: [PATCH 11/16] test: revert change to unrelated snapshot --- .../providers/npm/__snapshots__/package.test.js.snap | 10 ---------- 1 file changed, 10 deletions(-) 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 b04bae9391dd4..1386303baa81e 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,16 +66,6 @@ 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", From 4a8ac8d2b8447f4563b57e28fd413619bfecf081 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Benedikt=20R=C3=B6tsch?= Date: Mon, 22 Mar 2021 17:29:54 +0100 Subject: [PATCH 12/16] style: improve code style --- packages/gatsby-source-contentful/src/fetch.js | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/packages/gatsby-source-contentful/src/fetch.js b/packages/gatsby-source-contentful/src/fetch.js index 9ce5b03d10a9e..6ed7904ef05b3 100644 --- a/packages/gatsby-source-contentful/src/fetch.js +++ b/packages/gatsby-source-contentful/src/fetch.js @@ -17,15 +17,12 @@ const { CODES } = require(`./report`) const createContentfulErrorMessage = e => { // If we got a response, it is very likely that it is a Contentful API error. if (e.response) { - let parsedContentfulErrorData = {} + let parsedContentfulErrorData = null // Parse JSON response data, and add it to the object. if (typeof e.response.data === `string`) { try { parsedContentfulErrorData = JSON.parse(e.response.data) - if (typeof parsedContentfulErrorData !== `object`) { - throw new Error() - } } catch (err) { e.message = `Unable to extract API error data from:\n${e.response.data}` } @@ -39,8 +36,8 @@ const createContentfulErrorMessage = e => { let errorMessage = [ // Generic error values - e.code && `${e.code}`, - e.status && `${e.status}`, + e.code && String(e.code), + e.status && String(e.status), e.statusText, // Contentful API error response values e?.sys?.id, From ab66282f608c40d67bc578df3861b8a8a9f24a6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Benedikt=20R=C3=B6tsch?= Date: Mon, 22 Mar 2021 17:42:33 +0100 Subject: [PATCH 13/16] style: simplify optional chaining --- .../gatsby-source-contentful/src/extend-node-type.js | 10 +++++----- packages/gatsby-source-contentful/src/fetch.js | 2 +- packages/gatsby-source-contentful/src/normalize.js | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/gatsby-source-contentful/src/extend-node-type.js b/packages/gatsby-source-contentful/src/extend-node-type.js index 11aaa30e412b4..876050c5283e5 100644 --- a/packages/gatsby-source-contentful/src/extend-node-type.js +++ b/packages/gatsby-source-contentful/src/extend-node-type.js @@ -63,7 +63,7 @@ const CONTENTFUL_IMAGE_MAX_SIZE = 4000 const isImage = image => [`image/jpeg`, `image/jpg`, `image/png`, `image/webp`, `image/gif`].includes( - image?.file?.contentType + image.file?.contentType ) // Note: this may return a Promise, body (sync), or null @@ -494,7 +494,7 @@ const fixedNodeType = ({ name, getTracedSVG }) => { type: GraphQLString, resolve({ image, options }) { if ( - image?.file?.contentType === `image/webp` || + image.file?.contentType === `image/webp` || options.toFormat === `webp` ) { return null @@ -511,7 +511,7 @@ const fixedNodeType = ({ name, getTracedSVG }) => { type: GraphQLString, resolve({ image, options }) { if ( - image?.file?.contentType === `image/webp` || + image.file?.contentType === `image/webp` || options.toFormat === `webp` ) { return null @@ -587,7 +587,7 @@ const fluidNodeType = ({ name, getTracedSVG }) => { type: GraphQLString, resolve({ image, options }) { if ( - image?.file?.contentType === `image/webp` || + image.file?.contentType === `image/webp` || options.toFormat === `webp` ) { return null @@ -604,7 +604,7 @@ const fluidNodeType = ({ name, getTracedSVG }) => { type: GraphQLString, resolve({ image, options }) { if ( - image?.file?.contentType === `image/webp` || + image.file?.contentType === `image/webp` || options.toFormat === `webp` ) { return null diff --git a/packages/gatsby-source-contentful/src/fetch.js b/packages/gatsby-source-contentful/src/fetch.js index 6ed7904ef05b3..168dd3f043896 100644 --- a/packages/gatsby-source-contentful/src/fetch.js +++ b/packages/gatsby-source-contentful/src/fetch.js @@ -40,7 +40,7 @@ const createContentfulErrorMessage = e => { e.status && String(e.status), e.statusText, // Contentful API error response values - e?.sys?.id, + e.sys?.id, ] .filter(Boolean) .join(` `) diff --git a/packages/gatsby-source-contentful/src/normalize.js b/packages/gatsby-source-contentful/src/normalize.js index fde42f7507e72..5d664bc5b0b8c 100644 --- a/packages/gatsby-source-contentful/src/normalize.js +++ b/packages/gatsby-source-contentful/src/normalize.js @@ -151,7 +151,7 @@ exports.buildForeignReferenceMap = ({ }) } } else if ( - entryItemFieldValue?.sys?.type && + entryItemFieldValue.sys?.type && entryItemFieldValue.sys.id ) { const key = `${entryItemFieldValue.sys.id}___${ From 5a289ff9e93e016867b8c418ca322cba6b49ab44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Benedikt=20R=C3=B6tsch?= Date: Mon, 22 Mar 2021 17:43:52 +0100 Subject: [PATCH 14/16] refactor(contentful): catch string only error responses and simplify --- packages/gatsby-source-contentful/src/fetch.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/gatsby-source-contentful/src/fetch.js b/packages/gatsby-source-contentful/src/fetch.js index 168dd3f043896..3e09e4ce23243 100644 --- a/packages/gatsby-source-contentful/src/fetch.js +++ b/packages/gatsby-source-contentful/src/fetch.js @@ -15,6 +15,9 @@ const { CODES } = require(`./report`) * This code tries to work around this. */ const createContentfulErrorMessage = e => { + if (typeof e === `string`) { + return e + } // If we got a response, it is very likely that it is a Contentful API error. if (e.response) { let parsedContentfulErrorData = null @@ -24,9 +27,9 @@ const createContentfulErrorMessage = e => { try { parsedContentfulErrorData = JSON.parse(e.response.data) } catch (err) { - e.message = `Unable to extract API error data from:\n${e.response.data}` + e.message = e.response.data } - // If response data was parsed alredy, just add it. + // If response data was parsed already, just add it. } else if (typeof e.response.data === `object`) { parsedContentfulErrorData = e.response.data } From 8dca766fcf4257c7fca737cdbe8b2c502ca8ea92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Benedikt=20R=C3=B6tsch?= Date: Tue, 23 Mar 2021 15:16:37 +0100 Subject: [PATCH 15/16] style: optional chaning is required for images --- .../gatsby-source-contentful/src/extend-node-type.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/gatsby-source-contentful/src/extend-node-type.js b/packages/gatsby-source-contentful/src/extend-node-type.js index 876050c5283e5..11aaa30e412b4 100644 --- a/packages/gatsby-source-contentful/src/extend-node-type.js +++ b/packages/gatsby-source-contentful/src/extend-node-type.js @@ -63,7 +63,7 @@ const CONTENTFUL_IMAGE_MAX_SIZE = 4000 const isImage = image => [`image/jpeg`, `image/jpg`, `image/png`, `image/webp`, `image/gif`].includes( - image.file?.contentType + image?.file?.contentType ) // Note: this may return a Promise, body (sync), or null @@ -494,7 +494,7 @@ const fixedNodeType = ({ name, getTracedSVG }) => { type: GraphQLString, resolve({ image, options }) { if ( - image.file?.contentType === `image/webp` || + image?.file?.contentType === `image/webp` || options.toFormat === `webp` ) { return null @@ -511,7 +511,7 @@ const fixedNodeType = ({ name, getTracedSVG }) => { type: GraphQLString, resolve({ image, options }) { if ( - image.file?.contentType === `image/webp` || + image?.file?.contentType === `image/webp` || options.toFormat === `webp` ) { return null @@ -587,7 +587,7 @@ const fluidNodeType = ({ name, getTracedSVG }) => { type: GraphQLString, resolve({ image, options }) { if ( - image.file?.contentType === `image/webp` || + image?.file?.contentType === `image/webp` || options.toFormat === `webp` ) { return null @@ -604,7 +604,7 @@ const fluidNodeType = ({ name, getTracedSVG }) => { type: GraphQLString, resolve({ image, options }) { if ( - image.file?.contentType === `image/webp` || + image?.file?.contentType === `image/webp` || options.toFormat === `webp` ) { return null From d4716d5a31e380622039aa91052d1888c7e86d13 Mon Sep 17 00:00:00 2001 From: Ward Peeters Date: Tue, 23 Mar 2021 15:24:49 +0100 Subject: [PATCH 16/16] revert normalize changes --- packages/gatsby-source-contentful/src/normalize.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/gatsby-source-contentful/src/normalize.js b/packages/gatsby-source-contentful/src/normalize.js index 5d664bc5b0b8c..fde42f7507e72 100644 --- a/packages/gatsby-source-contentful/src/normalize.js +++ b/packages/gatsby-source-contentful/src/normalize.js @@ -151,7 +151,7 @@ exports.buildForeignReferenceMap = ({ }) } } else if ( - entryItemFieldValue.sys?.type && + entryItemFieldValue?.sys?.type && entryItemFieldValue.sys.id ) { const key = `${entryItemFieldValue.sys.id}___${