From 57c7d7586aee1325a6f221a844840d5c32a7f330 Mon Sep 17 00:00:00 2001 From: Pankaj Yadav Date: Thu, 18 Jul 2024 15:13:27 +0530 Subject: [PATCH] Add retry in withPage function (#993) * Add retry in withPage function * Fix lint * enable retry by default * Add env var in tests * Add test to check retry true * Fix tests * Fix tests --- src/snapshots.js | 2 +- src/utils.js | 105 +++++++++++++++++++++++------------------ test/storybook.test.js | 55 +++++++++++++++++++++ 3 files changed, 116 insertions(+), 46 deletions(-) diff --git a/src/snapshots.js b/src/snapshots.js index fd4fc1ef..35f10a73 100644 --- a/src/snapshots.js +++ b/src/snapshots.js @@ -232,7 +232,7 @@ export async function* takeStorybookSnapshots(percy, callback, { baseUrl, flags log.debug(`Page crashed while loading story: ${snapshots[0].name}`); // return true to retry as long as the length decreases return lastCount > snapshots.length; - }); + }, { snapshotName: snapshots[0].name }); } catch (e) { if (process.env.PERCY_SKIP_STORY_ON_ERROR === 'true') { let { name } = snapshots[0]; diff --git a/src/utils.js b/src/utils.js index fabed9b2..3a4e221a 100644 --- a/src/utils.js +++ b/src/utils.js @@ -1,4 +1,5 @@ import { request, createRootResource, yieldTo } from '@percy/cli-command/utils'; +import { logger } from '@percy/cli-command'; import spawn from 'cross-spawn'; // check storybook version @@ -138,52 +139,66 @@ export function decodeStoryArgs(value) { // Borrows a percy discovery browser page to navigate to a URL and evaluate a function, returning // the results and normalizing any thrown errors. -export async function* withPage(percy, url, callback, retry) { - // provide discovery options that may impact how the page loads - let page = yield percy.browser.page({ - networkIdleTimeout: percy.config.discovery.networkIdleTimeout, - requestHeaders: getAuthHeaders(percy.config.discovery), - captureMockedServiceWorker: percy.config.discovery.captureMockedServiceWorker, - userAgent: percy.config.discovery.userAgent - }); +export async function* withPage(percy, url, callback, retry, args) { + let log = logger('storybook:utils'); + let attempt = 0; + let retries = 3; + while (attempt < retries) { + try { + // provide discovery options that may impact how the page loads + let page = yield percy.browser.page({ + networkIdleTimeout: percy.config.discovery.networkIdleTimeout, + requestHeaders: getAuthHeaders(percy.config.discovery), + captureMockedServiceWorker: percy.config.discovery.captureMockedServiceWorker, + userAgent: percy.config.discovery.userAgent + }); + + // patch eval to include storybook specific helpers in the local scope + page.eval = (fn, ...args) => page.constructor.prototype.eval.call(page, ( + typeof fn === 'string' ? fn : [ + 'function withPercyStorybookHelpers() {', + ` const VAL_REG = ${VAL_REG};`, + ` const NUM_REG = ${NUM_REG};`, + ` const HEX_REG = ${HEX_REG};`, + ` const COL_REG = ${COL_REG};`, + ` const VALID_REG = ${VALID_REG};`, + ` return (${fn})(...arguments);`, + ` ${isPlainObject}`, + ` ${validateStoryArgs}`, + ` ${encodeStoryArgs}`, + ` ${decodeStoryArgs}`, + '}' + ].join('\n') + ), ...args); + + try { + yield page.goto(url); + return yield* yieldTo(callback(page)); + } catch (error) { + // if the page crashed and retry returns truthy, try again + if (error.message?.includes('crashed') && retry?.()) { + return yield* withPage(...arguments); + } - // patch eval to include storybook specific helpers in the local scope - page.eval = (fn, ...args) => page.constructor.prototype.eval.call(page, ( - typeof fn === 'string' ? fn : [ - 'function withPercyStorybookHelpers() {', - ` const VAL_REG = ${VAL_REG};`, - ` const NUM_REG = ${NUM_REG};`, - ` const HEX_REG = ${HEX_REG};`, - ` const COL_REG = ${COL_REG};`, - ` const VALID_REG = ${VALID_REG};`, - ` return (${fn})(...arguments);`, - ` ${isPlainObject}`, - ` ${validateStoryArgs}`, - ` ${encodeStoryArgs}`, - ` ${decodeStoryArgs}`, - '}' - ].join('\n') - ), ...args); - - try { - yield page.goto(url); - return yield* yieldTo(callback(page)); - } catch (error) { - // if the page crashed and retry returns truthy, try again - if (error.message?.includes('crashed') && retry?.()) { - return yield* withPage(...arguments); + /* istanbul ignore next: purposefully not handling real errors */ + throw (typeof error !== 'string' ? error : new Error(error.replace( + // strip generic error names and confusing stack traces + /^Error:\s((.+?)\n\s+at\s.+)$/s, + // keep the stack trace if the error came from a client script + /\n\s+at\s.+?\(https?:/.test(error) ? '$1' : '$2' + ))); + } finally { + // always clean up and close the page + await page?.close(); + } + } catch (error) { + attempt++; + let enableRetry = process.env.PERCY_RETRY_STORY_ON_ERROR || 'true'; + if (!(enableRetry === 'true') || attempt === retries) { + throw error; + } + log.warn(`Retrying Story: ${args.snapshotName}`); } - - /* istanbul ignore next: purposefully not handling real errors */ - throw (typeof error !== 'string' ? error : new Error(error.replace( - // strip generic error names and confusing stack traces - /^Error:\s((.+?)\n\s+at\s.+)$/s, - // keep the stack trace if the error came from a client script - /\n\s+at\s.+?\(https?:/.test(error) ? '$1' : '$2' - ))); - } finally { - // always clean up and close the page - await page?.close(); } } @@ -271,8 +286,8 @@ export function evalSetCurrentStory({ waitFor }, story) { let { id, queryParams, globals, args } = story; // emit a series of events to render the desired story - channel.emit('updateGlobals', { globals: {} }); channel.emit('setCurrentStory', { storyId: id }); + channel.emit('updateGlobals', { globals: {} }); channel.emit('updateQueryParams', { ...queryParams }); if (globals) channel.emit('updateGlobals', { globals: decodeStoryArgs(globals) }); if (args) channel.emit('updateStoryArgs', { storyId: id, updatedArgs: decodeStoryArgs(args) }); diff --git a/test/storybook.test.js b/test/storybook.test.js index 7530a97f..4a42d43a 100644 --- a/test/storybook.test.js +++ b/test/storybook.test.js @@ -107,6 +107,7 @@ describe('percy storybook', () => { }); it('errors when the client api is missing', async () => { + process.env.PERCY_RETRY_STORY_ON_ERROR = false; await expectAsync(storybook(['http://localhost:8000'])).toBeRejected(); expect(logger.stderr).toEqual([ @@ -147,6 +148,7 @@ describe('percy storybook', () => { }); it('errors when the storybook page errors', async () => { + process.env.PERCY_RETRY_STORY_ON_ERROR = false; server.reply('/iframe.html', () => [200, 'text/html', [ ``, + `` + ].join('')]); + + server.reply('/iframe.html?id=1&viewMode=story', () => [200, 'text/html', [ + ``, + `` + ].join('')]); + + // does not reject + await storybook(['http://localhost:8000']); + + // contains logs of story error + expect(logger.stderr).toEqual([ + '[percy] Retrying Story: foo: bar', + '[percy] Retrying Story: foo: bar', + '[percy] Failed to capture story: foo: bar', + // error logs contain the client stack trace + jasmine.stringMatching(/^\[percy\] Error: Story Error\n.*\/iframe\.html.*$/s), + // does not create a build if all stories failed [ 1 in this case ] + '[percy] Build not created' + ]); + }); + }); + it('uses the preview dom when javascript is enabled', async () => { const FAKE_PREVIEW_V8 = `{ async extract() { return ${JSON.stringify([ { id: '1', kind: 'foo', name: 'bar' }, @@ -514,6 +568,7 @@ describe('percy storybook', () => { }); it('handles page crashes while taking snapshots', async () => { + process.env.PERCY_RETRY_STORY_ON_ERROR = false; // eslint-disable-next-line import/no-extraneous-dependencies let { Percy } = await import('@percy/core');