-
Notifications
You must be signed in to change notification settings - Fork 10.3k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix(gatsby): get-page-data should timeout gracefully (#28131)
* fix(gatsby): get-page-data should timeout gracefully * lower timeout interval, add retries, make timeout configurable, add more tests * Update packages/gatsby/src/utils/__tests__/get-page-data.ts Co-authored-by: Vladimir Razuvaev <vladimir.razuvaev@gmail.com> Co-authored-by: Michal Piechowiak <misiek.piechowiak@gmail.com>
- Loading branch information
Showing
2 changed files
with
392 additions
and
13 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,334 @@ | ||
import { store } from "../../redux" | ||
import { getPageData, RETRY_INTERVAL } from "../get-page-data" | ||
import { fixedPagePath } from "../page-data" | ||
import { | ||
IGatsbyPage, | ||
IGatsbyPlugin, | ||
IQueryStartAction, | ||
} from "../../redux/types" | ||
import { pageQueryRun, queryStart } from "../../redux/actions/internal" | ||
import { join as pathJoin } from "path" | ||
|
||
let MOCK_FILE_INFO = {} | ||
|
||
jest.mock(`fs-extra`, () => { | ||
return { | ||
readFile: jest.fn(path => { | ||
if (MOCK_FILE_INFO[path]) { | ||
return MOCK_FILE_INFO[path] | ||
} | ||
throw new Error(`Cannot read file "${path}"`) | ||
}), | ||
} | ||
}) | ||
|
||
describe(`get-page-data-util`, () => { | ||
const pageDataContent = { | ||
result: { | ||
foo: `bar`, | ||
}, | ||
} | ||
|
||
const pageDataStaleContent = { | ||
result: { | ||
foo: `I'm stale`, | ||
}, | ||
} | ||
|
||
let Pages | ||
beforeAll(() => { | ||
Pages = { | ||
foo: { | ||
path: `/foo`, | ||
componentPath: `/foo.js`, | ||
component: `/foo.js`, | ||
}, | ||
} | ||
|
||
store.dispatch({ | ||
type: `SET_PROGRAM`, | ||
payload: { | ||
directory: __dirname, | ||
}, | ||
}) | ||
}) | ||
|
||
beforeEach(() => { | ||
store.dispatch({ | ||
type: `DELETE_CACHE`, | ||
}) | ||
deletePageDataFilesFromFs() | ||
}) | ||
|
||
it(`Resolves immediately if query result is up to date`, async () => { | ||
// query did already run before | ||
createPage(Pages.foo) | ||
startPageQuery(Pages.foo) | ||
finishQuery(Pages.foo) | ||
writePageDataFileToFs(Pages.foo, pageDataContent) | ||
|
||
const resultPromise = getPageData(Pages.foo.path) | ||
expect(await resultPromise).toEqual(pageDataContent) | ||
}) | ||
|
||
it(`Waits for query to run and resolve properly`, async () => { | ||
createPage(Pages.foo) | ||
const resultPromise = getPageData(Pages.foo.path) | ||
|
||
startPageQuery(Pages.foo) | ||
finishQuery(Pages.foo) | ||
writePageDataFileToFs(Pages.foo, pageDataContent) | ||
|
||
expect(await resultPromise).toEqual(pageDataContent) | ||
}) | ||
|
||
describe(`timeouts and retries`, () => { | ||
it(`it times out eventually (default timeout)`, async () => { | ||
jest.useFakeTimers() | ||
|
||
createPage(Pages.foo) | ||
const resultPromise = getPageData(Pages.foo.path) | ||
|
||
jest.runAllTimers() | ||
|
||
expect(setTimeout).toHaveBeenCalledTimes(3) | ||
expect(setTimeout).toHaveBeenNthCalledWith( | ||
1, | ||
expect.any(Function), | ||
RETRY_INTERVAL | ||
) | ||
expect(setTimeout).toHaveBeenNthCalledWith( | ||
2, | ||
expect.any(Function), | ||
RETRY_INTERVAL | ||
) | ||
expect(setTimeout).toHaveBeenNthCalledWith( | ||
3, | ||
expect.any(Function), | ||
RETRY_INTERVAL | ||
) | ||
|
||
await expect(resultPromise).rejects.toMatchInlineSnapshot( | ||
`[Error: Couldn't get query results for "/foo" in 15.000s.]` | ||
) | ||
}) | ||
|
||
it(`it times out eventually (7 second timeout - 5s + 2s)`, async () => { | ||
jest.useFakeTimers() | ||
|
||
createPage(Pages.foo) | ||
const resultPromise = getPageData(Pages.foo.path, 7000) | ||
|
||
jest.runAllTimers() | ||
|
||
expect(setTimeout).toHaveBeenCalledTimes(2) | ||
expect(setTimeout).toHaveBeenNthCalledWith( | ||
1, | ||
expect.any(Function), | ||
RETRY_INTERVAL | ||
) | ||
expect(setTimeout).toHaveBeenNthCalledWith(2, expect.any(Function), 2000) | ||
|
||
await expect(resultPromise).rejects.toMatchInlineSnapshot( | ||
`[Error: Couldn't get query results for "/foo" in 7.000s.]` | ||
) | ||
}) | ||
|
||
it(`Can resolve after retry`, async () => { | ||
jest.useFakeTimers() | ||
|
||
expect(clearTimeout).toHaveBeenCalledTimes(0) | ||
|
||
createPage(Pages.foo) | ||
const resultPromise = getPageData(Pages.foo.path) | ||
startPageQuery(Pages.foo) | ||
|
||
jest.runOnlyPendingTimers() | ||
|
||
// we don't resolve in first timeout and we set timeout for second one | ||
expect(setTimeout).toHaveBeenCalledTimes(2) | ||
expect(setTimeout).toHaveBeenNthCalledWith( | ||
1, | ||
expect.any(Function), | ||
RETRY_INTERVAL | ||
) | ||
expect(setTimeout).toHaveBeenNthCalledWith( | ||
1, | ||
expect.any(Function), | ||
RETRY_INTERVAL | ||
) | ||
|
||
finishQuery(Pages.foo) | ||
writePageDataFileToFs(Pages.foo, pageDataContent) | ||
|
||
// we cancel second timeout | ||
expect(clearTimeout).toHaveBeenCalledTimes(1) | ||
|
||
// and result are correct | ||
expect(await resultPromise).toEqual(pageDataContent) | ||
}) | ||
|
||
it(`Can fallback to stale page-data if it exists (better to potentially unblock user to start doing some work than fail completely)`, async () => { | ||
jest.useFakeTimers() | ||
|
||
writePageDataFileToFs(Pages.foo, pageDataStaleContent) | ||
|
||
createPage(Pages.foo) | ||
const resultPromise = getPageData(Pages.foo.path) | ||
|
||
jest.runAllTimers() | ||
|
||
expect(setTimeout).toHaveBeenCalledTimes(3) | ||
expect(setTimeout).toHaveBeenNthCalledWith( | ||
1, | ||
expect.any(Function), | ||
RETRY_INTERVAL | ||
) | ||
expect(setTimeout).toHaveBeenNthCalledWith( | ||
2, | ||
expect.any(Function), | ||
RETRY_INTERVAL | ||
) | ||
expect(setTimeout).toHaveBeenNthCalledWith( | ||
3, | ||
expect.any(Function), | ||
RETRY_INTERVAL | ||
) | ||
|
||
// we didn't get fresh results, but we resolved to stale ones | ||
expect(await resultPromise).toEqual(pageDataStaleContent) | ||
}) | ||
}) | ||
|
||
describe(`handles page deletion in the middle of execution gracefully`, () => { | ||
describe.each([ | ||
// both variants should report that page was deleted | ||
[ | ||
`Doesn't have stale page-data file for a page`, | ||
{ hasPreviousResults: false }, | ||
], | ||
[`Has stale page-data file for a page`, { hasPreviousResults: true }], | ||
])(`%s`, (_title, { hasPreviousResults }) => { | ||
beforeEach(() => { | ||
if (hasPreviousResults) { | ||
writePageDataFileToFs(Pages.foo, pageDataStaleContent) | ||
} | ||
}) | ||
|
||
it(`page is deleted before we start query running`, async () => { | ||
jest.useFakeTimers() | ||
|
||
createPage(Pages.foo) | ||
const resultPromise = getPageData(Pages.foo.path) | ||
|
||
deletePage(Pages.foo) | ||
|
||
jest.runAllTimers() | ||
|
||
await expect(resultPromise).rejects.toMatchInlineSnapshot( | ||
`[Error: Page "/foo" doesn't exist. It might have been deleted recently.]` | ||
) | ||
}) | ||
|
||
it(`page is deleted after we start query running`, async () => { | ||
jest.useFakeTimers() | ||
|
||
createPage(Pages.foo) | ||
const resultPromise = getPageData(Pages.foo.path) | ||
startPageQuery(Pages.foo) | ||
deletePage(Pages.foo) | ||
finishQuery(Pages.foo) | ||
|
||
jest.runAllTimers() | ||
|
||
await expect(resultPromise).rejects.toMatchInlineSnapshot( | ||
`[Error: Page "/foo" doesn't exist. It might have been deleted recently.]` | ||
) | ||
}) | ||
|
||
it(`page is deleted before we flush page data`, async () => { | ||
jest.useFakeTimers() | ||
|
||
createPage(Pages.foo) | ||
const resultPromise = getPageData(Pages.foo.path) | ||
startPageQuery(Pages.foo) | ||
finishQuery(Pages.foo) | ||
deletePage(Pages.foo) | ||
|
||
jest.runAllTimers() | ||
|
||
await expect(resultPromise).rejects.toMatchInlineSnapshot( | ||
`[Error: Page "/foo" doesn't exist. It might have been deleted recently.]` | ||
) | ||
}) | ||
}) | ||
}) | ||
}) | ||
|
||
function createPage(page: Partial<IGatsbyPage>): void { | ||
store.dispatch({ | ||
type: `CREATE_PAGE`, | ||
payload: { ...page, context: {} }, | ||
plugin: { name: `get-page-data-test` }, | ||
}) | ||
} | ||
|
||
function deletePage(page: Partial<IGatsbyPage>): void { | ||
store.dispatch({ | ||
type: `DELETE_PAGE`, | ||
payload: page, | ||
plugin: { name: `get-page-data-test` }, | ||
}) | ||
} | ||
|
||
function startPageQuery(page: Partial<IGatsbyPage>): void { | ||
const payload: IQueryStartAction["payload"] = { | ||
path: page.path!, | ||
componentPath: page.componentPath!, | ||
isPage: true, | ||
} | ||
store.dispatch( | ||
queryStart(payload, { name: `page-data-test` } as IGatsbyPlugin) | ||
) | ||
} | ||
|
||
function finishQuery(page: Partial<IGatsbyPage>): void { | ||
const payload: IQueryStartAction["payload"] = { | ||
path: page.path!, | ||
componentPath: page.componentPath!, | ||
isPage: true, | ||
} | ||
store.dispatch( | ||
pageQueryRun(payload, { name: `page-data-test` } as IGatsbyPlugin) | ||
) | ||
|
||
store.dispatch({ | ||
type: `ADD_PENDING_PAGE_DATA_WRITE`, | ||
payload: { | ||
path: page.path, | ||
}, | ||
}) | ||
} | ||
|
||
function deletePageDataFilesFromFs(): void { | ||
MOCK_FILE_INFO = {} | ||
} | ||
|
||
function writePageDataFileToFs( | ||
page: Partial<IGatsbyPage>, | ||
jsonObject: object | ||
): void { | ||
MOCK_FILE_INFO[ | ||
pathJoin( | ||
__dirname, | ||
`public`, | ||
`page-data`, | ||
fixedPagePath(page.path!), | ||
`page-data.json` | ||
) | ||
] = JSON.stringify(jsonObject) | ||
store.dispatch({ | ||
type: `CLEAR_PENDING_PAGE_DATA_WRITE`, | ||
payload: { page: page.path }, | ||
}) | ||
} |
Oops, something went wrong.