fix(gatsby-source-contentful): Improve network error handling (#30257)

Co-authored-by: Ward Peeters <ward@coding-tech.com>
gatsbyjs · Mar 29, 2021 · c1ac5e4 · c1ac5e4
1 parent f0751d8
commit c1ac5e4
Show file tree

Hide file tree

Showing 4 changed files with 344 additions and 58 deletions.
diff --git a/packages/gatsby-source-contentful/README.md b/packages/gatsby-source-contentful/README.md
@@ -114,6 +114,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 does set 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/__tests__/fetch-network-errors.js 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()
+  })
+})