From e72f5db47ee599732fd931b05a1b8ea6f35be471 Mon Sep 17 00:00:00 2001 From: Matt Kane Date: Fri, 24 Jul 2020 09:38:54 +0100 Subject: [PATCH 1/5] feat(gatsby): Add top-level error handling to state machine --- .../src/state-machines/develop/actions.ts | 16 ++++++++++ .../src/state-machines/develop/index.ts | 31 +++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/packages/gatsby/src/state-machines/develop/actions.ts b/packages/gatsby/src/state-machines/develop/actions.ts index 56fc89edb4ae3..fd278117881e5 100644 --- a/packages/gatsby/src/state-machines/develop/actions.ts +++ b/packages/gatsby/src/state-machines/develop/actions.ts @@ -129,6 +129,20 @@ export const finishParentSpan = ({ parentSpan }: IBuildContext): void => export const saveDbState = (): Promise => saveState() +export const logError: ActionFunction = ( + _context, + event +) => { + reporter.error(event.data) +} + +export const panic: ActionFunction = ( + _context, + event +) => { + reporter.panic(event.data) +} + /** * Event handler used in all states where we're not ready to process a file change * Instead we add it to a batch to process when we're next idle @@ -158,4 +172,6 @@ export const buildActions: ActionFunctionMap = { markSourceFilesClean, saveDbState, setQueryRunningFinished, + panic, + logError, } diff --git a/packages/gatsby/src/state-machines/develop/index.ts b/packages/gatsby/src/state-machines/develop/index.ts index 780451562e3f5..62e690ac2f496 100644 --- a/packages/gatsby/src/state-machines/develop/index.ts +++ b/packages/gatsby/src/state-machines/develop/index.ts @@ -45,6 +45,9 @@ const developConfig: MachineConfig = { target: `initializingData`, actions: [`assignStoreAndWorkerPool`, `spawnMutationListener`], }, + onError: { + actions: `panic`, + }, }, }, // Sourcing nodes, customising and inferring schema, then running createPages @@ -77,6 +80,10 @@ const developConfig: MachineConfig = { ], target: `runningQueries`, }, + onError: { + actions: `logError`, + target: `waiting`, + }, }, }, // Running page and static queries and generating the SSRed HTML and page data @@ -126,6 +133,10 @@ const developConfig: MachineConfig = { target: `waiting`, }, ], + onError: { + actions: `logError`, + target: `waiting`, + }, }, }, // Recompile the JS bundle @@ -136,6 +147,10 @@ const developConfig: MachineConfig = { actions: `markSourceFilesClean`, target: `waiting`, }, + onError: { + actions: `logError`, + target: `waiting`, + }, }, }, // Spin up webpack and socket.io @@ -150,6 +165,10 @@ const developConfig: MachineConfig = { `markSourceFilesClean`, ], }, + onError: { + actions: `panic`, + target: `waiting`, + }, }, }, // Idle, waiting for events that make us rebuild @@ -183,6 +202,10 @@ const developConfig: MachineConfig = { actions: `assignServiceResult`, target: `recreatingPages`, }, + onError: { + actions: `panic`, + target: `waiting`, + }, }, }, // Almost the same as initializing data, but skips various first-run stuff @@ -217,6 +240,10 @@ const developConfig: MachineConfig = { ], target: `runningQueries`, }, + onError: { + actions: `logError`, + target: `waiting`, + }, }, }, // Rebuild pages if a node has been mutated outside of sourceNodes @@ -230,6 +257,10 @@ const developConfig: MachineConfig = { actions: `assignServiceResult`, target: `runningQueries`, }, + onError: { + actions: `logError`, + target: `waiting`, + }, }, }, }, From 083fc17e365f7884634090c1b7c872b78ecac837 Mon Sep 17 00:00:00 2001 From: Matt Kane Date: Fri, 24 Jul 2020 17:40:33 +0100 Subject: [PATCH 2/5] Add initial tests --- .../src/state-machines/__tests__/develop.ts | 46 +++++++++++++++++++ .../src/state-machines/develop/index.ts | 1 + 2 files changed, 47 insertions(+) create mode 100644 packages/gatsby/src/state-machines/__tests__/develop.ts diff --git a/packages/gatsby/src/state-machines/__tests__/develop.ts b/packages/gatsby/src/state-machines/__tests__/develop.ts new file mode 100644 index 0000000000000..0d12bd14ef7a4 --- /dev/null +++ b/packages/gatsby/src/state-machines/__tests__/develop.ts @@ -0,0 +1,46 @@ +import { developMachine } from "../develop" +import { interpret } from "xstate" +import { IProgram } from "../../commands/types" + +const actions = { + assignStoreAndWorkerPool: jest.fn(), + markNodesDirty: jest.fn(), + callApi: jest.fn(), +} + +const services = { + initialize: jest.fn(), + initializeData: jest.fn(), +} + +const machine = developMachine.withConfig( + { + actions, + services, + }, + { + program: {} as IProgram, + } +) + +describe(`the develop state machine`, () => { + it(`initialises`, async () => { + const payload = { foo: 1 } + const service = interpret(machine) + const transitionWatcher = jest.fn() + service.onTransition(transitionWatcher) + service.start() + expect(service.state.value).toBe(`initializing`) + service.send(`done.invoke.initialize`) + expect(service.state.value).toBe(`initializingData`) + expect(actions.assignStoreAndWorkerPool).toHaveBeenCalled() + service.send(`ADD_NODE_MUTATION`, payload) + expect(actions.callApi).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ type: `ADD_NODE_MUTATION`, ...payload }), + expect.anything() + ) + + expect(transitionWatcher).toHaveBeenCalledWith({}) + }) +}) diff --git a/packages/gatsby/src/state-machines/develop/index.ts b/packages/gatsby/src/state-machines/develop/index.ts index 780451562e3f5..7df112435f152 100644 --- a/packages/gatsby/src/state-machines/develop/index.ts +++ b/packages/gatsby/src/state-machines/develop/index.ts @@ -40,6 +40,7 @@ const developConfig: MachineConfig = { WEBHOOK_RECEIVED: undefined, }, invoke: { + id: `initialize`, src: `initialize`, onDone: { target: `initializingData`, From c4ee7a1269d4964867792d4d973bbeab05cf48c2 Mon Sep 17 00:00:00 2001 From: Matt Kane Date: Mon, 27 Jul 2020 16:14:41 +0100 Subject: [PATCH 3/5] Add tests for top-level machine --- .../src/state-machines/__tests__/develop.ts | 141 +++++++++++++++++- .../src/state-machines/develop/index.ts | 1 + 2 files changed, 136 insertions(+), 6 deletions(-) diff --git a/packages/gatsby/src/state-machines/__tests__/develop.ts b/packages/gatsby/src/state-machines/__tests__/develop.ts index 0d12bd14ef7a4..9624172d58451 100644 --- a/packages/gatsby/src/state-machines/__tests__/develop.ts +++ b/packages/gatsby/src/state-machines/__tests__/develop.ts @@ -1,16 +1,25 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ import { developMachine } from "../develop" import { interpret } from "xstate" import { IProgram } from "../../commands/types" const actions = { assignStoreAndWorkerPool: jest.fn(), - markNodesDirty: jest.fn(), + assignServiceResult: jest.fn(), callApi: jest.fn(), + finishParentSpan: jest.fn(), + saveDbState: jest.fn(), } const services = { initialize: jest.fn(), initializeData: jest.fn(), + reloadData: jest.fn(), + runQueries: jest.fn(), + startWebpackServer: jest.fn(), + recompile: jest.fn(), + waitForMutations: jest.fn(), + recreatePages: jest.fn(), } const machine = developMachine.withConfig( @@ -23,24 +32,144 @@ const machine = developMachine.withConfig( } ) +const resetMocks = (mocks: Record): void => + Object.values(mocks).forEach(mock => mock.mockReset()) + +const resetAllMocks = (): void => { + resetMocks(services) + resetMocks(actions) +} + describe(`the develop state machine`, () => { + beforeEach(() => { + resetAllMocks() + }) + it(`initialises`, async () => { - const payload = { foo: 1 } const service = interpret(machine) - const transitionWatcher = jest.fn() - service.onTransition(transitionWatcher) service.start() expect(service.state.value).toBe(`initializing`) + }) + + it(`runs node mutation during initialising data state`, () => { + const payload = { foo: 1 } + const service = interpret(machine) + + service.start() service.send(`done.invoke.initialize`) expect(service.state.value).toBe(`initializingData`) - expect(actions.assignStoreAndWorkerPool).toHaveBeenCalled() service.send(`ADD_NODE_MUTATION`, payload) expect(actions.callApi).toHaveBeenCalledWith( expect.anything(), expect.objectContaining({ type: `ADD_NODE_MUTATION`, ...payload }), expect.anything() ) + expect(service.state.context.nodesMutatedDuringQueryRun).toBeTruthy() + }) + + it(`marks source file as dirty during node sourcing`, () => { + const service = interpret(machine) + + service.start() + expect(service.state.value).toBe(`initializing`) + service.send(`done.invoke.initialize`) + expect(service.state.value).toBe(`initializingData`) + expect(service.state.context.sourceFilesDirty).toBeFalsy() + service.send(`SOURCE_FILE_CHANGED`) + expect(service.state.context.sourceFilesDirty).toBeTruthy() + }) - expect(transitionWatcher).toHaveBeenCalledWith({}) + // This is current behaviour, but it will be queued in future + it(`handles a webhook during node sourcing`, () => { + const webhookBody = { foo: 1 } + const service = interpret(machine) + service.start() + expect(service.state.value).toBe(`initializing`) + service.send(`done.invoke.initialize`) + expect(service.state.value).toBe(`initializingData`) + expect(service.state.context.webhookBody).toBeUndefined() + service.send(`WEBHOOK_RECEIVED`, { payload: { webhookBody } }) + expect(service.state.context.webhookBody).toEqual(webhookBody) + expect(services.reloadData).toHaveBeenCalled() + }) + + it(`queues a node mutation during query running`, () => { + const payload = { foo: 1 } + + const service = interpret(machine) + service.start() + service.send(`done.invoke.initialize`) + service.send(`done.invoke.initialize-data`) + expect(service.state.context.nodeMutationBatch).toBeUndefined() + service.send(`ADD_NODE_MUTATION`, { payload }) + expect(service.state.context.nodeMutationBatch).toEqual( + expect.arrayContaining([payload]) + ) + }) + + it(`starts webpack if there is no compiler`, () => { + const service = interpret(machine) + service.start() + service.send(`done.invoke.initialize`) + service.send(`done.invoke.initialize-data`) + expect(service.state.context.compiler).toBeUndefined() + services.startWebpackServer.mockReset() + service.send(`done.invoke.run-queries`) + expect(services.startWebpackServer).toHaveBeenCalled() + }) + + it(`recompiles if source files have changed`, () => { + const service = interpret(machine) + service.start() + service.send(`done.invoke.initialize`) + service.send(`SOURCE_FILE_CHANGED`) + + service.send(`done.invoke.initialize-data`) + // So we don't start webpack instead + service.state.context.compiler = {} as any + services.recompile.mockReset() + service.send(`done.invoke.run-queries`) + expect(services.startWebpackServer).not.toHaveBeenCalled() + expect(services.recompile).toHaveBeenCalled() + }) + + it(`skips compilation if source files are unchanged`, () => { + const service = interpret(machine) + service.start() + service.send(`done.invoke.initialize`) + service.send(`done.invoke.initialize-data`) + service.state.context.compiler = {} as any + services.recompile.mockReset() + service.send(`done.invoke.run-queries`) + expect(services.startWebpackServer).not.toHaveBeenCalled() + expect(services.recompile).not.toHaveBeenCalled() + }) + + it(`recreates pages when waiting is complete`, () => { + const service = interpret(machine) + service.start() + service.send(`done.invoke.initialize`) + service.send(`done.invoke.initialize-data`) + service.state.context.compiler = {} as any + service.send(`done.invoke.run-queries`) + service.send(`done.invoke.waiting`) + + expect(services.recreatePages).toHaveBeenCalled() + }) + + it(`extracts queries when waiting requests it`, () => { + const service = interpret(machine) + service.start() + service.send(`done.invoke.initialize`) + service.send(`done.invoke.initialize-data`) + service.state.context.compiler = {} as any + service.send(`done.invoke.run-queries`) + service.send(`EXTRACT_QUERIES_NOW`) + expect(services.runQueries).toHaveBeenCalled() }) }) + +// const transitionWatcher = jest.fn() +// service.onTransition(transitionWatcher) + +// expect(transitionWatcher).toHaveBeenCalledWith({}) diff --git a/packages/gatsby/src/state-machines/develop/index.ts b/packages/gatsby/src/state-machines/develop/index.ts index 7df112435f152..c112ed887edea 100644 --- a/packages/gatsby/src/state-machines/develop/index.ts +++ b/packages/gatsby/src/state-machines/develop/index.ts @@ -57,6 +57,7 @@ const developConfig: MachineConfig = { }, }, invoke: { + id: `initialize-data`, src: `initializeData`, data: ({ parentSpan, From 023280fb37a96bf533bfbf89623597c881129ea3 Mon Sep 17 00:00:00 2001 From: Matt Kane Date: Mon, 27 Jul 2020 17:10:31 +0100 Subject: [PATCH 4/5] Test error handling --- .../src/state-machines/__tests__/develop.ts | 89 +++++++++++++++++-- .../src/state-machines/develop/index.ts | 1 - 2 files changed, 84 insertions(+), 6 deletions(-) diff --git a/packages/gatsby/src/state-machines/__tests__/develop.ts b/packages/gatsby/src/state-machines/__tests__/develop.ts index 9624172d58451..56102ef18bb25 100644 --- a/packages/gatsby/src/state-machines/__tests__/develop.ts +++ b/packages/gatsby/src/state-machines/__tests__/develop.ts @@ -9,6 +9,8 @@ const actions = { callApi: jest.fn(), finishParentSpan: jest.fn(), saveDbState: jest.fn(), + logError: jest.fn(), + panic: jest.fn(), } const services = { @@ -22,6 +24,12 @@ const services = { recreatePages: jest.fn(), } +const throwService = async (): Promise => { + throw new Error(`fail`) +} + +const rejectService = async (): Promise => Promise.reject(`fail`) + const machine = developMachine.withConfig( { actions, @@ -32,6 +40,8 @@ const machine = developMachine.withConfig( } ) +const tick = (): Promise => new Promise(resolve => setTimeout(resolve, 0)) + const resetMocks = (mocks: Record): void => Object.values(mocks).forEach(mock => mock.mockReset()) @@ -40,7 +50,7 @@ const resetAllMocks = (): void => { resetMocks(actions) } -describe(`the develop state machine`, () => { +describe(`the top-level develop state machine`, () => { beforeEach(() => { resetAllMocks() }) @@ -167,9 +177,78 @@ describe(`the develop state machine`, () => { service.send(`EXTRACT_QUERIES_NOW`) expect(services.runQueries).toHaveBeenCalled() }) -}) -// const transitionWatcher = jest.fn() -// service.onTransition(transitionWatcher) + it(`panics on error during initialisation`, async () => { + const service = interpret(machine) + services.initialize.mockImplementationOnce(throwService) + service.start() + await tick() + expect(actions.panic).toHaveBeenCalled() + }) + + it(`panics on rejection during initialisation`, async () => { + const service = interpret(machine) + services.initialize.mockImplementationOnce(rejectService) + service.start() + await tick() + expect(actions.panic).toHaveBeenCalled() + }) + + it(`logs errors during sourcing and transitions to waiting`, async () => { + const service = interpret(machine) + services.initializeData.mockImplementationOnce(throwService) + service.start() + service.send(`done.invoke.initialize`) + await tick() + expect(actions.logError).toHaveBeenCalled() + expect(service.state.value).toEqual(`waiting`) + }) + + it(`logs errors during query running and transitions to waiting`, async () => { + const service = interpret(machine) + services.runQueries.mockImplementationOnce(throwService) + service.start() + service.send(`done.invoke.initialize`) + service.send(`done.invoke.initialize-data`) + await tick() + expect(actions.logError).toHaveBeenCalled() + expect(service.state.value).toEqual(`waiting`) + }) + + it(`panics on errors when launching webpack`, async () => { + const service = interpret(machine) + services.startWebpackServer.mockImplementationOnce(throwService) + service.start() + service.send(`done.invoke.initialize`) + service.send(`done.invoke.initialize-data`) + service.send(`done.invoke.run-queries`) + await tick() + expect(actions.panic).toHaveBeenCalled() + }) -// expect(transitionWatcher).toHaveBeenCalledWith({}) + it(`logs errors during compilation and transitions to waiting`, async () => { + const service = interpret(machine) + services.recompile.mockImplementationOnce(throwService) + service.start() + service.send(`done.invoke.initialize`) + service.send(`done.invoke.initialize-data`) + service.state.context.compiler = {} as any + service.state.context.sourceFilesDirty = true + service.send(`done.invoke.run-queries`) + await tick() + expect(actions.logError).toHaveBeenCalled() + expect(service.state.value).toEqual(`waiting`) + }) + + it(`panics on errors while waiting`, async () => { + const service = interpret(machine) + services.waitForMutations.mockImplementationOnce(throwService) + service.start() + service.send(`done.invoke.initialize`) + service.send(`done.invoke.initialize-data`) + service.state.context.compiler = {} as any + service.send(`done.invoke.run-queries`) + await tick() + expect(actions.panic).toHaveBeenCalled() + }) +}) diff --git a/packages/gatsby/src/state-machines/develop/index.ts b/packages/gatsby/src/state-machines/develop/index.ts index 5a918720fe10b..98f94914e4734 100644 --- a/packages/gatsby/src/state-machines/develop/index.ts +++ b/packages/gatsby/src/state-machines/develop/index.ts @@ -206,7 +206,6 @@ const developConfig: MachineConfig = { }, onError: { actions: `panic`, - target: `waiting`, }, }, }, From ccc99e1a3467f478a39144d8ab79a2abe32889b6 Mon Sep 17 00:00:00 2001 From: Matt Kane Date: Tue, 28 Jul 2020 09:32:26 +0100 Subject: [PATCH 5/5] Add post-bootstrap to tests --- .../gatsby/src/state-machines/__tests__/develop.ts | 11 ++++++++++- packages/gatsby/src/state-machines/develop/index.ts | 1 + 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/gatsby/src/state-machines/__tests__/develop.ts b/packages/gatsby/src/state-machines/__tests__/develop.ts index 56102ef18bb25..fd842ae79ecc5 100644 --- a/packages/gatsby/src/state-machines/__tests__/develop.ts +++ b/packages/gatsby/src/state-machines/__tests__/develop.ts @@ -110,6 +110,7 @@ describe(`the top-level develop state machine`, () => { service.start() service.send(`done.invoke.initialize`) service.send(`done.invoke.initialize-data`) + service.send(`done.invoke.post-bootstrap`) expect(service.state.context.nodeMutationBatch).toBeUndefined() service.send(`ADD_NODE_MUTATION`, { payload }) expect(service.state.context.nodeMutationBatch).toEqual( @@ -122,6 +123,7 @@ describe(`the top-level develop state machine`, () => { service.start() service.send(`done.invoke.initialize`) service.send(`done.invoke.initialize-data`) + service.send(`done.invoke.post-bootstrap`) expect(service.state.context.compiler).toBeUndefined() services.startWebpackServer.mockReset() service.send(`done.invoke.run-queries`) @@ -133,8 +135,8 @@ describe(`the top-level develop state machine`, () => { service.start() service.send(`done.invoke.initialize`) service.send(`SOURCE_FILE_CHANGED`) - service.send(`done.invoke.initialize-data`) + service.send(`done.invoke.post-bootstrap`) // So we don't start webpack instead service.state.context.compiler = {} as any services.recompile.mockReset() @@ -148,6 +150,7 @@ describe(`the top-level develop state machine`, () => { service.start() service.send(`done.invoke.initialize`) service.send(`done.invoke.initialize-data`) + service.send(`done.invoke.post-bootstrap`) service.state.context.compiler = {} as any services.recompile.mockReset() service.send(`done.invoke.run-queries`) @@ -160,6 +163,7 @@ describe(`the top-level develop state machine`, () => { service.start() service.send(`done.invoke.initialize`) service.send(`done.invoke.initialize-data`) + service.send(`done.invoke.post-bootstrap`) service.state.context.compiler = {} as any service.send(`done.invoke.run-queries`) service.send(`done.invoke.waiting`) @@ -172,6 +176,7 @@ describe(`the top-level develop state machine`, () => { service.start() service.send(`done.invoke.initialize`) service.send(`done.invoke.initialize-data`) + service.send(`done.invoke.post-bootstrap`) service.state.context.compiler = {} as any service.send(`done.invoke.run-queries`) service.send(`EXTRACT_QUERIES_NOW`) @@ -210,6 +215,7 @@ describe(`the top-level develop state machine`, () => { service.start() service.send(`done.invoke.initialize`) service.send(`done.invoke.initialize-data`) + service.send(`done.invoke.post-bootstrap`) await tick() expect(actions.logError).toHaveBeenCalled() expect(service.state.value).toEqual(`waiting`) @@ -221,6 +227,7 @@ describe(`the top-level develop state machine`, () => { service.start() service.send(`done.invoke.initialize`) service.send(`done.invoke.initialize-data`) + service.send(`done.invoke.post-bootstrap`) service.send(`done.invoke.run-queries`) await tick() expect(actions.panic).toHaveBeenCalled() @@ -232,6 +239,7 @@ describe(`the top-level develop state machine`, () => { service.start() service.send(`done.invoke.initialize`) service.send(`done.invoke.initialize-data`) + service.send(`done.invoke.post-bootstrap`) service.state.context.compiler = {} as any service.state.context.sourceFilesDirty = true service.send(`done.invoke.run-queries`) @@ -246,6 +254,7 @@ describe(`the top-level develop state machine`, () => { service.start() service.send(`done.invoke.initialize`) service.send(`done.invoke.initialize-data`) + service.send(`done.invoke.post-bootstrap`) service.state.context.compiler = {} as any service.send(`done.invoke.run-queries`) await tick() diff --git a/packages/gatsby/src/state-machines/develop/index.ts b/packages/gatsby/src/state-machines/develop/index.ts index 295659b7dcf52..6c04a4b12c323 100644 --- a/packages/gatsby/src/state-machines/develop/index.ts +++ b/packages/gatsby/src/state-machines/develop/index.ts @@ -90,6 +90,7 @@ const developConfig: MachineConfig = { }, runningPostBootstrap: { invoke: { + id: `post-bootstrap`, src: `postBootstrap`, onDone: `runningQueries`, },