diff --git a/packages/cli-command/src/intelliStory.js b/packages/cli-command/src/intelliStory.js index ed74a3aed..9a282b627 100644 --- a/packages/cli-command/src/intelliStory.js +++ b/packages/cli-command/src/intelliStory.js @@ -287,9 +287,39 @@ function readStats(statsFile, projectRoot, log, configDirs) { return { files, modules, buildId: stats.buildId }; } +// Every other failure mode in this module — git, fs, config, lockfile, glob, +// degraded graph — is converted to IntelliStoryBailError so the caller can +// degrade to a full snapshot run. API and network failures are no different: a +// 403 means the organization is not entitled to IntelliStory, and a 5xx / +// timeout / connection reset is transient. Neither should hard-fail the +// storybook command, which is what a bare Error does once @percy/storybook +// catches IntelliStoryBailError specifically. +function isNotAllowed(e) { + return e?.response?.statusCode === 403; +} + +function apiBailError(e, action) { + if (isNotAllowed(e)) { + return new IntelliStoryBailError(`IntelliStory: not enabled for this organization (${action} was not allowed); running full snapshot set`); + } + return new IntelliStoryBailError(`IntelliStory: ${action} failed: ${e?.message}; running full snapshot set`); +} + async function pollGraphStatus(percy, buildId, log) { for (let i = 0; i < POLL_ATTEMPTS; i++) { - const res = await percy.client.getStatus('intelli_story_graph', [buildId]); + let res; + try { + res = await percy.client.getStatus('intelli_story_graph', [buildId]); + } catch (e) { + // A 403 will not resolve by polling again, so stop immediately rather + // than burning the full POLL_ATTEMPTS window on it. Anything else is + // treated as "not done yet" and retried; if it never clears, the caller's + // timed-out bail covers it. + if (isNotAllowed(e)) throw apiBailError(e, 'graph status poll'); + log.debug(`IntelliStory: graph status (attempt ${i + 1}) errored: ${e?.message}`); + if (i < POLL_ATTEMPTS - 1) await new Promise(r => setTimeout(r, POLL_INTERVAL_MS)); + continue; + } const status = res?.status; log.debug(`IntelliStory: graph status (attempt ${i + 1}) = ${status}`); if (status === 'done' || status === 'failed') return { status, data: res?.data }; @@ -328,7 +358,12 @@ export async function getBaselineAndAffectedNodes(percy, baseline, log) { // Always look up the base build: its `browsers_changed_from_base` flag forces // a full snapshot run regardless of whether an explicit baseline was configured. - const baseLookup = await percy.client.getIntelliStorySnapshotNameToCommit(percy.build?.id); + let baseLookup; + try { + baseLookup = await percy.client.getIntelliStorySnapshotNameToCommit(percy.build?.id); + } catch (e) { + throw apiBailError(e, 'base build lookup'); + } log.debug(`IntelliStory: base lookup ${JSON.stringify(baseLookup)}`); if (baseLookup?.browsers_changed_from_base) { @@ -552,9 +587,13 @@ export function extractStorybookPaths(snapshots, normalizeImportPath, log) { export async function runGraphGeneration(percy, buildId, payload, log) { const { files, modules, storybookPaths, affectedNodes, affectedFileLocations } = payload; log.debug(`IntelliStory: starting graph generation job ${JSON.stringify({ buildId, files, modules, storybookPaths, affectedNodes, affectedFileLocations })}`); - await percy.client.generateIntelliStoryGraph(buildId, { - files, modules, storybookPaths, affectedNodes, affectedFileLocations - }); + try { + await percy.client.generateIntelliStoryGraph(buildId, { + files, modules, storybookPaths, affectedNodes, affectedFileLocations + }); + } catch (e) { + throw apiBailError(e, 'graph generation request'); + } const { status } = await pollGraphStatus(percy, buildId, log); if (status !== 'done') { diff --git a/packages/cli-command/test/intelliStory.test.js b/packages/cli-command/test/intelliStory.test.js index aec56de93..5c0b6c1a3 100644 --- a/packages/cli-command/test/intelliStory.test.js +++ b/packages/cli-command/test/intelliStory.test.js @@ -292,6 +292,43 @@ describe('intelliStory', () => { () => getBaselineAndAffectedNodes(percy, '--upload-pack=evil', log), 'unsafe baseline ref'); }); + + it('bails when the base lookup rejects with a transient API error', async () => { + let percy = { + client: { + getIntelliStorySnapshotNameToCommit: async () => { + throw Object.assign(new Error('socket hang up'), { response: { statusCode: 502 } }); + } + } + }; + await expectBail( + () => getBaselineAndAffectedNodes(percy, 'HEAD', log), + 'base build lookup failed: socket hang up'); + }); + + it('bails with an entitlement message when the base lookup is not allowed', async () => { + let percy = { + client: { + getIntelliStorySnapshotNameToCommit: async () => { + throw Object.assign(new Error('Forbidden'), { response: { statusCode: 403 } }); + } + } + }; + await expectBail( + () => getBaselineAndAffectedNodes(percy, 'HEAD', log), + 'not enabled for this organization'); + }); + + it('bails when the base lookup rejects with a bare network error (no response)', async () => { + let percy = { + client: { + getIntelliStorySnapshotNameToCommit: async () => { throw new Error('ECONNRESET'); } + } + }; + await expectBail( + () => getBaselineAndAffectedNodes(percy, 'HEAD', log), + 'base build lookup failed: ECONNRESET'); + }); }); describe('assertNoDotStorybookChange()', () => { @@ -668,6 +705,87 @@ describe('intelliStory', () => { () => runGraphGeneration(percy, 'bld-1', { files: [], modules: [], storybookPaths: [], affectedNodes: [] }, log), 'did not complete'); }); + + const emptyPayload = { files: [], modules: [], storybookPaths: [], affectedNodes: [] }; + + it('bails when the generate request rejects with a transient API error', async () => { + let log = mockLog(); + let percy = { + client: { + generateIntelliStoryGraph: async () => { + throw Object.assign(new Error('503 Service Unavailable'), { response: { statusCode: 503 } }); + }, + getStatus: async () => ({ status: 'done' }) + } + }; + await expectBail( + () => runGraphGeneration(percy, 'bld-1', emptyPayload, log), + 'graph generation request failed: 503 Service Unavailable'); + }); + + it('bails with an entitlement message when the generate request is not allowed', async () => { + let log = mockLog(); + let percy = { + client: { + generateIntelliStoryGraph: async () => { + throw Object.assign(new Error('Forbidden'), { response: { statusCode: 403 } }); + }, + getStatus: async () => ({ status: 'done' }) + } + }; + await expectBail( + () => runGraphGeneration(percy, 'bld-1', emptyPayload, log), + 'not enabled for this organization'); + }); + + // A 403 mid-poll will never clear, so it stops on the first attempt rather + // than burning the whole POLL_ATTEMPTS window — hence no fake clock here. + it('stops polling immediately when the status poll is not allowed', async () => { + let log = mockLog(); + let getStatus = jasmine.createSpy('getStatus').and.callFake(async () => { + throw Object.assign(new Error('Forbidden'), { response: { statusCode: 403 } }); + }); + let percy = { client: { generateIntelliStoryGraph: async () => {}, getStatus } }; + + await expectBail( + () => runGraphGeneration(percy, 'bld-1', emptyPayload, log), + 'not enabled for this organization'); + expect(getStatus).toHaveBeenCalledTimes(1); + }); + + describe('when the status poll keeps failing transiently', () => { + beforeEach(() => jasmine.clock().install()); + afterEach(() => jasmine.clock().uninstall()); + + // Flush microtasks between clock ticks so the poll loop advances. + async function drainPolls(promise, rounds = 20) { + for (let i = 0; i < rounds; i++) { + await Promise.resolve(); + await Promise.resolve(); + jasmine.clock().tick(5000); + } + return promise; + } + + it('retries and then bails as timed out rather than throwing raw', async () => { + let log = mockLog(); + let percy = { + client: { + generateIntelliStoryGraph: async () => {}, + getStatus: async () => { throw new Error('ETIMEDOUT'); } + } + }; + + let err = null; + await drainPolls( + runGraphGeneration(percy, 'bld-1', emptyPayload, log).catch(e => { err = e; }) + ); + + expect(err).toBeInstanceOf(IntelliStoryBailError); + expect(err.message).toContain('did not complete'); + expect(log.debug).toHaveBeenCalledWith(jasmine.stringMatching(/errored: ETIMEDOUT/)); + }); + }); }); describe('maybeWriteTrace()', () => { diff --git a/packages/client/src/client.js b/packages/client/src/client.js index ad9f29881..c8e6d3757 100644 --- a/packages/client/src/client.js +++ b/packages/client/src/client.js @@ -456,15 +456,13 @@ export class PercyClient { async getIntelliStorySnapshotNameToCommit(buildId) { this.log.debug('IntelliStory: looking up baselines...'); + // `build_id` is the only input. The build already exists by the time this is + // called, so its base build has been selected server-side and the endpoint + // reads through to that base build's commit — there is nothing to predict + // from git/PR context any more. const qs = new URLSearchParams(); if (buildId) qs.append('build_id', buildId); - if (this.env.git?.branch) qs.append('branch', this.env.git.branch); - if (this.env.target?.branch) qs.append('target_branch', this.env.target.branch); - if (this.env.git?.sha) qs.append('commit_sha', this.env.git.sha); - if (this.env.target?.commit) qs.append('target_commit_sha', this.env.target.commit); - if (this.env.pullRequest != null) qs.append('pull_request_number', String(this.env.pullRequest)); - if (this.env.partial) qs.append('partial', 'true'); const query = qs.toString(); return this.get( diff --git a/packages/client/test/client.test.js b/packages/client/test/client.test.js index bdc56f81e..bc2754cb9 100644 --- a/packages/client/test/client.test.js +++ b/packages/client/test/client.test.js @@ -917,7 +917,7 @@ describe('PercyClient', () => { beforeEach(() => stubEnv()); - it('issues a GET with no query params when env is empty', async () => { + it('issues a GET with no query params when no build id is given', async () => { const path = '/intelli_story/snapshot-name-to-commit'; api.reply(path, () => [200, { data: { foo: 'sha-foo' } }]); @@ -929,7 +929,22 @@ describe('PercyClient', () => { expect(api.requests[path][0].method).toBe('GET'); }); - it('appends git/target/PR/partial context when present in env', async () => { + it('includes the build_id when provided', async () => { + const expectedPath = '/intelli_story/snapshot-name-to-commit?build_id=bld-123'; + api.reply(expectedPath, () => [200, { data: { b: 'sha-b' } }]); + + await expectAsync( + client.getIntelliStorySnapshotNameToCommit('bld-123') + ).toBeResolvedTo({ data: { b: 'sha-b' } }); + + expect(api.requests[expectedPath]).toBeDefined(); + expect(api.requests[expectedPath][0].method).toBe('GET'); + }); + + // The endpoint resolves the base build from the build id alone, so git/PR + // context is no longer sent — build_id must be the only query param even + // when the environment has a full git context to offer. + it('does not send git/target/PR/partial context even when present in env', async () => { stubEnv({ git: { branch: 'feature/x', sha: 'commit-sha-1' }, target: { branch: 'main', commit: 'commit-sha-2' }, @@ -937,48 +952,14 @@ describe('PercyClient', () => { partial: true }); - const expectedPath = '/intelli_story/snapshot-name-to-commit?' + [ - 'branch=feature%2Fx', - 'target_branch=main', - 'commit_sha=commit-sha-1', - 'target_commit_sha=commit-sha-2', - 'pull_request_number=42', - 'partial=true' - ].join('&'); - + const expectedPath = '/intelli_story/snapshot-name-to-commit?build_id=bld-456'; api.reply(expectedPath, () => [200, { data: { a: 'sha-a' } }]); await expectAsync( - client.getIntelliStorySnapshotNameToCommit() + client.getIntelliStorySnapshotNameToCommit('bld-456') ).toBeResolvedTo({ data: { a: 'sha-a' } }); - expect(api.requests[expectedPath]).toBeDefined(); - expect(api.requests[expectedPath][0].method).toBe('GET'); - }); - - it('includes pull_request_number=0 when env.pullRequest is 0 (not null)', async () => { - stubEnv({ pullRequest: 0 }); - - const expectedPath = '/intelli_story/snapshot-name-to-commit?pull_request_number=0'; - api.reply(expectedPath, () => [200, { data: {} }]); - - await expectAsync( - client.getIntelliStorySnapshotNameToCommit() - ).toBeResolvedTo({ data: {} }); - - expect(api.requests[expectedPath]).toBeDefined(); - }); - - it('includes the build_id when provided', async () => { - const expectedPath = '/intelli_story/snapshot-name-to-commit?build_id=bld-123'; - api.reply(expectedPath, () => [200, { data: { b: 'sha-b' } }]); - - await expectAsync( - client.getIntelliStorySnapshotNameToCommit('bld-123') - ).toBeResolvedTo({ data: { b: 'sha-b' } }); - - expect(api.requests[expectedPath]).toBeDefined(); - expect(api.requests[expectedPath][0].method).toBe('GET'); + expect(Object.keys(api.requests)).toEqual([expectedPath]); }); });