Skip to content

Commit

Permalink
fix: pretty print Contentful errors after all retry attempts failed
Browse files Browse the repository at this point in the history
  • Loading branch information
axe312ger committed Mar 16, 2021
1 parent 227754c commit 8946b74
Show file tree
Hide file tree
Showing 3 changed files with 290 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
@@ -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()
})
})
90 changes: 61 additions & 29 deletions packages/gatsby-source-contentful/src/fetch.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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`] &&
Expand All @@ -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)
}

Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down

0 comments on commit 8946b74

Please sign in to comment.