From 95b7612c32a9446edb14017660c3351ab2612e51 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Tue, 12 Oct 2021 23:32:06 +1100 Subject: [PATCH 1/4] Show errors in a redbox in the preview and silently in the manager --- lib/api/src/lib/StoryIndexClient.ts | 4 +++- lib/api/src/modules/stories.ts | 21 ++++++++++++++------- lib/preview-web/src/PreviewWeb.test.ts | 19 +++++++++++++++---- lib/preview-web/src/PreviewWeb.tsx | 22 ++++++++++++++-------- lib/preview-web/src/StoryIndexClient.ts | 5 ++++- 5 files changed, 50 insertions(+), 21 deletions(-) diff --git a/lib/api/src/lib/StoryIndexClient.ts b/lib/api/src/lib/StoryIndexClient.ts index c1ca9c992221..186b35590bb5 100644 --- a/lib/api/src/lib/StoryIndexClient.ts +++ b/lib/api/src/lib/StoryIndexClient.ts @@ -21,6 +21,8 @@ export class StoryIndexClient extends EventSource { async fetch() { const result = await fetch(PATH); - return result.json() as StoryIndex; + if (result.status === 200) return result.json() as StoryIndex; + + throw new Error(await result.text()); } } diff --git a/lib/api/src/modules/stories.ts b/lib/api/src/modules/stories.ts index 79649d23504c..c25899b282a3 100644 --- a/lib/api/src/modules/stories.ts +++ b/lib/api/src/modules/stories.ts @@ -354,15 +354,22 @@ export const init: ModuleFn = ({ }); }, fetchStoryList: async () => { - const storyIndex = await indexClient.fetch(); + try { + const storyIndex = await indexClient.fetch(); - // We can only do this if the stories.json is a proper storyIndex - if (storyIndex.v !== 3) { - logger.warn(`Skipping story index with version v${storyIndex.v}, awaiting SET_STORIES.`); - return; - } + // We can only do this if the stories.json is a proper storyIndex + if (storyIndex.v !== 3) { + logger.warn(`Skipping story index with version v${storyIndex.v}, awaiting SET_STORIES.`); + return; + } - await fullAPI.setStoryList(storyIndex); + await fullAPI.setStoryList(storyIndex); + } catch (err) { + store.setState({ + storiesConfigured: true, + storiesFailed: err, + }); + } }, setStoryList: async (storyIndex: StoryIndex) => { const hash = transformStoryIndexToStoriesHash(storyIndex, { diff --git a/lib/preview-web/src/PreviewWeb.test.ts b/lib/preview-web/src/PreviewWeb.test.ts index 32066ca8f823..7d9096f4696d 100644 --- a/lib/preview-web/src/PreviewWeb.test.ts +++ b/lib/preview-web/src/PreviewWeb.test.ts @@ -25,6 +25,7 @@ const { history, document } = global; const mockStoryIndex = jest.fn(() => storyIndex); +let mockFetchResult; jest.mock('global', () => ({ ...(jest.requireActual('global') as any), history: { replaceState: jest.fn() }, @@ -39,7 +40,7 @@ jest.mock('global', () => ({ breakingChangesV7: true, // xxx }, - fetch: async () => ({ json: mockStoryIndex }), + fetch: async () => mockFetchResult, })); jest.mock('@storybook/client-logger'); @@ -70,10 +71,11 @@ beforeEach(() => { mockStoryIndex.mockReset().mockReturnValue(storyIndex); addons.setChannel(mockChannel as any); + mockFetchResult = { status: 200, json: mockStoryIndex }; }); describe('PreviewWeb', () => { - describe('constructor', () => { + describe('initialize', () => { it('shows an error if getProjectAnnotations throws', async () => { const err = new Error('meta error'); const preview = new PreviewWeb(); @@ -87,9 +89,18 @@ describe('PreviewWeb', () => { expect(preview.view.showErrorDisplay).toHaveBeenCalled(); expect(mockChannel.emit).toHaveBeenCalledWith(Events.CONFIG_ERROR, err); }); - }); - describe('initialize', () => { + it('shows an error if the stories.json endpoint 500s', async () => { + const err = new Error('sort error'); + mockFetchResult = { status: 500, text: async () => err.toString() }; + + const preview = new PreviewWeb(); + await preview.initialize({ importFn, getProjectAnnotations }); + + expect(preview.view.showErrorDisplay).toHaveBeenCalled(); + expect(mockChannel.emit).toHaveBeenCalledWith(Events.CONFIG_ERROR, expect.any(Error)); + }); + it('sets globals from the URL', async () => { document.location.search = '?id=*&globals=a:c'; diff --git a/lib/preview-web/src/PreviewWeb.tsx b/lib/preview-web/src/PreviewWeb.tsx index 78b584bee4bd..27d3aaa74744 100644 --- a/lib/preview-web/src/PreviewWeb.tsx +++ b/lib/preview-web/src/PreviewWeb.tsx @@ -97,15 +97,21 @@ export class PreviewWeb { if (FEATURES?.storyStoreV7) { this.indexClient = new StoryIndexClient(); - return this.indexClient.fetch().then((fetchedStoryIndex: StoryIndex) => { - this.storyStore.initialize({ - getStoryIndex: () => fetchedStoryIndex, - importFn, - projectAnnotations, - cache: false, + return this.indexClient + .fetch() + .then((fetchedStoryIndex: StoryIndex) => { + this.storyStore.initialize({ + getStoryIndex: () => fetchedStoryIndex, + importFn, + projectAnnotations, + cache: false, + }); + return this.setupListenersAndRenderSelection(); + }) + .catch((err) => { + logger.warn(err); + this.renderPreviewEntryError(err); }); - return this.setupListenersAndRenderSelection(); - }); } if (!getStoryIndex) { diff --git a/lib/preview-web/src/StoryIndexClient.ts b/lib/preview-web/src/StoryIndexClient.ts index e2c891fb945c..501506b9c16a 100644 --- a/lib/preview-web/src/StoryIndexClient.ts +++ b/lib/preview-web/src/StoryIndexClient.ts @@ -20,6 +20,9 @@ export class StoryIndexClient extends EventSource { async fetch() { const result = await fetch(PATH); - return result.json() as StoryIndex; + + if (result.status === 200) return result.json() as StoryIndex; + + throw new Error(await result.text()); } } From d03102ec00e548a8665bed8b0c168669ac0f7e19 Mon Sep 17 00:00:00 2001 From: Michael Shilman Date: Tue, 12 Oct 2021 22:33:49 +0800 Subject: [PATCH 2/4] Fix broken test cases --- lib/preview-web/src/PreviewWeb.integration.test.ts | 2 +- lib/preview-web/src/PreviewWeb.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/preview-web/src/PreviewWeb.integration.test.ts b/lib/preview-web/src/PreviewWeb.integration.test.ts index 2973d03dff4d..75ed6bcfafcc 100644 --- a/lib/preview-web/src/PreviewWeb.integration.test.ts +++ b/lib/preview-web/src/PreviewWeb.integration.test.ts @@ -38,7 +38,7 @@ jest.mock('global', () => ({ FEATURES: { storyStoreV7: true, }, - fetch: async () => ({ json: async () => mockStoryIndex }), + fetch: async () => ({ status: 200, json: async () => mockStoryIndex, text: () => 'error text' }), })); beforeEach(() => { diff --git a/lib/preview-web/src/PreviewWeb.test.ts b/lib/preview-web/src/PreviewWeb.test.ts index 7d9096f4696d..8fb3bf046148 100644 --- a/lib/preview-web/src/PreviewWeb.test.ts +++ b/lib/preview-web/src/PreviewWeb.test.ts @@ -71,7 +71,7 @@ beforeEach(() => { mockStoryIndex.mockReset().mockReturnValue(storyIndex); addons.setChannel(mockChannel as any); - mockFetchResult = { status: 200, json: mockStoryIndex }; + mockFetchResult = { status: 200, json: mockStoryIndex, text: () => 'error text' }; }); describe('PreviewWeb', () => { From 4bbeac786cb79f6f54fb14f5d40fe074476be6e7 Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Tue, 12 Oct 2021 23:38:35 +1100 Subject: [PATCH 3/4] Add a manager-side test --- lib/api/src/tests/stories.test.js | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/lib/api/src/tests/stories.test.js b/lib/api/src/tests/stories.test.js index f7cb35fae989..94998ab3f59e 100644 --- a/lib/api/src/tests/stories.test.js +++ b/lib/api/src/tests/stories.test.js @@ -18,7 +18,7 @@ const mockStories = jest.fn(); jest.mock('../lib/events'); jest.mock('global', () => ({ ...jest.requireActual('global'), - fetch: jest.fn(() => ({ json: () => ({ v: 3, stories: mockStories() }) })), + fetch: jest.fn(), })); beforeEach(() => { @@ -58,6 +58,9 @@ const provider = { getConfig: jest.fn() }; beforeEach(() => { provider.getConfig.mockReturnValue({}); global.EventSource.reset(); + global.fetch + .mockReset() + .mockReturnValue({ status: 200, json: () => ({ v: 3, stories: mockStories() }) }); }); describe('stories API', () => { @@ -877,7 +880,23 @@ describe('stories API', () => { }); }); - describe('fetchStoryList', () => { + describe('fetchStoryIndex', () => { + it('deals with 500 errors', async () => { + const navigate = jest.fn(); + const store = createMockStore(); + const fullAPI = Object.assign(new EventEmitter(), {}); + + global.fetch.mockReturnValue({ status: 500, text: async () => new Error('sorting error') }); + const { api, init } = initStories({ store, navigate, provider, fullAPI }); + Object.assign(fullAPI, api); + + await init(); + + const { storiesConfigured, storiesFailed } = store.getState(); + expect(storiesConfigured).toBe(true); + expect(storiesFailed.message).toMatch(/sorting error/); + }); + it('sets the initial set of stories in the stories hash', async () => { const navigate = jest.fn(); const store = createMockStore(); From 6699ba7761b22e8fedf468f514af4ba450ebb41b Mon Sep 17 00:00:00 2001 From: Tom Coleman Date: Wed, 13 Oct 2021 08:55:01 +1100 Subject: [PATCH 4/4] Fix integration test --- lib/preview-web/src/PreviewWeb.integration.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/preview-web/src/PreviewWeb.integration.test.ts b/lib/preview-web/src/PreviewWeb.integration.test.ts index 75ed6bcfafcc..b14853e9de0a 100644 --- a/lib/preview-web/src/PreviewWeb.integration.test.ts +++ b/lib/preview-web/src/PreviewWeb.integration.test.ts @@ -38,7 +38,7 @@ jest.mock('global', () => ({ FEATURES: { storyStoreV7: true, }, - fetch: async () => ({ status: 200, json: async () => mockStoryIndex, text: () => 'error text' }), + fetch: async () => ({ status: 200, json: async () => mockStoryIndex }), })); beforeEach(() => {