Merge pull request #21112 from storybookjs/20663-rationalize-index-errors

Core: Don't crash when there are errors indexing

code/lib/core-server/src/utils/StoryIndexGenerator.test.ts

@@ -895,7 +895,8 @@ describe('StoryIndexGenerator', () => {
           [normalizeStoriesEntry('./src/docs2/MetaOf.mdx', options)],
           options
         );
-        await expect(() => generator.initialize()).rejects.toThrowError(
+        await generator.initialize();
+        await expect(() => generator.getIndex()).rejects.toThrowError(
           /Could not find "..\/A.stories" for docs file/
         );
       });

code/lib/core-server/src/utils/StoryIndexGenerator.ts

@@ -35,7 +35,11 @@ type StoriesCacheEntry = {
   dependents: Path[];
   type: 'stories';
 };
-type CacheEntry = false | StoriesCacheEntry | DocsCacheEntry;
+type ErrorEntry = {
+  type: 'error';
+  err: Error;
+};
+type CacheEntry = false | StoriesCacheEntry | DocsCacheEntry | ErrorEntry;
 type SpecifierStoriesCache = Record<Path, CacheEntry>;
 
 export const AUTODOCS_TAG = 'autodocs';
@@ -99,6 +103,9 @@ export class StoryIndexGenerator {
   //  - the preview changes [not yet implemented]
   private lastIndex?: StoryIndex;
 
+  // Same as the above but for the error case
+  private lastError?: Error;
+
   constructor(
     public readonly specifiers: NormalizedStoriesSpecifier[],
     public readonly options: {
@@ -165,7 +172,12 @@ export class StoryIndexGenerator {
         return Promise.all(
           Object.keys(entry).map(async (absolutePath) => {
             if (entry[absolutePath] && !overwrite) return;
-            entry[absolutePath] = await updater(specifier, absolutePath, entry[absolutePath]);
+
+            try {
+              entry[absolutePath] = await updater(specifier, absolutePath, entry[absolutePath]);
+            } catch (err) {
+              entry[absolutePath] = { type: 'error', err };
+            }
           })
         );
       })
@@ -176,7 +188,7 @@ export class StoryIndexGenerator {
     return /(?<!\.stories)\.mdx$/i.test(absolutePath);
   }
 
-  async ensureExtracted(): Promise<IndexEntry[]> {
+  async ensureExtracted(): Promise<(IndexEntry | ErrorEntry)[]> {
     // First process all the story files. Then, in a second pass,
     // process the docs files. The reason for this is that the docs
     // files may use the `<Meta of={XStories} />` syntax, which requires
@@ -191,9 +203,10 @@ export class StoryIndexGenerator {
 
     return this.specifiers.flatMap((specifier) => {
       const cache = this.specifierToCache.get(specifier);
-      return Object.values(cache).flatMap((entry): IndexEntry[] => {
+      return Object.values(cache).flatMap((entry): (IndexEntry | ErrorEntry)[] => {
         if (!entry) return [];
         if (entry.type === 'docs') return [entry];
+        if (entry.type === 'error') return [entry];
         return entry.entries;
       });
     });
@@ -475,44 +488,56 @@ export class StoryIndexGenerator {
 
   async getIndex() {
     if (this.lastIndex) return this.lastIndex;
+    if (this.lastError) throw this.lastError;
 
     // Extract any entries that are currently missing
     // Pull out each file's stories into a list of stories, to be composed and sorted
     const storiesList = await this.ensureExtracted();
-    const sorted = await this.sortStories(storiesList);
 
-    let compat = sorted;
-    if (this.options.storiesV2Compatibility) {
-      const titleToStoryCount = Object.values(sorted).reduce((acc, story) => {
-        acc[story.title] = (acc[story.title] || 0) + 1;
-        return acc;
-      }, {} as Record<ComponentTitle, number>);
+    try {
+      const firstError = storiesList.find((entry) => entry.type === 'error');
+      if (firstError) throw (firstError as ErrorEntry).err;
 
-      // @ts-expect-error (Converted from ts-ignore)
-      compat = Object.entries(sorted).reduce((acc, entry) => {
-        const [id, story] = entry;
-        if (story.type === 'docs') return acc;
+      const sorted = await this.sortStories(storiesList as IndexEntry[]);
 
-        acc[id] = {
-          ...story,
-          kind: story.title,
-          story: story.name,
-          parameters: {
-            __id: story.id,
-            docsOnly: titleToStoryCount[story.title] === 1 && story.name === 'Page',
-            fileName: story.importPath,
-          },
-        };
-        return acc;
-      }, {} as Record<StoryId, V2CompatIndexEntry>);
+      let compat = sorted;
+      if (this.options.storiesV2Compatibility) {
+        const titleToStoryCount = Object.values(sorted).reduce((acc, story) => {
+          acc[story.title] = (acc[story.title] || 0) + 1;
+          return acc;
+        }, {} as Record<ComponentTitle, number>);
+
+        // @ts-expect-error (Converted from ts-ignore)
+        compat = Object.entries(sorted).reduce((acc, entry) => {
+          const [id, story] = entry;
+          if (story.type === 'docs') return acc;
+
+          acc[id] = {
+            ...story,
+            kind: story.title,
+            story: story.name,
+            parameters: {
+              __id: story.id,
+              docsOnly: titleToStoryCount[story.title] === 1 && story.name === 'Page',
+              fileName: story.importPath,
+            },
+          };
+          return acc;
+        }, {} as Record<StoryId, V2CompatIndexEntry>);
+      }
+
+      this.lastIndex = {
+        v: 4,
+        entries: compat,
+      };
+
+      return this.lastIndex;
+    } catch (err) {
+      this.lastError = err;
+      logger.warn(`🚨 Couldn't fetch index`);
+      logger.warn(this.lastError.stack);
+      throw this.lastError;
     }
-
-    this.lastIndex = {
-      v: 4,
-      entries: compat,
-    };
-
-    return this.lastIndex;
   }
 
   invalidate(specifier: NormalizedStoriesSpecifier, importPath: Path, removed: boolean) {
@@ -551,6 +576,7 @@ export class StoryIndexGenerator {
       cache[absolutePath] = false;
     }
     this.lastIndex = null;
+    this.lastError = null;
   }
 
   async getStorySortParameter() {

code/lib/core-server/src/utils/doTelemetry.ts

@@ -1,10 +1,11 @@
-import type { CoreConfig, Options } from '@storybook/types';
+import type { CoreConfig, Options, StoryIndex } from '@storybook/types';
 import { telemetry, getPrecedingUpgrade } from '@storybook/telemetry';
 import { useStorybookMetadata } from './metadata';
 import type { StoryIndexGenerator } from './StoryIndexGenerator';
 import { summarizeIndex } from './summarizeIndex';
 import { router } from './router';
 import { versionStatus } from './versionStatus';
+import { sendTelemetryError } from '../withTelemetry';
 
 export async function doTelemetry(
   core: CoreConfig,
@@ -13,7 +14,18 @@ export async function doTelemetry(
 ) {
   if (!core?.disableTelemetry) {
     initializedStoryIndexGenerator.then(async (generator) => {
-      const storyIndex = await generator?.getIndex();
+      let storyIndex: StoryIndex;
+      try {
+        storyIndex = await generator?.getIndex();
+      } catch (err) {
+        // If we fail to get the index, treat it as a recoverable error, but send it up to telemetry
+        // as if we crashed. In the future we will revisit this to send a distinct error
+        sendTelemetryError(err, 'dev', {
+          cliOptions: options,
+          presetOptions: { ...options, corePresets: [], overridePresets: [] },
+        });
+        return;
+      }
       const { versionCheck, versionUpdates } = options;
       const payload = {
         precedingUpgrade: await getPrecedingUpgrade(),

code/lib/core-server/src/withTelemetry.ts

@@ -58,6 +58,36 @@ async function getErrorLevel({ cliOptions, presetOptions }: TelemetryOptions): P
   return 'full';
 }
 
+export async function sendTelemetryError(
+  error: Error,
+  eventType: EventType,
+  options: TelemetryOptions
+) {
+  try {
+    const errorLevel = await getErrorLevel(options);
+    if (errorLevel !== 'none') {
+      const precedingUpgrade = await getPrecedingUpgrade();
+
+      await telemetry(
+        'error',
+        {
+          eventType,
+          precedingUpgrade,
+          error: errorLevel === 'full' ? error : undefined,
+          errorHash: oneWayHash(error.message),
+        },
+        {
+          immediate: true,
+          configDir: options.cliOptions.configDir || options.presetOptions?.configDir,
+          enableCrashReports: errorLevel === 'full',
+        }
+      );
+    }
+  } catch (err) {
+    // if this throws an error, we just move on
+  }
+}
+
 export async function withTelemetry(
   eventType: EventType,
   options: TelemetryOptions,
@@ -69,30 +99,7 @@ export async function withTelemetry(
   try {
     await run();
   } catch (error) {
-    try {
-      const errorLevel = await getErrorLevel(options);
-      if (errorLevel !== 'none') {
-        const precedingUpgrade = await getPrecedingUpgrade();
-
-        await telemetry(
-          'error',
-          {
-            eventType,
-            precedingUpgrade,
-            error: errorLevel === 'full' ? error : undefined,
-            errorHash: oneWayHash(error.message),
-          },
-          {
-            immediate: true,
-            configDir: options.cliOptions.configDir || options.presetOptions?.configDir,
-            enableCrashReports: errorLevel === 'full',
-          }
-        );
-      }
-    } catch (err) {
-      // if this throws an error, we just move on
-    }
-
+    await sendTelemetryError(error, eventType, options);
     throw error;
   }
 }

code/lib/manager-api/src/modules/stories.ts

@@ -354,7 +354,7 @@ export const init: ModuleFn<SubAPI, SubState, true> = ({
       // Now we need to patch in the existing prepared stories
       const oldHash = store.getState().index;
 
-      await store.setState({ index: addPreparedStories(newHash, oldHash) });
+      await store.setState({ index: addPreparedStories(newHash, oldHash), indexError: undefined });
     },
     updateStory: async (
       storyId: StoryId,

code/lib/manager-api/src/tests/stories.test.ts

@@ -630,6 +630,48 @@ describe('stories API', () => {
 
       expect(Object.keys(index)).toEqual(['component-a', 'component-a--story-1']);
     });
+
+    it('clears 500 errors when invalidated', async () => {
+      const navigate = jest.fn();
+      const store = createMockStore();
+      const fullAPI = Object.assign(new EventEmitter(), {
+        setIndex: jest.fn(),
+      });
+
+      (global.fetch as jest.Mock<ReturnType<typeof global.fetch>>).mockReturnValueOnce(
+        Promise.resolve({
+          status: 500,
+          text: async () => new Error('sorting error'),
+        } as any as Response)
+      );
+      const { api, init } = initStoriesAndSetState({ store, navigate, provider, fullAPI } as any);
+      Object.assign(fullAPI, api);
+
+      await init();
+
+      const { indexError } = store.getState();
+      expect(indexError).toBeDefined();
+
+      (global.fetch as jest.Mock<ReturnType<typeof global.fetch>>).mockClear();
+      mockGetEntries.mockReturnValueOnce({
+        'component-a--story-1': {
+          type: 'story',
+          id: 'component-a--story-1',
+          title: 'Component A',
+          name: 'Story 1',
+          importPath: './path/to/component-a.ts',
+        },
+      });
+      provider.serverChannel.emit(STORY_INDEX_INVALIDATED);
+      expect(global.fetch).toHaveBeenCalledTimes(1);
+
+      // Let the promise/await chain resolve
+      await new Promise((r) => setTimeout(r, 0));
+      const { index, indexError: newIndexError } = store.getState();
+      expect(newIndexError).not.toBeDefined();
+
+      expect(Object.keys(index)).toEqual(['component-a', 'component-a--story-1']);
+    });
   });
 
   describe('STORY_SPECIFIED event', () => {