updating error message and fixing retry snapshot bug (#1072)

* updating error message and fixing retry snapshot bug * addressing comment * addressing comments * addressing comments test fix test fix test fix test fix test fix test fix * addressig comments * adding from key * test fix * test fix * test fix * comment fix * typo fix
percy · Nov 27, 2024 · 051819a · 051819a
1 parent c3cc70f
commit 051819a
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 11 deletions.
diff --git a/src/snapshots.js b/src/snapshots.js
@@ -173,8 +173,8 @@ export async function* takeStorybookSnapshots(percy, callback, { baseUrl, flags
 
     // gather storybook data in parallel
     let [environmentInfo, stories] = yield* yieldAll([
-      withPage(percy, aboutUrl, p => p.eval(evalStorybookEnvironmentInfo)),
-      withPage(percy, previewUrl, p => p.eval(evalStorybookStorySnapshots))
+      withPage(percy, aboutUrl, p => p.eval(evalStorybookEnvironmentInfo), undefined, { from: 'about url' }),
+      withPage(percy, previewUrl, p => p.eval(evalStorybookStorySnapshots), undefined, { from: 'preview url' })
     ]);
 
     // map stories to snapshot options

diff --git a/src/utils.js b/src/utils.js
@@ -170,7 +170,6 @@ export async function* withPage(percy, url, callback, retry, args) {
           '}'
         ].join('\n')
       ), ...args);
-
       try {
         yield page.goto(url);
         return yield* yieldTo(callback(page));
@@ -194,10 +193,29 @@ export async function* withPage(percy, url, callback, retry, args) {
     } catch (error) {
       attempt++;
       let enableRetry = process.env.PERCY_RETRY_STORY_ON_ERROR || 'true';
+      const from = args?.from;
       if (!(enableRetry === 'true') || attempt === retries) {
+        // Add snapshotName to the error message
+        const snapshotName = args?.snapshotName;
+        if (from) {
+          error.message = `${from}: \n${error.message}`;
+        }
+        if (snapshotName) {
+          error.message = `Snapshot Name: ${snapshotName}: \n${error.message}`;
+        }
         throw error;
       }
-      log.warn(`Retrying Story: ${args.snapshotName}`);
+
+      // throw warning message with snapshot name if it is present.
+      if (args?.snapshotName) {
+        log.warn(`Retrying Story: ${args.snapshotName}, attempt: ${attempt}`);
+      }
+      // throw warning message with from where it is called if from in present.
+      if (from) {
+        log.warn(
+          `Retrying because error occurred in: ${from}, attempt: ${attempt}`
+        );
+      }
     }
   }
 }

diff --git a/test/storybook.test.js b/test/storybook.test.js
@@ -112,7 +112,7 @@ describe('percy storybook', () => {
 
     expect(logger.stderr).toEqual([
       '[percy] Build not created',
-      '[percy] Error: Storybook object not found on the window. ' +
+      '[percy] Error: preview url: \nStorybook object not found on the window. ' +
         'Open Storybook and check the console for errors.'
     ]);
   });
@@ -173,12 +173,15 @@ describe('percy storybook', () => {
 
     await expectAsync(storybook(['http://localhost:8000']))
     // message contains the client stack trace
-      .toBeRejectedWithError(/^Story Error\n.*\/iframe\.html.*$/s);
+      .toBeRejectedWithError(/Story Error\n.*\/iframe\.html.*$/s);
 
     expect(logger.stderr).toEqual([
       '[percy] Build not created',
       // message contains the client stack trace
-      jasmine.stringMatching(/^\[percy\] Error: Story Error\n.*\/iframe\.html.*$/s)
+      jasmine.stringMatching(
+        /^\[percy\] Error: Snapshot Name:/s
+      )
+
     ]);
   });
 
@@ -223,7 +226,9 @@ describe('percy storybook', () => {
       expect(logger.stderr).toEqual([
         '[percy] Failed to capture story: foo: bar',
         // error logs contain the client stack trace
-        jasmine.stringMatching(/^\[percy\] Error: Story Error\n.*\/iframe\.html.*$/s),
+        jasmine.stringMatching(
+          /^\[percy\] Error: Snapshot Name:/s
+        ),
         // does not create a build if all stories failed [ 1 in this case ]
         '[percy] Build not created'
       ]);
@@ -269,11 +274,14 @@ describe('percy storybook', () => {
 
       // contains logs of story error
       expect(logger.stderr).toEqual([
-        '[percy] Retrying Story: foo: bar',
-        '[percy] Retrying Story: foo: bar',
+        '[percy] Retrying Story: foo: bar, attempt: 1',
+        '[percy] Retrying Story: foo: bar, attempt: 2',
         '[percy] Failed to capture story: foo: bar',
         // error logs contain the client stack trace
-        jasmine.stringMatching(/^\[percy\] Error: Story Error\n.*\/iframe\.html.*$/s),
+        jasmine.stringMatching(
+          /^\[percy\] Error: Snapshot Name:/s
+        ),
+
         // does not create a build if all stories failed [ 1 in this case ]
         '[percy] Build not created'
       ]);